ibmruntimes / openj9-openjdk-jdk

Extensions for OpenJDK for Eclipse OpenJ9
GNU General Public License v2.0
17 stars 73 forks source link

Fix mapping of .copyrightignore patterns to regular expressions #809

Closed keithc-ca closed 2 months ago

keithc-ca commented 2 months ago

The .copyrightignore file in each of the extension repositories contains some comments attempting to describe how patterns are interpreted. Those comments and the actual behavior of the groovy script are not consistent. See the console log of https://openj9-jenkins.osuosl.org/job/PullRequest-CopyrightCheck-OpenJDK11/1001 for a recent example.

The table below shows a few patterns from jdk11, showing for each the derived regular expression and problems for that entry.

Pattern Resulting Regular Expression Comments
src/bsd/doc src/+bsd/+doc.* no need to use +
src/jdk.crypto.ec/share/native/libsunec/impl src/+jdk.crypto\.ec/+share/+native/+libsunec/+impl first . should be escaped, .\* suffix is missing
/test test/.*|$ unwanted \|, useless $ anchor
/*LICENSE .*LICENSE/.*|$ unwanted /.*, unwanted \|, useless $ anchor
*.gif .*\.gif (no problem)

This change improves the mapping of those patterns to regular expressions.

This table shows a few exmples of the new behavior.

Pattern Resulting Regular Expression
src/bsd/doc (.+/)?src/bsd/doc(/.+)?
/test test(/.+)?
*.jpg (.+/)?[^/]*\.jpg(/.+)?

This also tidies up .copyrightignore to:

AdamBrousseau commented 2 months ago

Jenkins copyright check

AdamBrousseau commented 2 months ago

Jenkins copyright check

AdamBrousseau commented 2 months ago

Jenkins copyright check

AdamBrousseau commented 2 months ago

Jenkins copyright check

AdamBrousseau commented 2 months ago

Jenkins copyright check

AdamBrousseau commented 2 months ago

Jenkins copyright check

AdamBrousseau commented 2 months ago

Jenkins copyright check

keithc-ca commented 2 months ago

@AdamBrousseau I managed to get a good run by using the "replay" link; see https://openj9-jenkins.osuosl.org/job/PullRequest-CopyrightCheck-OpenJDK/1587.

AdamBrousseau commented 2 months ago

Jenkins copyright check

AdamBrousseau commented 2 months ago

Ok. Yea I did notice a green build but didn't look into how/why. This last run should work too.

AdamBrousseau commented 2 months ago

Is it worth testing this on one/all of the entire repos? eg https://openj9-jenkins.osuosl.org/job/CopyrightCheck-OpenJDK/

AdamBrousseau commented 2 months ago

Seems there are 2 groovy files and one shell script. Since the other job uses the other groovy file and you aren't changing the sh script, doesn't seem like we'd be exercising more than just the ignore file if we run one of those jobs.

keithc-ca commented 2 months ago

there are 2 groovy files and one shell script

Are copyrightCheckDir.groovy and copyrightCheckDir.sh used anywhere? The latter doesn't appear to consult .copyrightignore (the patterns are hard-coded). Should those two files just be removed?

AdamBrousseau commented 2 months ago

The Copyright*OpenJDK jobs use copyrightCheckDir.groovy which calls the sh. https://openj9-jenkins.osuosl.org/view/Copyright%20Checks/ https://github.com/ibmruntimes/openj9-openjdk-jdk/blob/844db8c8449a5d82f5beaaef0a533aafeeae5e84/buildenv/jenkins/jobs/infrastructure/copyrightCheckDir.groovy#L54

Not really sure why it isn't one groovy file and not sure why the PR one doesn't call the sh.

keithc-ca commented 2 months ago

Those other two files seem to serve an entirely different purpose. Perhaps they should be updated to consult .copyrightignore as well, but that change should not delay this one.