LICENSE and check for grpc-rust.#2748
Conversation
There was a problem hiding this comment.
I think we could simplify this with a few minor changes.
-
We don't need to worry about files with apache licenses at all. We should just be enforcing that our source files have the MIT license. The Apache licensed code that is grpc-authored is all the protos that we just make copies of here.
-
The ALLOWED_AUTHORS thing can be removed and we can assert that the copyright directly states "gRPC authors".
-
I don't think we need a separate LICENSE in the grpc directory. We should update the LICENSE in the root, and also have @LucioFranco update all the copyright notices to have the same author ("gRPC authors") - or send him a PR that does this for him to approve. [EDIT: by "all" I mean the ones that are currently "(c) Lucio Franco"]
-
The
.pre-commit-config.yamland surrounding tooling feels like overkill to me at this point.
|
If someone can send me a PR that would be best otherwise i think what @dfawley said makes sense. |
|
I'll send you a PR @LucioFranco - I didn't want to overstep, but if you're good switching the licenses at this juncture I'll go ahead and do that for the entire repo. I won't merge this PR until that one is in. Should I move the tools/ directory then to the top level and have this check the entire repo for the MIT/gRPC authors license or leave it across the grpc/ directory only? Also note this doesn't cover protos really, but should be adaptable to do so if we need it. |
SGTM.
If it's not too painful, that would be great. If there are a ton of problems then we can expand the scope over time. |
|
It's probably easy enough to put this across the entire repo. I was being conservative in scope just because I didn't want to overstep. One last question: should I also try and cover proto files? These are largely under Apache licenses in the larger repo. Ignoring third_party and special cases (rls) is easy, but I'm not wholly sure what to do with the protos, some of which date back to 2015 or so and are licensed to Google LLC. For instance: examples/proto/googleapis/google/api/http.proto |
|
We don't need an automated checker for those. However we want to ignore them should be fine. The main thing we need the checker for is to ensure that new files get the proper license clause in them. AFAIK we don't intend to author new proto files in this repo so if you were wanting to exclude anything ending in |
|
Ok, I think this is ready to go. |
dfawley
left a comment
dfawley
left a comment
There was a problem hiding this comment.
Thanks for jumping on this! Just a couple more minor nits.
| Checks source files (*.rs, *.proto) for exact match against MIT or Apache 2.0 license boilerplate. | ||
| Performs piecewise validation and unified diffing against expected boilerplate templates. |
|
|
||
| MIN_YEAR = 2025 | ||
|
|
||
| # Regex to capture: (prefix) Copyright [(c)] (year_spec) |
There was a problem hiding this comment.
Why is there an optional (c)? Are we inconsistent?
| elif first_line.startswith("//"): | ||
| for line in lines: | ||
| if line.strip().startswith("//"): | ||
| header.append(line.rstrip()) | ||
| else: | ||
| break | ||
| return header, None |
There was a problem hiding this comment.
Can this bit be removed now? Or do we support //? Also: related things mentioning // above/below.
|
|
||
| def validate_copyright_years(year_spec: str) -> tuple[bool, str | None]: | ||
| current_year = datetime.date.today().year | ||
| years = [int(y) for y in re.findall(r"\b\d{4}\b", year_spec)] |
There was a problem hiding this comment.
I think this is more complex than it needs to be since we removed the range support?
3 participants
Motivation
Ensure that licenses are being added and not improperly modified to all code files, in compliance with CNCF requirements.
Solution
This is a python script that runs in the CI and can be configured to run in a pre-commit. It also adds a LICENSE file to the grpc directory to conform with the MIT license being used throughout that part of the code-base. This script only checks the grpc directory at this time.