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
Integrated: 8332699: ubsan: jfrEventSetting.inline.hpp:31:43: runtime error: index 163 out of bounds for type 'jfrNativeEventSetting [162]'
On Mon, 10 Jun 2024 12:30:59 GMT, Matthias Baesken wrote: > When running hs :tier1 tests or jdk/jfr tests, with ubsan enabled (configure > flag --enable-ubsan), in a lot of jfr related tests like > compiler/intrinsics/klass/CastNullCheckDroppingsTest.jtr > serviceability/jvmti/RedefineClasses/RedefineSharedClassJFR.jtr > this oob error can be seen : > > /jdk/src/hotspot/share/jfr/recorder/jfrEventSetting.inline.hpp:31:43: runtime > error: index 163 out of bounds for type 'jfrNativeEventSetting [162]' > #0 0x7f6b75a8634b in JfrEventSetting::setting(JfrEventId) > /jdk/src/hotspot/share/jfr/recorder/jfrEventSetting.inline.hpp:31 > #1 0x7f6b75a8634b in JfrEventSetting::set_stacktrace(long, bool) > /jdk/src/hotspot/share/jfr/recorder/jfrEventSetting.cpp:47 > > Looks like the array in generated code is too small. > With > `jfrNativeEventSetting bits[NUMBER_OF_EVENTS];` > and > > static const int NUMBER_OF_EVENTS = 162; > static const int NUMBER_OF_RESERVED_EVENTS = 2; > > > Access at index 163 cannot work. > But it looks like there is always enough memory after the array so we do not > crash and it was not noticed before. This pull request has now been integrated. Changeset: abbf45b5 Author:Matthias Baesken URL: https://git.openjdk.org/jdk/commit/abbf45b57edf2f5bf9a3f2fa408f35a43ebe9bb9 Stats: 2 lines in 1 file changed: 0 ins; 0 del; 2 mod 8332699: ubsan: jfrEventSetting.inline.hpp:31:43: runtime error: index 163 out of bounds for type 'jfrNativeEventSetting [162]' Reviewed-by: jbechberger, stuefe - PR: https://git.openjdk.org/jdk/pull/19628
Re: RFR: 8332699: ubsan: jfrEventSetting.inline.hpp:31:43: runtime error: index 163 out of bounds for type 'jfrNativeEventSetting [162]'
On Mon, 10 Jun 2024 12:30:59 GMT, Matthias Baesken wrote: > When running hs :tier1 tests or jdk/jfr tests, with ubsan enabled (configure > flag --enable-ubsan), in a lot of jfr related tests like > compiler/intrinsics/klass/CastNullCheckDroppingsTest.jtr > serviceability/jvmti/RedefineClasses/RedefineSharedClassJFR.jtr > this oob error can be seen : > > /jdk/src/hotspot/share/jfr/recorder/jfrEventSetting.inline.hpp:31:43: runtime > error: index 163 out of bounds for type 'jfrNativeEventSetting [162]' > #0 0x7f6b75a8634b in JfrEventSetting::setting(JfrEventId) > /jdk/src/hotspot/share/jfr/recorder/jfrEventSetting.inline.hpp:31 > #1 0x7f6b75a8634b in JfrEventSetting::set_stacktrace(long, bool) > /jdk/src/hotspot/share/jfr/recorder/jfrEventSetting.cpp:47 > > Looks like the array in generated code is too small. > With > `jfrNativeEventSetting bits[NUMBER_OF_EVENTS];` > and > > static const int NUMBER_OF_EVENTS = 162; > static const int NUMBER_OF_RESERVED_EVENTS = 2; > > > Access at index 163 cannot work. > But it looks like there is always enough memory after the array so we do not > crash and it was not noticed before. Hi Thomas and Johannes, thanks for the reviews ! - PR Comment: https://git.openjdk.org/jdk/pull/19628#issuecomment-2162188598
Re: RFR: 8332699: ubsan: jfrEventSetting.inline.hpp:31:43: runtime error: index 163 out of bounds for type 'jfrNativeEventSetting [162]'
On Mon, 10 Jun 2024 12:30:59 GMT, Matthias Baesken wrote: > When running hs :tier1 tests or jdk/jfr tests, with ubsan enabled (configure > flag --enable-ubsan), in a lot of jfr related tests like > compiler/intrinsics/klass/CastNullCheckDroppingsTest.jtr > serviceability/jvmti/RedefineClasses/RedefineSharedClassJFR.jtr > this oob error can be seen : > > /jdk/src/hotspot/share/jfr/recorder/jfrEventSetting.inline.hpp:31:43: runtime > error: index 163 out of bounds for type 'jfrNativeEventSetting [162]' > #0 0x7f6b75a8634b in JfrEventSetting::setting(JfrEventId) > /jdk/src/hotspot/share/jfr/recorder/jfrEventSetting.inline.hpp:31 > #1 0x7f6b75a8634b in JfrEventSetting::set_stacktrace(long, bool) > /jdk/src/hotspot/share/jfr/recorder/jfrEventSetting.cpp:47 > > Looks like the array in generated code is too small. > With > `jfrNativeEventSetting bits[NUMBER_OF_EVENTS];` > and > > static const int NUMBER_OF_EVENTS = 162; > static const int NUMBER_OF_RESERVED_EVENTS = 2; > > > Access at index 163 cannot work. > But it looks like there is always enough memory after the array so we do not > crash and it was not noticed before. @egahlin thank you. Patch is okay then. - Marked as reviewed by stuefe (Reviewer). PR Review: https://git.openjdk.org/jdk/pull/19628#pullrequestreview-2112007936
Integrated: 8334036: Update JCov for class file version 68
On Tue, 11 Jun 2024 19:02:29 GMT, Alexandre Iline wrote: > Update JCov for class file version 68 This pull request has now been integrated. Changeset: bbd3b1d8 Author:Alexandre Iline URL: https://git.openjdk.org/jdk/commit/bbd3b1d812da997347fca4c06e22794285ab00d3 Stats: 1 line in 1 file changed: 0 ins; 0 del; 1 mod 8334036: Update JCov for class file version 68 Reviewed-by: alanb, erikj - PR: https://git.openjdk.org/jdk/pull/19665
Re: RFR: 8334036: Update JCov for class file version 68
On Tue, 11 Jun 2024 19:02:29 GMT, Alexandre Iline wrote: > Update JCov for class file version 68 Marked as reviewed by erikj (Reviewer). - PR Review: https://git.openjdk.org/jdk/pull/19665#pullrequestreview-2111324522
Re: RFR: 8334036: Update JCov for class file version 68
On Tue, 11 Jun 2024 19:02:29 GMT, Alexandre Iline wrote: > Update JCov for class file version 68 Marked as reviewed by alanb (Reviewer). - PR Review: https://git.openjdk.org/jdk/pull/19665#pullrequestreview-2111285609
RFR: 8334036: Update JCov for class file version 68
Update JCov for class file version 68 - Commit messages: - JDK-8334036: Update JCov for class file version 68 Changes: https://git.openjdk.org/jdk/pull/19665/files Webrev: https://webrevs.openjdk.org/?repo=jdk&pr=19665&range=00 Issue: https://bugs.openjdk.org/browse/JDK-8334036 Stats: 1 line in 1 file changed: 0 ins; 0 del; 1 mod Patch: https://git.openjdk.org/jdk/pull/19665.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/19665/head:pull/19665 PR: https://git.openjdk.org/jdk/pull/19665
Re: RFR: 8332699: ubsan: jfrEventSetting.inline.hpp:31:43: runtime error: index 163 out of bounds for type 'jfrNativeEventSetting [162]'
On Tue, 11 Jun 2024 16:55:39 GMT, Thomas Stuefe wrote: > About your fix, do you know why MetadataEvent and CheckPointEvent would not > count toward the number of events? In other words, why NUMBER_OF_EVENTS is > 162 if we have 164 events? Maybe the number of events is wrong? > > @egahlin ? The metadata and check point events are built-in and hold labels/descriptions and constants, so they can't be enabled/disabled and thus don't need event settings. - PR Comment: https://git.openjdk.org/jdk/pull/19628#issuecomment-2161391485
Re: RFR: 8332699: ubsan: jfrEventSetting.inline.hpp:31:43: runtime error: index 163 out of bounds for type 'jfrNativeEventSetting [162]'
On Mon, 10 Jun 2024 12:30:59 GMT, Matthias Baesken wrote: > When running hs :tier1 tests or jdk/jfr tests, with ubsan enabled (configure > flag --enable-ubsan), in a lot of jfr related tests like > compiler/intrinsics/klass/CastNullCheckDroppingsTest.jtr > serviceability/jvmti/RedefineClasses/RedefineSharedClassJFR.jtr > this oob error can be seen : > > /jdk/src/hotspot/share/jfr/recorder/jfrEventSetting.inline.hpp:31:43: runtime > error: index 163 out of bounds for type 'jfrNativeEventSetting [162]' > #0 0x7f6b75a8634b in JfrEventSetting::setting(JfrEventId) > /jdk/src/hotspot/share/jfr/recorder/jfrEventSetting.inline.hpp:31 > #1 0x7f6b75a8634b in JfrEventSetting::set_stacktrace(long, bool) > /jdk/src/hotspot/share/jfr/recorder/jfrEventSetting.cpp:47 > > Looks like the array in generated code is too small. > With > `jfrNativeEventSetting bits[NUMBER_OF_EVENTS];` > and > > static const int NUMBER_OF_EVENTS = 162; > static const int NUMBER_OF_RESERVED_EVENTS = 2; > > > Access at index 163 cannot work. > But it looks like there is always enough memory after the array so we do not > crash and it was not noticed before. About your fix, do you know why MetadataEvent and CheckPointEvent would not count toward the number of events? In other words, why NUMBER_OF_EVENTS is 162 if we have 164 events? Maybe the number of events is wrong? @egahlin ? - PR Comment: https://git.openjdk.org/jdk/pull/19628#issuecomment-2161218498
Re: RFR: 8332699: ubsan: jfrEventSetting.inline.hpp:31:43: runtime error: index 163 out of bounds for type 'jfrNativeEventSetting [162]'
On Tue, 11 Jun 2024 14:46:10 GMT, Matthias Baesken wrote:
> My maximum JfrEventId is 163 , see the generated
> hotspot/variant-server/gensrc/jfrfiles/jfrEventIds.hpp
>
> ```
>
> enum JfrEventId {
> JfrMetadataEvent = 0,
> JfrCheckpointEvent = 1,
> JfrDurationEvent = 2,
> JfrInstantEvent = 3,
> JfrValueEvent = 4,
> JfrTextEvent = 5,
> JfrZThreadDebugEvent = 6,
>.
> JfrJavaAgentEvent = 161,
> JfrNativeAgentEvent = 162,
> JfrDeprecatedInvocationEvent = 163,
> };
> ```
>
> so NUMBER_OF_EVENTS + NUMBER_OF_RESERVED_EVENTS looks fine to me. 163 is the
> highest I could see while testing.
No, I am quite sure the `ev` substructure is wrong. The structure contains
members that are not events, that's why its larger.
But it's an issue orthogonal to the one you are fixing. I opened
https://bugs.openjdk.org/browse/JDK-8334031 to track it.
-
PR Comment: https://git.openjdk.org/jdk/pull/19628#issuecomment-2161205376
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: 8332699: ubsan: jfrEventSetting.inline.hpp:31:43: runtime error: index 163 out of bounds for type 'jfrNativeEventSetting [162]'
On Mon, 10 Jun 2024 12:30:59 GMT, Matthias Baesken wrote:
> When running hs :tier1 tests or jdk/jfr tests, with ubsan enabled (configure
> flag --enable-ubsan), in a lot of jfr related tests like
> compiler/intrinsics/klass/CastNullCheckDroppingsTest.jtr
> serviceability/jvmti/RedefineClasses/RedefineSharedClassJFR.jtr
> this oob error can be seen :
>
> /jdk/src/hotspot/share/jfr/recorder/jfrEventSetting.inline.hpp:31:43: runtime
> error: index 163 out of bounds for type 'jfrNativeEventSetting [162]'
> #0 0x7f6b75a8634b in JfrEventSetting::setting(JfrEventId)
> /jdk/src/hotspot/share/jfr/recorder/jfrEventSetting.inline.hpp:31
> #1 0x7f6b75a8634b in JfrEventSetting::set_stacktrace(long, bool)
> /jdk/src/hotspot/share/jfr/recorder/jfrEventSetting.cpp:47
>
> Looks like the array in generated code is too small.
> With
> `jfrNativeEventSetting bits[NUMBER_OF_EVENTS];`
> and
>
> static const int NUMBER_OF_EVENTS = 162;
> static const int NUMBER_OF_RESERVED_EVENTS = 2;
>
>
> Access at index 163 cannot work.
> But it looks like there is always enough memory after the array so we do not
> crash and it was not noticed before.
My maximum JfrEventId is 163 , see the generated
hotspot/variant-server/gensrc/jfrfiles/jfrEventIds.hpp
enum JfrEventId {
JfrMetadataEvent = 0,
JfrCheckpointEvent = 1,
JfrDurationEvent = 2,
JfrInstantEvent = 3,
JfrValueEvent = 4,
JfrTextEvent = 5,
JfrZThreadDebugEvent = 6,
.
JfrJavaAgentEvent = 161,
JfrNativeAgentEvent = 162,
JfrDeprecatedInvocationEvent = 163,
};
so NUMBER_OF_EVENTS + NUMBER_OF_RESERVED_EVENTS looks fine to me.
163 is the highest I could see while testing.
-
PR Comment: https://git.openjdk.org/jdk/pull/19628#issuecomment-2160953294
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&pr=19605&range=01 - incr: https://webrevs.openjdk.org/?repo=jdk&pr=19605&range=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: 8332699: ubsan: jfrEventSetting.inline.hpp:31:43: runtime error: index 163 out of bounds for type 'jfrNativeEventSetting [162]'
On Mon, 10 Jun 2024 12:30:59 GMT, Matthias Baesken wrote: > When running hs :tier1 tests or jdk/jfr tests, with ubsan enabled (configure > flag --enable-ubsan), in a lot of jfr related tests like > compiler/intrinsics/klass/CastNullCheckDroppingsTest.jtr > serviceability/jvmti/RedefineClasses/RedefineSharedClassJFR.jtr > this oob error can be seen : > > /jdk/src/hotspot/share/jfr/recorder/jfrEventSetting.inline.hpp:31:43: runtime > error: index 163 out of bounds for type 'jfrNativeEventSetting [162]' > #0 0x7f6b75a8634b in JfrEventSetting::setting(JfrEventId) > /jdk/src/hotspot/share/jfr/recorder/jfrEventSetting.inline.hpp:31 > #1 0x7f6b75a8634b in JfrEventSetting::set_stacktrace(long, bool) > /jdk/src/hotspot/share/jfr/recorder/jfrEventSetting.cpp:47 > > Looks like the array in generated code is too small. > With > `jfrNativeEventSetting bits[NUMBER_OF_EVENTS];` > and > > static const int NUMBER_OF_EVENTS = 162; > static const int NUMBER_OF_RESERVED_EVENTS = 2; > > > Access at index 163 cannot work. > But it looks like there is always enough memory after the array so we do not > crash and it was not noticed before. Good catch! But I am not sure the solution is correct, or not enough. On my machine (linux x64), I see 169 explicitly named events in `JfrNativeSettings::ev`. But then, `NUMBER_OF_EVENTS = 162;` and `NUMBER_OF_RESERVED_EVENTS = 2;` . => 164. So, the last 5 events are not covered by the `bits` array even with your change. So, there is a mismatch somewhere between `metadata.getEventsAndStructs()` (used to print out individual event data) and `metadata.eventCounter.count` (used to determine NUMBER_OF_EVENTS). If you have figured this out, and fixed it, could you please let the generator add a STATIC_ASSERT( sizeof(JfrNativeSettings::ev) == sizeof(JfrNativeSettings::bits) ); to the header? - PR Comment: https://git.openjdk.org/jdk/pull/19628#issuecomment-2160739146
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
