Re: RFR: 8333685: Make update-copyright-year script more useful [v3]
On Wed, 12 Jun 2024 14:04:41 GMT, Sonia Zaldana Calles wrote: >> Hi all, >> >> This PR addresses [8333685](https://bugs.openjdk.org/browse/JDK-8333685). >> >> I have added the following enhancements: >> - Removed uses of `fgrep` and `egrep` with `grep -F` and `grep -E` >> respectively. >> - Altered default behaviour to limit the processed changesets to those in >> the current branch and the current year. >> - Enabled an option to modify all changesets in the year if desired (this >> was the previous default behaviour). >> - Added named parameters and enabled `--help`. >> - Removed mercurial support. >> - Fixed a bug where copyright headers that didn't have a comma following the >> initial year of creation were not getting picked up. For example, >> [here](https://github.com/openjdk/jdk/blob/512b2b4f141f9a202984150b0427372e1a409a50/src/hotspot/cpu/zero/copy_zero.hpp#L3). >> >> - Fixed a bug where copyright headers missing the copyright symbol (c) were >> not getting picked up. Refer to the example above as well. >> >> Thanks, >> Sonia > > Sonia Zaldana Calles has updated the pull request incrementally with two > additional commits since the last revision: > > - Support for copyright headers that don't add a comma after final copyright > year > - Changes based on feedback Marked as reviewed by erikj (Reviewer). - PR Review: https://git.openjdk.org/jdk/pull/19605#pullrequestreview-2115736731
Re: RFR: 8333685: Make update-copyright-year script more useful [v3]
On Wed, 12 Jun 2024 14:04:41 GMT, Sonia Zaldana Calles wrote: >> Hi all, >> >> This PR addresses [8333685](https://bugs.openjdk.org/browse/JDK-8333685). >> >> I have added the following enhancements: >> - Removed uses of `fgrep` and `egrep` with `grep -F` and `grep -E` >> respectively. >> - Altered default behaviour to limit the processed changesets to those in >> the current branch and the current year. >> - Enabled an option to modify all changesets in the year if desired (this >> was the previous default behaviour). >> - Added named parameters and enabled `--help`. >> - Removed mercurial support. >> - Fixed a bug where copyright headers that didn't have a comma following the >> initial year of creation were not getting picked up. For example, >> [here](https://github.com/openjdk/jdk/blob/512b2b4f141f9a202984150b0427372e1a409a50/src/hotspot/cpu/zero/copy_zero.hpp#L3). >> >> - Fixed a bug where copyright headers missing the copyright symbol (c) were >> not getting picked up. Refer to the example above as well. >> >> Thanks, >> Sonia > > Sonia Zaldana Calles has updated the pull request incrementally with two > additional commits since the last revision: > > - Support for copyright headers that don't add a comma after final copyright > year > - Changes based on feedback Good. Thank you! - Marked as reviewed by stuefe (Reviewer). PR Review: https://git.openjdk.org/jdk/pull/19605#pullrequestreview-2115194804
Re: RFR: 8333685: Make update-copyright-year script more useful [v2]
On Wed, 12 Jun 2024 06:46:39 GMT, Thomas Stuefe wrote: > A lot better, but not fully there yet. Can you make it so that it also picks > up SAP's copyrights when they specify multiple years? They are weird in that > the last year does not have a trailing comma, e.g.: > > ``` > * Copyright (c) 2020, 2023 SAP SE. All rights reserved. > ``` Nice catch! I also noticed the trailing comma was mistakenly getting added to these cases. I've pushed the fix. - PR Comment: https://git.openjdk.org/jdk/pull/19605#issuecomment-2163089496
Re: RFR: 8333685: Make update-copyright-year script more useful [v3]
> Hi all, > > This PR addresses [8333685](https://bugs.openjdk.org/browse/JDK-8333685). > > I have added the following enhancements: > - Removed uses of `fgrep` and `egrep` with `grep -F` and `grep -E` > respectively. > - Altered default behaviour to limit the processed changesets to those in the > current branch and the current year. > - Enabled an option to modify all changesets in the year if desired (this was > the previous default behaviour). > - Added named parameters and enabled `--help`. > - Removed mercurial support. > - Fixed a bug where copyright headers that didn't have a comma following the > initial year of creation were not getting picked up. For example, > [here](https://github.com/openjdk/jdk/blob/512b2b4f141f9a202984150b0427372e1a409a50/src/hotspot/cpu/zero/copy_zero.hpp#L3). > > - Fixed a bug where copyright headers missing the copyright symbol (c) were > not getting picked up. Refer to the example above as well. > > Thanks, > Sonia Sonia Zaldana Calles has updated the pull request incrementally with two additional commits since the last revision: - Support for copyright headers that don't add a comma after final copyright year - Changes based on feedback - Changes: - all: https://git.openjdk.org/jdk/pull/19605/files - new: https://git.openjdk.org/jdk/pull/19605/files/483c7fa9..754213b4 Webrevs: - full: https://webrevs.openjdk.org/?repo=jdk=19605=02 - incr: https://webrevs.openjdk.org/?repo=jdk=19605=01-02 Stats: 9 lines in 1 file changed: 3 ins; 0 del; 6 mod Patch: https://git.openjdk.org/jdk/pull/19605.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/19605/head:pull/19605 PR: https://git.openjdk.org/jdk/pull/19605
Re: RFR: 8333685: Make update-copyright-year script more useful [v2]
On Wed, 12 Jun 2024 06:43:15 GMT, Thomas Stuefe wrote: >> Sonia Zaldana Calles has updated the pull request incrementally with one >> additional commit since the last revision: >> >> Fixing optional group to work with bsd as well > > make/scripts/update_copyright_year.sh line 70: > >> 68: echo "y Specifies the copyright year. Set to current year by >> default." >> 69: echo "h Print this help." >> 70: echo "f Updates all change sets in a full year." > > - Please specify the leading dash on all options > - I would print "y" after "f" and write something along these lines "If -f is > specified, defines the year of changes for which to modify the copyright > (current year if omitted)" Hi Thomas, I assume with this we hope to clarify what time period of changes we are addressing with `-f`. Would something like this communicate what you suggested? Usage: update_copyright_year.sh [-c company] [-y year] [-h|f] options: -c Specifies the company. Set to Oracle by default. -y Specifies the copyright year. Set to current year by default. -f Updates the copyright for all change sets in a given year, as specified by -y. -h Print this help. - PR Review Comment: https://git.openjdk.org/jdk/pull/19605#discussion_r1636490568
Re: RFR: 8333685: Make update-copyright-year script more useful [v2]
On Tue, 11 Jun 2024 13:34:24 GMT, Sonia Zaldana Calles
wrote:
>> Hi all,
>>
>> This PR addresses [8333685](https://bugs.openjdk.org/browse/JDK-8333685).
>>
>> I have added the following enhancements:
>> - Removed uses of `fgrep` and `egrep` with `grep -F` and `grep -E`
>> respectively.
>> - Altered default behaviour to limit the processed changesets to those in
>> the current branch and the current year.
>> - Enabled an option to modify all changesets in the year if desired (this
>> was the previous default behaviour).
>> - Added named parameters and enabled `--help`.
>> - Removed mercurial support.
>> - Fixed a bug where copyright headers that didn't have a comma following the
>> initial year of creation were not getting picked up. For example,
>> [here](https://github.com/openjdk/jdk/blob/512b2b4f141f9a202984150b0427372e1a409a50/src/hotspot/cpu/zero/copy_zero.hpp#L3).
>>
>> - Fixed a bug where copyright headers missing the copyright symbol (c) were
>> not getting picked up. Refer to the example above as well.
>>
>> Thanks,
>> Sonia
>
> Sonia Zaldana Calles has updated the pull request incrementally with one
> additional commit since the last revision:
>
> Fixing optional group to work with bsd as well
A lot better, but not fully there yet. Can you make it so that it also picks up
SAP's copyrights when they specify multiple years? They are weird in that the
last year does not have a trailing comma, e.g.:
* Copyright (c) 2020, 2023 SAP SE. All rights reserved.
make/scripts/update_copyright_year.sh line 70:
> 68: echo "y Specifies the copyright year. Set to current year by
> default."
> 69: echo "h Print this help."
> 70: echo "f Updates all change sets in a full year."
- Please specify the leading dash on all options
- I would print "y" after "f" and write something along these lines "If -f is
specified, defines the year of changes for which to modify the copyright
(current year if omitted)"
make/scripts/update_copyright_year.sh line 103:
> 101: [ -d "${this_script_dir}/../../.git" ] && git_found=true
> 102: if [ "$git_found" != "true" ]; then
> 103: echo "Error: could not auto-detect git as the version control system"
Pre-existing. The error message is confusing (I first tried your script from a
different location). Better would be adding something like this:
"Please execute script from within make/scripts"
-
Changes requested by stuefe (Reviewer).
PR Review: https://git.openjdk.org/jdk/pull/19605#pullrequestreview-2112081245
PR Review Comment: https://git.openjdk.org/jdk/pull/19605#discussion_r1635897295
PR Review Comment: https://git.openjdk.org/jdk/pull/19605#discussion_r1635901072
Re: RFR: 8333685: Make update-copyright-year script more useful [v2]
On Tue, 11 Jun 2024 13:34:24 GMT, Sonia Zaldana Calles wrote: >> Hi all, >> >> This PR addresses [8333685](https://bugs.openjdk.org/browse/JDK-8333685). >> >> I have added the following enhancements: >> - Removed uses of `fgrep` and `egrep` with `grep -F` and `grep -E` >> respectively. >> - Altered default behaviour to limit the processed changesets to those in >> the current branch and the current year. >> - Enabled an option to modify all changesets in the year if desired (this >> was the previous default behaviour). >> - Added named parameters and enabled `--help`. >> - Removed mercurial support. >> - Fixed a bug where copyright headers that didn't have a comma following the >> initial year of creation were not getting picked up. For example, >> [here](https://github.com/openjdk/jdk/blob/512b2b4f141f9a202984150b0427372e1a409a50/src/hotspot/cpu/zero/copy_zero.hpp#L3). >> >> - Fixed a bug where copyright headers missing the copyright symbol (c) were >> not getting picked up. Refer to the example above as well. >> >> Thanks, >> Sonia > > Sonia Zaldana Calles has updated the pull request incrementally with one > additional commit since the last revision: > > Fixing optional group to work with bsd as well Looks good to me. I haven't tried it myself though. - Marked as reviewed by erikj (Reviewer). PR Review: https://git.openjdk.org/jdk/pull/19605#pullrequestreview-2110924115
Re: RFR: 8333685: Make update-copyright-year script more useful
On Tue, 11 Jun 2024 13:19:48 GMT, Thomas Stuefe wrote:
>> Opinion: while it's good to see improvements to the existent script, since
>> JEP 330, we can now conveniently implement a similar script in Java. That'll
>> also automatically take care of OS specifics.
>
>> Opinion: while it's good to see improvements to the existent script, since
>> JEP 330, we can now conveniently implement a similar script in Java. That'll
>> also automatically take care of OS specifics.
>
> Sure, but the script is here, it works, and the work is almost done.
Thanks for the pointers @tstuefe @erikj79. I updated the script to use `{0,1}`
instead.
-
PR Comment: https://git.openjdk.org/jdk/pull/19605#issuecomment-2160778973
Re: RFR: 8333685: Make update-copyright-year script more useful [v2]
> Hi all, > > This PR addresses [8333685](https://bugs.openjdk.org/browse/JDK-8333685). > > I have added the following enhancements: > - Removed uses of `fgrep` and `egrep` with `grep -F` and `grep -E` > respectively. > - Altered default behaviour to limit the processed changesets to those in the > current branch and the current year. > - Enabled an option to modify all changesets in the year if desired (this was > the previous default behaviour). > - Added named parameters and enabled `--help`. > - Removed mercurial support. > - Fixed a bug where copyright headers that didn't have a comma following the > initial year of creation were not getting picked up. For example, > [here](https://github.com/openjdk/jdk/blob/512b2b4f141f9a202984150b0427372e1a409a50/src/hotspot/cpu/zero/copy_zero.hpp#L3). > > - Fixed a bug where copyright headers missing the copyright symbol (c) were > not getting picked up. Refer to the example above as well. > > Thanks, > Sonia Sonia Zaldana Calles has updated the pull request incrementally with one additional commit since the last revision: Fixing optional group to work with bsd as well - Changes: - all: https://git.openjdk.org/jdk/pull/19605/files - new: https://git.openjdk.org/jdk/pull/19605/files/00ec9b5e..483c7fa9 Webrevs: - full: https://webrevs.openjdk.org/?repo=jdk=19605=01 - incr: https://webrevs.openjdk.org/?repo=jdk=19605=00-01 Stats: 3 lines in 1 file changed: 0 ins; 0 del; 3 mod Patch: https://git.openjdk.org/jdk/pull/19605.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/19605/head:pull/19605 PR: https://git.openjdk.org/jdk/pull/19605
Re: RFR: 8333685: Make update-copyright-year script more useful
On Tue, 11 Jun 2024 13:12:48 GMT, Pavel Rappo wrote: > Opinion: while it's good to see improvements to the existent script, since > JEP 330, we can now conveniently implement a similar script in Java. That'll > also automatically take care of OS specifics. Sure, but the script is here, it works, and the work is almost done. - PR Comment: https://git.openjdk.org/jdk/pull/19605#issuecomment-2160749823
Re: RFR: 8333685: Make update-copyright-year script more useful
On Fri, 7 Jun 2024 19:01:45 GMT, Sonia Zaldana Calles wrote: > Hi all, > > This PR addresses [8333685](https://bugs.openjdk.org/browse/JDK-8333685). > > I have added the following enhancements: > - Removed uses of `fgrep` and `egrep` with `grep -F` and `grep -E` > respectively. > - Altered default behaviour to limit the processed changesets to those in the > current branch and the current year. > - Enabled an option to modify all changesets in the year if desired (this was > the previous default behaviour). > - Added named parameters and enabled `--help`. > - Removed mercurial support. > - Fixed a bug where copyright headers that didn't have a comma following the > initial year of creation were not getting picked up. For example, > [here](https://github.com/openjdk/jdk/blob/512b2b4f141f9a202984150b0427372e1a409a50/src/hotspot/cpu/zero/copy_zero.hpp#L3). > > - Fixed a bug where copyright headers missing the copyright symbol (c) were > not getting picked up. Refer to the example above as well. > > Thanks, > Sonia Opinion: while it's good to see improvements to the existent script, since JEP 330, we can now conveniently implement a similar script in Java. That'll also automatically take care of OS specifics. - PR Comment: https://git.openjdk.org/jdk/pull/19605#issuecomment-2160734896
Re: RFR: 8333685: Make update-copyright-year script more useful
On Tue, 11 Jun 2024 12:32:59 GMT, Thomas Stuefe wrote:
> Tested on Linux, there it works.
>
> Note, IIRC, the original unpatched version also worked on MacOS. So, I think
> the original sed expressions were probably ok. Something about
> `(${copyright_symbol} )?` perhabs.
Mac has bsd sed by default which does not handle '?'. You would have to express
it as `{0,1}` instead to be compatible.
-
PR Comment: https://git.openjdk.org/jdk/pull/19605#issuecomment-2160712447
Re: RFR: 8333685: Make update-copyright-year script more useful
On Fri, 7 Jun 2024 19:01:45 GMT, Sonia Zaldana Calles
wrote:
> Hi all,
>
> This PR addresses [8333685](https://bugs.openjdk.org/browse/JDK-8333685).
>
> I have added the following enhancements:
> - Removed uses of `fgrep` and `egrep` with `grep -F` and `grep -E`
> respectively.
> - Altered default behaviour to limit the processed changesets to those in the
> current branch and the current year.
> - Enabled an option to modify all changesets in the year if desired (this was
> the previous default behaviour).
> - Added named parameters and enabled `--help`.
> - Removed mercurial support.
> - Fixed a bug where copyright headers that didn't have a comma following the
> initial year of creation were not getting picked up. For example,
> [here](https://github.com/openjdk/jdk/blob/512b2b4f141f9a202984150b0427372e1a409a50/src/hotspot/cpu/zero/copy_zero.hpp#L3).
>
> - Fixed a bug where copyright headers missing the copyright symbol (c) were
> not getting picked up. Refer to the example above as well.
>
> Thanks,
> Sonia
Tested on Linux, there it works.
Note, IIRC, the original unpatched version also worked on MacOS. So, I think
the original sed expressions were probably ok. Something about
`(${copyright_symbol} )?` perhabs.
-
PR Comment: https://git.openjdk.org/jdk/pull/19605#issuecomment-2160646983
Re: RFR: 8333685: Make update-copyright-year script more useful
On Fri, 7 Jun 2024 19:01:45 GMT, Sonia Zaldana Calles wrote: > Hi all, > > This PR addresses [8333685](https://bugs.openjdk.org/browse/JDK-8333685). > > I have added the following enhancements: > - Removed uses of `fgrep` and `egrep` with `grep -F` and `grep -E` > respectively. > - Altered default behaviour to limit the processed changesets to those in the > current branch and the current year. > - Enabled an option to modify all changesets in the year if desired (this was > the previous default behaviour). > - Added named parameters and enabled `--help`. > - Removed mercurial support. > - Fixed a bug where copyright headers that didn't have a comma following the > initial year of creation were not getting picked up. For example, > [here](https://github.com/openjdk/jdk/blob/512b2b4f141f9a202984150b0427372e1a409a50/src/hotspot/cpu/zero/copy_zero.hpp#L3). > > - Fixed a bug where copyright headers missing the copyright symbol (c) were > not getting picked up. Refer to the example above as well. > > Thanks, > Sonia Thanks for tackling this. When it will work, it will be very useful, but so far I did not get it to work on my Mac m1. It identifies the commits correctly that are part of the delta to master, as it should. But it does not modify any copyrights. I tracked this down to the sed's in updateFile not doing anything. I tried the script on this branch: https://github.com/tstuefe/jdk/tree/do-some-stuff-in-aix and called the script without command line args. It should have found and fixed the Oracle copyrights in osThread_aix.xxx For reference, this is how the expanded sed arguments look like: s@(Copyright ((c) )?[12][0-9][0-9][0-9],) [12][0-9][0-9][0-9], Oracle@\1 2024, Oracle@ s@(Copyright ((c) )?[12][0-9][0-9][0-9],) Oracle@\1 2024, Oracle@ s@(Copyright ((c) )?[12][0-9][0-9][0-9]) Oracle@\1, 2024, Oracle@ s@Copyright 2024, 2024, Oracle@Copyright 2024, Oracle@ s@Copyright (c) 2024, 2024, Oracle@Copyright (c) 2024, Oracle@ - PR Comment: https://git.openjdk.org/jdk/pull/19605#issuecomment-2160035105
RFR: 8333685: Make update-copyright-year script more useful
Hi all, This PR addresses [8333685](https://bugs.openjdk.org/browse/JDK-8333685). I have added the following enhancements: - Removed uses of `fgrep` and `egrep` with `grep -F` and `grep -E` respectively. - Altered default behaviour to limit the processed changesets to those in the current branch and the current year. - Enabled an option to modify all changesets in the year if desired (this was the previous default behaviour). - Added named parameters and enabled `--help`. - Removed mercurial support. - Fixed a bug where copyright headers that didn't have a comma following the initial year of creation were not getting picked up. For example, [here](https://github.com/openjdk/jdk/blob/512b2b4f141f9a202984150b0427372e1a409a50/src/hotspot/cpu/zero/copy_zero.hpp#L3). - Fixed a bug where copyright headers missing the copyright symbol (c) were not getting picked up. Refer to the example above as well. Thanks, Sonia - Commit messages: - Support for copyright statements missing symbol - 8333685: Make update-copyright-year script more useful Changes: https://git.openjdk.org/jdk/pull/19605/files Webrev: https://webrevs.openjdk.org/?repo=jdk=19605=00 Issue: https://bugs.openjdk.org/browse/JDK-8333685 Stats: 97 lines in 1 file changed: 38 ins; 26 del; 33 mod Patch: https://git.openjdk.org/jdk/pull/19605.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/19605/head:pull/19605 PR: https://git.openjdk.org/jdk/pull/19605
