RFR: 8308961: Automatically find configuration matching name of checked of branch

[David Holmes]

On Mon, 29 May 2023 13:21:27 GMT, Erik Helin <ehelin at openjdk.org> wrote:

> Hi all,
> 
> please review this smaller patch to the build system. This patch changes what the build system will do when there are multiple configurations available and none has been selected (neither via `CONF` nor via `SPEC`). Instead of printing an error message (current behavior) the build system will instead check if the name of any configuration exactly matches the name of the checked out Git branch. If so, then that configuration will be built.
> 
> The rationale for this change is that many developers (including me) use branches to work on multiple things concurrently. Having (at least) one configuration per branch, named after the branch, is a very natural model then for setting up one's build configurations. Having the build configuration corresponding to the checked out Git branch built automatically is then very convenient (instead of typing `make CONF=$(git branch --show-current)` every time).
> 
> A couple of design considerations:
> - why use `$$(shell command -v git 2>/dev/null)` instead of `$$(GIT)` from autoconf? Because we don't have a `spec.gmk` available because we haven't chosen a configuration yet, so we don't have `$$(GIT)`.
> - why use `git rev-parse --abbrev-ref HEAD` instead of `git branch --show-current`? Because `git rev-parse --abbrev-ref` has been around since [2009](https://github.com/git/git/commit/a45d34691ea624e93863e95706eeb1b1909304f3) and `git branch --show-current` was introduced in Git 2.22 in 2019 (plus that `rev-parse --abbrev-ref` works with a detached `HEAD`). `git rev-parse --is-inside-work-tree` is from [2007](https://github.com/git/git/commit/892c41b98ae2e6baf3aa13901cb10db9ac67d2f3), but I think requiring a Git installation more recent than 2009 should be ok.
> - why match the branch name exactly instead of a looser matching? This could be beneficial if branches are named e.g. `$(git branch --show-current)-fastdebug`, but I wanted to start out with something strict and then the matching can be made looser if needed.
> - is this backwards compatible? Yes, at least I think so. Previously the build system would return an error but now it might build a configuration, that seems compatible. The patch does not interfere at all with the code paths where `CONF` or `SPEC` has been set.
> 
> ### Testing
> - [x] Tested locally on macOS/aarch64 and Linux/x64 with and without configurations matching the named Git branch.
> - [x] Tested locally on macOS/aarch64 and Linux/x64 without a Git client.
> - [x] Tested locally on macOS/aarch64 a...

I'm not sure how useful this is in general as it very much depends on your own style of working. I use branch-based configurations as well but I use a wrapper script to manage that - in part because the wrapper will also run configure if no configuration currently exists. The main limitation I see with the proposal (and I may be misreading it) is that the exact name match seems to preclude working when your configurations have the form `mybranch` and `mybranch-debug` etc.

make/InitSupport.gmk line 251:

> 249:         ifeq ($$(words $$(all_spec_files)), 1)
> 250:           # We found exactly one configuration, use it
> 251:           SPECS := $$(strip $$(all_spec_files))

It may be better to keep this as the first action and only do the git checks if necessary. That would minimise potential interference with single branch repos.

-------------

PR Review: https://git.openjdk.org/jdk/pull/14202#pullrequestreview-1449929051
PR Review Comment: https://git.openjdk.org/jdk/pull/14202#discussion_r1209607563