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

Enable copyright-check and update copyrights #4650

Closed
wants to merge 1 commit into from

Conversation

jbescos
Copy link
Member

@jbescos jbescos commented Dec 2, 2020

Currently the build doesn't fail when copyright fails because we need to execute this one: glassfish-copyright:check

There are many dates to update.

  • Jersey-bot updates pom files version, but the date was not updated. When jersey-bot runs it should invoke 'glassfish-copyright:repair'
  • Many files were updated in this commit 'Move CDI integration tests to a common CDI-Integration module (Move CDI integration tests to a common CDI-Integration module #4280)' but the date was not updated.

@senivam
Copy link
Contributor

senivam commented Dec 2, 2020

actually I do not agree with automatic build failure (that is why it was copyright:copyright). Typical use case is when new Jersey release is published and all pom.xml files are modified and they are being modified with jersey-bot which belongs to Eclipse and has nothing to do with Oracle. Any other contribution from community also does not necessary shall update Oracle's copyright. Payara for instance has its own copyright.

Basically the whole community shall not bother with Oracle's copyright, except people from Oracle :)

@jansupol
Copy link
Contributor

jansupol commented Dec 2, 2020

This is NOT Oracle copyright. This is Oracle and affiliates' copyright. If I understand it correctly, any contributor is understood as an affiliate.

Regarding the release, I believe it is possible to update the copyright year in poms by a sed swarm, is it not?

@@ -1,4 +1,5 @@
/*
* Copyright (c) 2020 Oracle and/or its affiliates. All rights reserved.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There seems to be something wrong. This file was not modified in 2020 and yet the copyright plugin requires a copyright change.

Copy link
Member Author

@jbescos jbescos Dec 2, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It is because the plugin removed the section, and this is considered as a new change.

  • Contributors:
  • Marcelo Rubim

There is also a new line added.

@@ -1,5 +1,5 @@
/*
* Copyright (c) 2014, 2019Oracle and/or its affiliates. All rights reserved.
* Copyright (c) 2020 Oracle and/or its affiliates. All rights reserved.
Copy link
Member Author

@jbescos jbescos Dec 2, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This seems wrong, I will fix it by 2014, 2020

@senivam
Copy link
Contributor

senivam commented Dec 2, 2020

This is NOT Oracle copyright. This is Oracle and affiliates' copyright. If I understand it correctly, any contributor is understood as an affiliate.

As I remember, this project was contributed to Eclipse, since then any contributor is understood as Eclipse's affiliate. That is why ECA sign is required for any IP contribution.

If it would be Helidon project, then yes, it belongs to Oracle, but this project belongs to Eclipse.

@m0mus
Copy link
Member

m0mus commented Dec 3, 2020

Just want to confirm the copyright should be in this format:

Copyright (c) 2002, 2020 Oracle and/or its affiliates. All rights reserved.

where 2002 is the year when this file was originally created and 2020 the year of last change.
If the file is new it's just

Copyright (c) 2020 Oracle and/or its affiliates. All rights reserved.

@senivam
Copy link
Contributor

senivam commented Dec 3, 2020

OK, after all discussions I have to agree with build failure in Travis build due to copyright year changes.

@jbescos jbescos force-pushed the copyright branch 2 times, most recently from cec2c9e to 77663d8 Compare December 4, 2020 07:20
@jbescos
Copy link
Member Author

jbescos commented Dec 4, 2020

For some reason in Travis it fails in the first module, but there is no information about why it fails. Locally it is working.

I need to push some commits to print more information, but the problem is that Travis seems to be running nightly. It is going to take some time to find why is it failing in Travis.

@@ -1,4 +1,20 @@
# Notice for Jersey Json Jackson module
#
# Copyright (c) 2020 Oracle and/or its affiliates. All rights reserved.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this is kinda .md file (only with .markdown extension), shall be excluded from Copyright check.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done, it is also added in the .markdown in the jersey/etc/config/copyright-exclude

pom.xml Outdated
<configuration>
<excludeFile>etc/config/copyright-exclude</excludeFile>
<!--svn|mercurial|git - defaults to svn-->
<scm>git</scm>
<!-- turn on/off debugging -->
<debug>false</debug>
<debug>true</debug>
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why in debug mode?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have added it temporarily to know why is it failing in Travis. Currently it fails but there is no information of the error.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

for now build did not even started since the last change :) Travis seems to be really slow recently.
Regarding additional info - it might not show it because of reduced output in Travis. We had to reduce it due to Travis' limitations, otherwise all build threads would fail. May be you can temporary try to run it with full log output (just for copyright-check thread) and this will show what actually happened there?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ah, no, copyright already runs with full output, so may be -e key will help when build finally starts :)

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I tried it yesterday with -e already but the stacktrace doesn't help, you can see it here:
https://travis-ci.org/github/eclipse-ee4j/jersey/jobs/747370995

Hopefully debug=true will help.

Locally there are no problems. I don't really understand why in Travis it works different.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the issue with no output on failures seems to be caused by this hard-coded logic inside the plugin:
https://github.com/eclipse-ee4j/glassfish-copyright-plugin/blob/94c7ba4c6d3151b57f92cf030f77661f1ff15d17/src/main/java/org/glassfish/copyright/CheckCopyrightMojo.java#L33
It's being always quiet in other words. And when you remove this hard-coded line (default is false), you can see why build fails. However fix requires new PR and then new plugin version shall be released...

@jbescos jbescos force-pushed the copyright branch 2 times, most recently from 155f57e to 4f65556 Compare December 7, 2020 07:37
Copy link
Contributor

@senivam senivam left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

current content looks good to me, however due to discovered issue with glassfish-copyright-plugin it's required to fix (or provide some workaround) for error output, and this will introduce some changes in the current PR (like plugin version update or something), so, it shall be merged only after reliable solution for error output inside glassfish-copyright-plugin is provided (and this PR is adjusted accordingly).

@senivam
Copy link
Contributor

senivam commented Dec 8, 2020

@jbescos could you please update plugin's version to 2.4? And it's required to disable quiet mode by external parameter -Dcopyright.quiet=false

@jbescos jbescos force-pushed the copyright branch 2 times, most recently from 912027f to 5de6d9f Compare December 10, 2020 08:04
@jbescos
Copy link
Member Author

jbescos commented Dec 14, 2020

@senivam finally it is working in Travis

Copy link
Contributor

@senivam senivam left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

it looks like all passes OK, settings for copyright plugin and related build configuration are adjusted correctly, changes can be merged.

Signed-off-by: Jorge Bescos Gascon <[email protected]>
@jbescos
Copy link
Member Author

jbescos commented Jan 26, 2021

There are some issues with the rebase of my commits, because my commits belongs to 2020 and new files have been modified in 2021. The plugin is not dealing well with that.

@jbescos jbescos closed this Jan 26, 2021
@jbescos jbescos mentioned this pull request Jan 26, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants