RFR: 8308961: Automatically find configuration matching name of checked of branch [v2]
Erik Helin
ehelin at openjdk.org
Tue May 30 13:49:56 UTC 2023
On Tue, 30 May 2023 00:38:50 GMT, David Holmes <dholmes at openjdk.org> wrote:
>> Erik Helin has updated the pull request incrementally with one additional commit since the last revision:
>>
>> Fuzzier matching
>
> 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.
That is a good idea, and I thought about it, but it is hard to structure the logic that way and not end up with duplicated code (early return doesn't seem to be a thing in GNU Make macros). What I means that the following is (at least to me) hard to express in a GNU Make macro:
ifeq ($$(words $$(all_spec_files)), 1)
# Somehow set SPECS and return, do not continue to execute
endif
GIT := ...
ifneq ($$(GIT),)
...
endif
...
The two Git commands potentially being executed are `O(1)`, they do not scale with repository size and should have minimal overhead. Since both flags to `git rev-parse` being used have been around since 2009 I think the risk of encountering a Git installation where they do not exist or do not work is very low. The `rev-parse` command is a so called "plumbing" command in Git which means that the Git project considers the CLI interface an API and keep it backwards compatible (so we shouldn't run in to future Git releases renaming and/or removing these flags). I therefore think it is worth keeping the logic a bit nicer and instead run these two extra `git rev-parse` commands and one extra `command -v` invocation (sometimes a shell built-in) for the single branch repo case.
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/14202#discussion_r1210301940
More information about the build-dev
mailing list