On Tue, 30 May 2023 00:38:50 GMT, David Holmes <dhol...@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
