-
Notifications
You must be signed in to change notification settings - Fork 747
trd: license and copyright policy #3318
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
First drat of TRD for license and copyright rules for the Tock project.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Updating #3317 to match the process in this document, as best as I can tell, would require the following changes:
- Updating non-rust files that are still code files (e.g. the python/bash code in
tools/
and our Makefiles) to include the license notice - Remove the "Copyright Tock Contributors" line from each file, only have a Copyright line if someone chooses to add it.
- Add the line "Licensed under the Apache License, Version 2.0 or the MIT License." as the first line of each file, before the SPDX license identifier.
Overall this approach looks good to me. Curious to hear what @jrvanwhy in particular thinks about not requiring the "Copyright Tock Contributors" line on all files without a different copyright assignment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This all looks like a reasonable approach to me.
We're going to take another pass on this to condense it and remove some unnecessary requirements. Unlike most other PRs, let's hold off on merging after approvals in order to get feedback from external contributors first. But please approve once you're comfortable so we can be confident that what we get feedback on is similar to what we'll merge. (draft coming later today) |
Quick comment re the formatting bits: the pedantic, low-level bits were written with an eye towards making Johnathan's life easier re: automatic enforcement. I do think top of file is important and clustering licenses and copyright; the rest I have less attachment to. |
In this respect, I think there are two distinct groups of license notices: Tock project notices: Notices added by Tock contributors when they contribute to the Tock project. When I commented on the Format section, I assumed this policy would apply to both sets of license notices. I think that being picky about 3p notices makes it difficult to import code from other projects. We could limit our formatting requirements and automated enforcement to Tock project notices. Then we could import code from other projects as long as that code is also (Apache 2.0 OR MIT)-licensed, while remaining picky about most of the license headers in the repository. We'll probably end up with an exceptions list in the tool. For example, if someone copies a file from another project without making any changes to it, then it should be committed without a Tock project notice. In that case, we'll need to list the file as an exception in the tool. When we eventually make our own contributions to that file, we will then add a Tock project notice and remove the license exception. |
I just finished typing up the notes from today's core WG call. Having listened to the discussion again, I've changed my mind slightly on this. If we copy a file from another (Apache 2.0 OR MIT) project into Tock, I think it is totally fine to add something like:
at the top of the file. It's the copyright line that we shouldn't add if we didn't contribute to the file. However, I do think we should allow some variation in the format if we use a file from another project. For example, if we copy in a file from another project with the following header:
I think we should leave it intact, rather than requiring us to change it to:
and the enforcement tool should be flexible enough to make that easy. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Based on my reading of the document, I think the enforcement tool should:
- Verify the license information is present near the top of each file and formatted according to this policy.
- NOT look for license information elsewhere in the file, and ignore any that is present.
With this implementation, we could copy code from other projects with license/copyright information in the middle of the file without adding an exception in the tool. The drawback is that it would not catch errant license headers in the middle of new contributions.
What do you think?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This looks pretty good to me.
Additional conclusions:
|
21609c7
I fixed the header formatting. |
I just noticed I never answered this question. Personally, I'm not a fan of making the copyright notice optional. Making the copyright notice optional will tend to overrepresent the contributions of some contributors relative to others. For example, suppose contributors A, B, C, D, and E work together on a file (all contributing significantly), but the only contributor who wants to add a copyright notice is A. In that case, the license header will become:
Someone who is not from the Tock project who stumbles across this file will see it as primarily A's work, rather than a community project. Requiring the "Copyright Tock Contributors" line would change that to:
which is a more accurate representation of the code's authorship. |
I share the spirit of @jrvanwhy's comment. Yet, I'm unsure whether requiring such a line might not introduce some issues: obviously we'd not add such a line on a compatibly-licensed, vendored file. But even when a file is added e.g. by its original author to Tock, I think it might be weird to force this developer to attribute the wider "Tock contributors" despite the file not having received any contributions other than the original author's (yet). I think having a "Copyright Tock Contributors" line in each file as a convention is good, and I would adapt that, but I'm wary about enforcing it. I presume every contributor would be able to decide on their own whether to attribute just themselves, the "Tock contributors", or both. |
We could require the "Copyright Tock Contributors" line only in cases where non-trivial contributions are made by contributors who do not have their own copyright lines. In practice, I think this would result in most files carrying only the "Copyright Tock Contributors" line (because most of y'all seem to prefer that over per-person copyrights), and a handful of files having 1 or 2 specific contributors named. The downside is that "non-trivial contributions" is a bit vague and therefore this cannot be automatically enforced, but I don't expect that will be an issue in practice. |
I disagree that this will not be an issue in practice. What will happen without automatic enforcement is there will be tons of files with "Copyright FIRST_AUTHOR" at the top that have in fact been modified by dozens of other people. For example, Phil used to often add an I think it is much easier in practice to just require "Copyright Tock Contributors". If an individual author wishes to add an additional copyright for just their own name they are free to do so. |
I'm going to convert this from a Draft PR to a real PR. I believe the remaining issue is just whether to require a copyright line or make it optional. Please comment if there is more missing still. |
…ate examples, include "Copyright Tock Contributors" in the examples.
License policy 2023-01-23 core call updates
…ividual contributor copyright from the main example.
bors r+ |
Build succeeded:
|
3345: Add a license checking tool. r=alevy a=jrvanwhy ### Pull Request Overview This pull request adds a tool that checks for license headers as specified in #3318. ### Testing Strategy I adapted `ci-job-tools` to run tests in the `tools/` workspace, and ran the tool by hand to verify it reports errors + success correctly. ### TODO or Help Wanted #3318 is undergoing revision. I currently have the license checker disabled on all files *except new files added in this PR*, because otherwise it does its job and fails CI. Also, after license headers are added, I need to evaluate the speed of the tool. It currently runs in 100 milliseconds, which seems fast enough, but it may slow down when license headers are added (it may also speed up). If it slows down too much (e.g. takes over a second), I'll revert to a simpler -- but less accurate -- parsing implementation. ### Documentation Updated - [X] Updated the relevant files in `/docs`, or no updates are required. ### Formatting - [X] Ran `make prepush`. Co-authored-by: Johnathan Van Why <jrvanwhy@google.com>
Pull Request Overview
First draft of TRD for license and copyright rules for the Tock project.
Testing Strategy
N/A
TODO or Help Wanted
Review of language and policy decisions.
Documentation Updated
/docs
, or no updates are required.Formatting
make prepush
.