Skip to content
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

Copyright should only contain year of file creation #241

Closed
afranken opened this issue Aug 14, 2020 · 5 comments
Closed

Copyright should only contain year of file creation #241

afranken opened this issue Aug 14, 2020 · 5 comments

Comments

@afranken
Copy link
Member

We're currently enforcing every file to contain a copyright header, which is mandatory for Adobe OpenSource projects.

Using the https://github.com/mycila/license-maven-plugin/, we're enforcing every header to contain a date in the format of
<year-of-project-inception> - <current-year>
even though that's completely unnecessary.

We should enforce the file to contain a date in the format of
<year-of-file-creation>
this will still comply to the mandatory copyright header format.

@afranken afranken self-assigned this Aug 14, 2020
@afranken afranken added the bug label Aug 14, 2020
@afranken
Copy link
Member Author

Looking at the https://github.com/mycila/license-maven-plugin/ documentation, this may be hard to configure.
Even though this would be an easy check using e.g. a regular expression, the license plugin is built not only to enforce the existence of a header, but it can also add the header.

Because of this feature, the plugin reads the template and replaces any properties found, in our case ${license.git.copyrightYears} which translates to <year-of-project-inception> - <current-year>.

It seems that other people are having the same problem:
mathieucarbou/license-maven-plugin#124

In version 4.0-rc1, a feature was added that allows tracking the file creation date using Git:
https://github.com/mycila/license-maven-plugin/milestone/14?closed=1
mathieucarbou/license-maven-plugin#145

I'll have a look if that works for us without breaking this project and changing all existing copyright headers...

@afranken
Copy link
Member Author

also see discussion in #231

@afranken
Copy link
Member Author

PoC:
adding <license.git.copyrightLastYearMaxCommitsLookup>100000</license.git.copyrightLastYearMaxCommitsLookup> to the configuration to enable finding a file's creation date even in large GIT histories and changing the line in the copyright template to Copyright ${license.git.copyrightCreationYear} Adobe. works with version `4.0.rc2.

Caveat:
we need to fix all headers in all files if we do this change.

Annoying:

  1. if a header is in the wrong format (e.g. wrong year) the plugin will complain:
    [WARNING] Missing header in: /Users/franken/github/adobe/S3Mock/pom.xml
    The error message couldn't be less helpful - the header is not missing...

I haven't found a way to change this to useful output.
Only the whole message could be changed, it will never change based on the problem found by the plugin.

  1. When you locally change a file, for some reason the plugin thinks it should use the current year instead of the creation year.
    So if we always run the check on every maven build, the build will always fail if you change a file. (I see this right now - I changed the plugin's configuration and now it complains about the main pom.xml header to contain the year 2020. But once I commit, it must contain the year 2017.

We could mitigate this by checking the license in a profile.
This would make things less understandable though - the build would only fail when you know which profiles to activate, so a contributor would see the build passing locally, and it would fail with an obscure error message in the PRB.
I could also open an issue against the plugin and provide a fix. It seems that this year lookup is supposed to work around the fact that previously uncommitted files do not have commits to look up the year. So they're checking to see if it's currently unstaged and return the current year before even checking whether commits exist:
https://github.com/mycila/license-maven-plugin/blob/master/license-maven-plugin-git/src/main/java/com/mycila/maven/plugin/license/git/GitLookup.java#L148
This should be done as a fallback after we know that there are no commits for a file.

=========

We could let the plugin automatically change/add the headers.
I'm personally not a big fan of having third party plugins change our code, but it would get rid of this problem from a contributor's point of view.
Vladimir even suggested to auto-format our code instead of just checking with the Checkstyle plugin (again a plugin with sometimes unhelpful error messages).

@afranken
Copy link
Member Author

afranken commented Sep 1, 2022

I looked into this two years ago, the option was not available in the plugin and just learned to live with it.
Updating the copyright header is a bit tedious but not too hard to do.

The license-maven-plugin has since added license.git.copyrightCreationYear which means "file creation date in GIT" to the copyright instead of license.git.copyrightYears which lists inceptionYear and currentYear, but it requires lookups in the GIT history.
This works for some files, but does not (really) work for others - or, rather, it probably depends on interpretation.
If I create a file src/main/java/com/test/Test.java in 2018, this year will be enforced in the copyright header.
If I later refactor this file in 2022 and move it to a different package src/main/java/com/test/newpackage/Test.java, the year 2022 will be enforced since that is the creation date of the file at that location.
Not sure is this would be correct since the file itself was created in 2018, and the copyright started there...

I'll close this issue and maybe try out the new version with file creation date.

@afranken
Copy link
Member Author

afranken commented Sep 1, 2022

In case I (or someone else) picks this up again:
I change the copyright header from
Copyright ${license.git.copyrightYears} Adobe.
to
Copyright ${license.git.copyrightCreationYear} Adobe.

and ran the command

mvn license:format

to re-format all files in the repo.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

1 participant