8320358: GHA: ignore jdk* branches

[kevinrushforth]

At some point we are likely to use stabilization branches in the mainline jdk repo rather than a separate repo. In preparation, this PR excludes branches matching jdk*, like we currently do for master and pr/*.

A potential drawback of doing this is that it will exclude developer branches named jdk-8888888 or similar, using a lower-case jdk. Developers who want a GHA run will need to use JDK (uppercase) or some other prefix.

This is unlikely to be a problem in practice. I checked the most recent 100 open pull requests in the jdk repo at the time I created this fix, and while many of them use "JDK" (upper case) as a prefix, I found none that use "jdk" (lower case).

Testing

I pushed the following branch that was even with jdk:master at the time I pushed it (thus without this fix). GHA was run as expected:

• jdk-8000000 : GHA run (NOTE: once this fix is integrated, such a branch would not get a GHA run)

I pushed the following branches that all include this fix. GHA runs were skipped on the branches that start with jdk and run on the others:

• JDK-8320358 : GHA run, (you can also see this from this PR's test results)
• gha-exclude-jdk : GHA run
• jdk12345 : No GHA run
• jdk-8320358-gha : No GHA run

---

Progress

• Change must be properly reviewed (1 review required, with at least 1 Reviewer)
• Change must not contain extraneous whitespace
• Commit message must refer to an issue

Issue

• JDK-8320358: GHA: ignore jdk* branches (Bug - P4)

Reviewers

• Magnus Ihse Bursie (@magicus - Reviewer)

Reviewing

Using git

Checkout this PR locally:
$ git fetch https://git.openjdk.org/jdk.git pull/16763/head:pull/16763
$ git checkout pull/16763

Update a local copy of the PR:
$ git checkout pull/16763
$ git pull https://git.openjdk.org/jdk.git pull/16763/head

Using Skara CLI tools

Checkout this PR locally:
$ git pr checkout 16763

View PR using the GUI difftool:
$ git pr show -t 16763

Using diff file

Download this PR as a diff file:
https://git.openjdk.org/jdk/pull/16763.diff

Webrev

Link to Webrev Comment

[bridgekeeper]

👋 Welcome back kcr! A progress list of the required criteria for merging this PR into master will be added to the body of your pull request. There are additional pull request commands available for use with this pull request.

[magicus]

Looks good to me.

[openjdk]

@kevinrushforth This change now passes all automated pre-integration checks.

ℹ️ This project also has non-automated pre-integration requirements. Please see the file CONTRIBUTING.md for details.

After integration, the commit message for the final commit will be:

8320358: GHA: ignore jdk* branches

Reviewed-by: ihse

You can use pull request commands such as /summary, /contributor and /issue to adjust it as needed.

At the time when this comment was updated there had been 114 new commits pushed to the master branch:

• 63ad868: 8319668: Fixup of jar filename typo in BadFactoryTest.sh
• 4bcda60: 8319713: Parallel: Remove PSAdaptiveSizePolicy::should_full_GC
• 99f870c: 8320781: Fix whitespace in j.l.Double and j.u.z.ZipInputStream @snippets
• a5ccd3b: 8267532: C2: Profile and prune untaken exception handlers
• 464dc3d: 8319633: runtime/posixSig/TestPosixSig.java intermittent timeouts on UNIX
• efc3922: 8319048: Monitor deflation unlink phase prolongs time to safepoint
• debf0ec: 8313355: javax/management/remote/mandatory/notif/ListenerScaleTest.java failed with "Exception: Failed: ratio=792.2791601423487"
• 20aae3c: 8320533: Adjust capstone integration for v6 changes
• 0678253: 8320602: Lock contention in SchemaDVFactory.getInstance()
• f1a24f6: 8318599: HttpURLConnection cache issues leading to crashes in JGSS w/ native GSS introduced by 8303809
• ... and 104 more: https://git.openjdk.org/jdk/compare/99570fbe76e05b463d31c3dd7f8f15f16971fb0a...master

As there are no conflicts, your changes will automatically be rebased on top of these commits when integrating. If you prefer to avoid this automatic rebasing, please check the documentation for the /integrate command for further details.

As you do not have Committer status in this project an existing Committer must agree to sponsor your change. Possible candidates are the reviewers of this PR (@magicus) but any other Committer may sponsor as well.

➡️ To flag this PR as ready for integration with the above commit message, type /integrate in a new comment. (Afterwards, your sponsor types /sponsor in a new comment to perform the integration).

[openjdk]

@kevinrushforth The following label will be automatically applied to this pull request:

• build

When this pull request is ready to be reviewed, an "RFR" email will be sent to the corresponding mailing list. If you would like to change these labels, use the /label pull request command.

[mlbridge]

Webrevs

• 00: Full (bc976d71)

[shipilev]

Looks to me it just asks for corner case problems. For example, bots regularly create backport branches like openjdk-bots:${username}-backport-${hash}, which means if anyone has a username starting from jdk (/me stares at general vicinity of @jdksjolen), then GHA would suddenly stop for them?

I think we can avoid this with more precise glob pattern, e.g. jdk[0-9]+*, which would match jdk21, jdk21u-dev, etc.

[magicus]

... or the bots should name the branches backport-${username}-${hash} instead ..?

I think it would be good to reserve the entire namespace beginning with jdk, to reserve for future use e.g. a jdk-next or whatever.

Also, we need to look at this where it is being used. I'm not sure that the on: push: branches-ignore: syntax allows for complex regexp syntax. Deeming from the example already present, it does not look like a regexp.

[shipilev]

... or the bots should name the branches backport-${username}-${hash} instead ..?

Yes, but the fact I can come up with an easy example where it breaks serves as signal there are likely less obvious broken cases :)

I think it would be good to reserve the entire namespace beginning with jdk, to reserve for future use e.g. a jdk-next or whatever.

True.

Also, we need to look at this where it is being used. I'm not sure that the on: push: branches-ignore: syntax allows for complex regexp syntax. Deeming from the example already present, it does not look like a regexp.

The docs imply it is a variant of glob, which means either jdk([0-9])+* or jdk+([0-9])* might work.

[shipilev]

Looking at pr/* exclusion, the alternative to reserve the jdk namespace would be to exclude jdk/*. This would unfortunately force stabilization branches to take somewhat awkward branch names like jdk/21 or something.

[magicus]

We could name the branches jdk-21 instead or jdk/21; I think that would make more sense. However, I still think it is better to be a bit more aggressive here in what we exclude, to open up for more possibilities for us in the future.

[kevinrushforth]

We are very likely to use the name jdkNN matching the current name of the stabilization repos, and matching the practice in other repos such as jdk21u and jfx, so jdk[0-9]* should work for that based on my reading of the GHA docs.

As Magnus says, we might want to leave the door open in the future for jdk-something-else, which is why I proposed the simpler jdk*. However, that's an interesting point about it excluding a username that begins with jdk given how the bot creates the branch name for the /backport command.

[kevinrushforth]

However, that's an interesting point about it excluding a username that begins with jdk given how the bot creates the branch name for the /backport command.

I note that this would be easy to fix by changing the bot to use backport-USERNAME-SHORTHASH instead of the current where USERNAME is first.

[magicus]

Argh, the syntax is something like a glob-meets-regexp mongrel.

*: Matches zero or more characters, but does not match the / character. For example, Octo* matches Octocat.
**: Matches zero or more of any character.
?: Matches zero or one of the preceding character.
+: Matches one or more of the preceding character.
[] Matches one character listed in the brackets or included in ranges. Ranges can only include a-z, A-Z, and 0-9. For example, the range[0-9a-z] matches any digit or lowercase letter. F

That means that jdk[0-9-]+ can not work, for including also jdk-. On the other hand, we can probably have two rules, one for jdk[0-9]** and one for jdk-**. But then it maybe does not make sense to "reserve" it even if it is not planned to be used.

Also note that we probably want to use ** instead of *, since the latter does not match all characters. (The person coming up with this syntax must have been smoking something peculiar...)

[magicus]

I note that this would be easy to fix by changing the bot to use backport-USERNAME-SHORTHASH instead of the current where USERNAME is first.

It also have the benefit of being more logical. :-)

[magicus]

Yes, but the fact I can come up with an easy example where it breaks serves as signal there are likely less obvious broken cases :)

I mean, absolutely there can be other things, unknown to us, that will break. Just as adding the pr/* could have caused breakage. But this is the case for all code changes, especially for stuff like this that can run in diverse environment.

I don't think that should stop us, however. The better the reason to reserve the entire jdk name space already, instead of finding out later on that we need to redo the entire thing just because someone decided that jdk30 is actually going to be called "Java3"... (You're laughing? Remember Java2 ...)

However, we might want to document somewhere that branches with a name that starts with jdk or pr/ will not be tested by GHA.

[kevinrushforth]

I don't think we need to match a /, so I recommend either:

1. A single rule excluding jdk*, as in the current PR
2. A single rule excluding jdk[0-9]*
3. Two rules, one excluding jdk[0-9]* and a second excluding jdk-*

My preference is 1.

[kevinrushforth]

I could live with option 3 as well, but I think that would just be harder to explain without much benefit.

Let's leave this sit until after the US Thanksgiving holiday weekend, so others can weigh in.

[erikj79]

I don't think we need to match a /, so I recommend either:

1. A single rule excluding jdk*, as in the current PR
2. A single rule excluding jdk[0-9]*
3. Two rules, one excluding jdk[0-9]* and a second excluding jdk-*

My preference is 1.

I vote for 1. I will file a SKARA issue for reordering the backport branch names. We currently aren't backporting to mainline so it shouldn't cause any issues short term.

[erikj79]

I don't think we need to match a /, so I recommend either:

1. A single rule excluding jdk*, as in the current PR
2. A single rule excluding jdk[0-9]*
3. Two rules, one excluding jdk[0-9]* and a second excluding jdk-*

My preference is 1.

I vote for 1. I will file a SKARA issue for reordering the backport branch names. We currently aren't backporting to mainline so it shouldn't cause any issues short term.

Filed https://bugs.openjdk.org/browse/SKARA-2107.

[intrigus-lgtm]

May I ask, why you all are so fixated on using branches-ignore:?
Can you not simply add another job, that ignore pushes if the branch name has a specific format and if the repository is "openjdk/jdk"?
Then use the output of this job in the if: condition of the select job?

[kevinrushforth]

May I ask, why you all are so fixated on using branches-ignore:? Can you not simply add another job, that ignore pushes if the branch name has a specific format and if the repository is "openjdk/jdk"?

I think you misunderstand how our GitHub Actions are configured. No GHA job ever runs in the openjdk/jdk repo. Instead, a GHA job is run every time a commit is pushed to a branch in your personal fork of that repo. The purpose of the branch ignore statements are to avoid running them for branches that are synced from the upstream openjdk/jdk repo. Using branches-ignore is the most straight-forward and efficient way of doing this.

I plan to integrate this PR later today.

[kevinrushforth]

/integrate

[openjdk]

@kevinrushforth
Your change (at version bc976d7) is now ready to be sponsored by a Committer.

[erikj79]

/sponsor

[openjdk]

Going to push as commit db7fedf.
Since your change was applied there have been 116 commits pushed to the master branch:

• e33b6c1: 8319437: NMT should show library names in call stacks
• 2fae07f: 8319311: JShell Process Builder should be configurable
• 63ad868: 8319668: Fixup of jar filename typo in BadFactoryTest.sh
• 4bcda60: 8319713: Parallel: Remove PSAdaptiveSizePolicy::should_full_GC
• 99f870c: 8320781: Fix whitespace in j.l.Double and j.u.z.ZipInputStream @snippets
• a5ccd3b: 8267532: C2: Profile and prune untaken exception handlers
• 464dc3d: 8319633: runtime/posixSig/TestPosixSig.java intermittent timeouts on UNIX
• efc3922: 8319048: Monitor deflation unlink phase prolongs time to safepoint
• debf0ec: 8313355: javax/management/remote/mandatory/notif/ListenerScaleTest.java failed with "Exception: Failed: ratio=792.2791601423487"
• 20aae3c: 8320533: Adjust capstone integration for v6 changes
• ... and 106 more: https://git.openjdk.org/jdk/compare/99570fbe76e05b463d31c3dd7f8f15f16971fb0a...master

Your commit was automatically rebased without conflicts.

[openjdk]

@erikj79 @kevinrushforth Pushed as commit db7fedf.

💡 You may see a message that your pull request was closed with unmerged commits. This can be safely ignored.