Ævar Arnfjörð Bjarmason [off-list ref] writes:
Refactor out the hard-to-read and maintain "ln || ln -s || cp"
pattern.
This was initially added in 3e073dc5611 (Makefile: always provide a
fallback when hardlinks fail, 2008-08-25), but since then it's become
a lot more complex as we've added:
* 3426e34fedd (Add NO_CROSS_DIRECTORY_HARDLINKS support to the
Makefile, 2009-05-11)
* NO_INSTALL_HARDLINKS in 70de5e65e8c (Makefile:
NO_INSTALL_HARDLINKS, 2012-05-02)
* INSTALL_SYMLINKS in ad874608d8c (Makefile: optionally symlink
libexec/git-core binaries to bin/git, 2018-03-13)
* SKIP_DASHED_BUILT_INS 179227d6e21 (Optionally skip linking/copying
the built-ins, 2020-09-21)
Each of those commits had to add a new special-case to this code,
resulting in quite an unmaintainable mess for adding any sort of new
option.
Let's use the newly introduced ln-or-cp.sh script instead, note that
we only sometimes pass the --no-cross-directory-hardlinks option, per
the previous behavior. The target of the "ln -s" is also another
special snowflake, but we're careful to carry that forward.
Nice. These explicit command-line options to the helper may be
harder to initially write and maintain than just exporting the
relevant $(MAKE) macros and using it from the helper, but once
it works correctly, it is much easier to see what is going on.
And obviously, if we want to fix the "I do not ever want to see any
symlinks", it is very clear that main_with_fallbacks is where the
change needs to go.
Raelly nice.
On Mon, Mar 29, 2021 at 03:24:33PM -0700, Junio C Hamano wrote:
Ævar Arnfjörð Bjarmason [off-list ref] writes:
quoted
Refactor out the hard-to-read and maintain "ln || ln -s || cp"
pattern.
This was initially added in 3e073dc5611 (Makefile: always provide a
fallback when hardlinks fail, 2008-08-25), but since then it's become
a lot more complex as we've added:
* 3426e34fedd (Add NO_CROSS_DIRECTORY_HARDLINKS support to the
Makefile, 2009-05-11)
* NO_INSTALL_HARDLINKS in 70de5e65e8c (Makefile:
NO_INSTALL_HARDLINKS, 2012-05-02)
* INSTALL_SYMLINKS in ad874608d8c (Makefile: optionally symlink
libexec/git-core binaries to bin/git, 2018-03-13)
* SKIP_DASHED_BUILT_INS 179227d6e21 (Optionally skip linking/copying
the built-ins, 2020-09-21)
Each of those commits had to add a new special-case to this code,
resulting in quite an unmaintainable mess for adding any sort of new
option.
Let's use the newly introduced ln-or-cp.sh script instead, note that
we only sometimes pass the --no-cross-directory-hardlinks option, per
the previous behavior. The target of the "ln -s" is also another
special snowflake, but we're careful to carry that forward.
Nice. These explicit command-line options to the helper may be
harder to initially write and maintain than just exporting the
relevant $(MAKE) macros and using it from the helper, but once
it works correctly, it is much easier to see what is going on.
Another option is to make "ln-or-cp" itself a target that "make" knows
how to build, and bake the values into its "built" version. Besides
making the invocations a bit shorter, it also means that the dependency
graph is more correct. If a rule invokes ln-or-cp, its behavior will
change if $NO_INSTALL_HARDLINKS changes, for example. Telling that to
make requires depending on a sentinel file in each such caller (like
GIT-BUILD-OPTIONS). Whereas we could do that once for the "ln-or-cp"
target, and then everyone who uses it just depends on it.
I had a series a long time ago that moved the whole Makefile in that
direction, but I got nervous that it was too disruptive and too
non-idiomatic to be worth pursuing. So I offer the alternative here
mostly as food for thought. It may not be a good direction (and we
already have good-enough solutions, like depending on
GIT-BUILD-OPTIONS).
-Peff