GIT-BUILD-OPTIONS can override manual invocations

In 4638e8806e (Makefile: use common template for GIT-BUILD-OPTIONS, 2024-12-06), the project's Makefile changed how it writes the GIT-BUILD-OPTIONS script. Prior to 4638e8806e, the Makefile would write the file itself, but post-4638e8806e it fills out a template ("GIT-BUILD-OPTIONS.in") with the appropriate values.

This has an interesting side effect when running e.g. the t/perf or t/interop suites. If I do:

    make && make -C t/perf GIT_PERF_MAKE_OPTS='NO_EXPAT=1'

, then we will still try and build with the libexpat headers!

For example, I removed the libexpat headers from my system, and ran the above to get the following output:

    $ find /usr/include -name expat.h | wc -l
    0

    $ make && make -C t/perf GIT_PERF_MAKE_OPTS='NO_EXPAT=1'
    [...]
    http-push.c:28:10: fatal error: expat.h: No such file or directory
       28 | #include <expat.h>
          |          ^~~~~~~~~

This is AFAICT fallout from a change in 4638e8806e where instead of *not* writing e.g. GIT_PERF_MAKE_OPTS into the GIT-BUILD-OPTIONS file, we now write it with an empty value. So when we run 'make -C t/perf' with a non-empty GIT_PERF_MAKE_OPTS, t/perf/run will source GIT-BUILD-OPTIONS, and override the value of GIT_PERF_MAKE_OPTS we specified.

Interestingly, 4638e8806e works around a similar issue in test-lib.sh where it stores the value of $TEST_OUTPUT_DIRECTORY in a temporary variable, and restores it after sourcing GIT-BUILD-OPTIONS if $TEST_OUTPUT_DIRECTORY is still empty. I think even this is subtly broken if your $TEST_OUTPUT_DIRECTORY is set to different non-empty values between GIT-BUILD-OPTIONS and when test-lib.sh is executed.

I noticed this along with Elijah while merging v2.48.1 into GitHub's private fork since our CI suite runs the t/interop tests with a custom GIT_INTEROP_MAKE_OPTS.

We could partially fix this in the same way as we do for TEST_OUTPUT_DIRECTORY, but I think that this isn't quite correct, and it makes me uneasy knowing that there are other places we might face similar issues. AFAICT, 4638e8806e has the potential to disrupt scripts that use any of the following variables:

  - FSMONITOR_DAEMON_BACKEND
  - FSMONITOR_OS_SETTINGS
  - GIT_INTEROP_MAKE_OPTS
  - GIT_PERF_LARGE_REPO
  - GIT_PERF_MAKE_COMMAND
  - GIT_PERF_MAKE_OPTS
  - GIT_PERF_REPEAT_COUNT
  - GIT_PERF_REPO
  - GIT_TEST_CMP
  - GIT_TEST_CMP_USE_COPIED_CONTEXT
  - GIT_TEST_INDEX_VERSION
  - GIT_TEST_OPTS
  - GIT_TEST_PERL_FATAL_WARNINGS
  - GIT_TEST_UTF8_LOCALE
  - TEST_OUTPUT_DIRECTORY

So I think a more robust fix might look like only filling out those lines in the GIT-BUILD-OPTIONS template when they are non-empty, similar to the pre-4638e8806e behavior. Something like:

--- 8< ---