| Message ID | patch-6.7-9ada8979890-20210329T162327Z-avarab@gmail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Makefile: make non-symlink & non-hardlink install sane | expand |
Ævar Arnfjörð Bjarmason <avarab@gmail.com> writes: > Change the default behavior on "make install" where we fallback > through a chain of "ln || ln -s || cp" to instead error out when we > can't symlink or hardlink, and not then fallback on a "cp" (or from a > symlink to hardlink etc.). > > The fallback behavior was introduced in 3e073dc5611 (Makefile: always > provide a fallback when hardlinks fail, 2008-08-25), since then we've > gained the ability to specify e.g. that we'd like symlinks via the > INSTALL_SYMLINKS setting. Hmph, I am not so sure. "Use hardlink if we can, as that would not consume inode, but where hardlinks cannot be used, it is OK to use symlink, and I do not want to waste disk blocks with cp" is probably one of the sensible wishes, but at least without "ln || ln -s" fallback, you cannot do that, no? So I would understand if there are two orthogonal knobs - the order of preference (e.g. hardlink > symlink > copy) - which ones are allowed (e.g. "no symlinks please") but I cannot quite imagine how a system without any fallback would be useful. > +main_no_fallbacks () { > + if test -n "$no_install_hardlinks" -a -z "$install_symlinks" As the values of these variables are (presumably) tightly under our control, the use of -a/-o with test may be safe in these examples, but to avoid letting clueless shell script newbies to cargo cult this code, let's use the safer "test -n A && test -z B" form.
On Tue, Mar 30 2021, Junio C Hamano wrote: > Ævar Arnfjörð Bjarmason <avarab@gmail.com> writes: > >> Change the default behavior on "make install" where we fallback >> through a chain of "ln || ln -s || cp" to instead error out when we >> can't symlink or hardlink, and not then fallback on a "cp" (or from a >> symlink to hardlink etc.). >> >> The fallback behavior was introduced in 3e073dc5611 (Makefile: always >> provide a fallback when hardlinks fail, 2008-08-25), since then we've >> gained the ability to specify e.g. that we'd like symlinks via the >> INSTALL_SYMLINKS setting. > > Hmph, I am not so sure. "Use hardlink if we can, as that would not > consume inode, but where hardlinks cannot be used, it is OK to use > symlink, and I do not want to waste disk blocks with cp" is probably > one of the sensible wishes, but at least without "ln || ln -s" fallback, > you cannot do that, no? > > So I would understand if there are two orthogonal knobs > > - the order of preference (e.g. hardlink > symlink > copy) > - which ones are allowed (e.g. "no symlinks please") > > but I cannot quite imagine how a system without any fallback would > be useful. Because with explicit knobs I'd like to tell it what to do and not have it auto-guess. E.g. if I say I want openssl I don't want it to see it's not there and auto-fallback on gnutls or whatever. The same for "I want hardlinks/symlinks", usually someone picking one is building a package, and under a lot of packaging formats that difference really matters, and either won't be notinced in time or will break further down the chain. >> +main_no_fallbacks () { >> + if test -n "$no_install_hardlinks" -a -z "$install_symlinks" > > As the values of these variables are (presumably) tightly under our > control, the use of -a/-o with test may be safe in these examples, > but to avoid letting clueless shell script newbies to cargo cult > this code, let's use the safer "test -n A && test -z B" form. *nod*
Ævar Arnfjörð Bjarmason <avarab@gmail.com> writes: >> So I would understand if there are two orthogonal knobs >> >> - the order of preference (e.g. hardlink > symlink > copy) >> - which ones are allowed (e.g. "no symlinks please") >> >> but I cannot quite imagine how a system without any fallback would >> be useful. > > Because with explicit knobs I'd like to tell it what to do and not have > it auto-guess. So how would I explicitly tell "I want hardlinks for everything else, but use cp when going between /usr/bin and /usr/libexec" (because /usr/bin and /usr/libexec is not on the same filesystem on this particular box---I'll tell you to use hardlink everywhere on another box of mine where they reside on the same filesystem)? Your argument or analogy with openssl does not make much sense to me in this case.
On Wed, Mar 31 2021, Junio C Hamano wrote: > Ævar Arnfjörð Bjarmason <avarab@gmail.com> writes: > >>> So I would understand if there are two orthogonal knobs >>> >>> - the order of preference (e.g. hardlink > symlink > copy) >>> - which ones are allowed (e.g. "no symlinks please") >>> >>> but I cannot quite imagine how a system without any fallback would >>> be useful. >> >> Because with explicit knobs I'd like to tell it what to do and not have >> it auto-guess. > > So how would I explicitly tell "I want hardlinks for everything > else, but use cp when going between /usr/bin and /usr/libexec" > (because /usr/bin and /usr/libexec is not on the same filesystem > on this particular box---I'll tell you to use hardlink everywhere > on another box of mine where they reside on the same filesystem)? Just as you would now: make NO_CROSS_DIRECTORY_HARDLINKS=Y install That'll get you hardlinks within bin/ and libexec, but copies between them, and no symlinks. The behavior change in this series is if your "ln" errors out we'll no longer silently plow ahead and e.g. not hardlink *within* bin and libexec in that case. > Your argument or analogy with openssl does not make much sense to me > in this case. The point is the Makefile shouldn't be second-guessing explicit requests. That's what defaults are for.
diff --git a/Makefile b/Makefile index 466df1a8e90..ccbded79093 100644 --- a/Makefile +++ b/Makefile @@ -350,6 +350,13 @@ all:: # or if that fails fall back on a "cp" instead of a "ln". Useful for # when you don't want hardlinks at all. # +# Define INSTALL_FALLBACK_LN_CP if you'd like your +# "INSTALL_SYMLINKS=Y" to fall back on hardlinks if we can't run "ln +# -s", and for "ln" to fall back on "NO_INSTALL_HARDLINKS=Y" if we +# can't perform a "ln" and need to fall-back to a "cp". This used to +# be the default behavior, but we'll now error if we can't satisfy +# your INSTALL_SYMLINKS, NO_INSTALL_HARDLINKS etc. settings. +# # Define SKIP_DASHED_BUILT_INS if you do not need the dashed versions of the # built-ins to be linked/copied at all. # @@ -3020,6 +3027,7 @@ endif { test "$$bindir/" = "$$execdir/" || \ for p in git$X $(filter $(install_bindir_programs),$(ALL_PROGRAMS)); do \ ./ln-or-cp.sh \ + --install-fallback-ln-cp "$(INSTALL_FALLBACK_LN_CP)" \ --install-symlinks "$(INSTALL_SYMLINKS)" \ --no-install-hardlinks "$(NO_INSTALL_HARDLINKS)" \ --no-cross-directory-hardlinks "$(NO_CROSS_DIRECTORY_HARDLINKS)" \ @@ -3029,6 +3037,7 @@ endif } && \ for p in $(filter $(install_bindir_programs),$(BUILT_INS)); do \ ./ln-or-cp.sh \ + --install-fallback-ln-cp "$(INSTALL_FALLBACK_LN_CP)" \ --install-symlinks "$(INSTALL_SYMLINKS)" \ --no-install-hardlinks "$(NO_INSTALL_HARDLINKS)" \ --symlink-target "git$X" \ @@ -3038,6 +3047,7 @@ endif if test -z "$(SKIP_DASHED_BUILT_INS)"; \ then \ ./ln-or-cp.sh \ + --install-fallback-ln-cp "$(INSTALL_FALLBACK_LN_CP)" \ --install-symlinks "$(INSTALL_SYMLINKS)" \ --no-install-hardlinks "$(NO_INSTALL_HARDLINKS)" \ --symlink-target "$$destdir_from_execdir_SQ/$(bindir_relative_SQ)/git$X" \ @@ -3047,6 +3057,7 @@ endif remote_curl_aliases="$(REMOTE_CURL_ALIASES)" && \ for p in $$remote_curl_aliases; do \ ./ln-or-cp.sh \ + --install-fallback-ln-cp "$(INSTALL_FALLBACK_LN_CP)" \ --install-symlinks "$(INSTALL_SYMLINKS)" \ --no-install-hardlinks "$(NO_INSTALL_HARDLINKS)" \ --symlink-target "git-remote-http$X" \ diff --git a/ln-or-cp.sh b/ln-or-cp.sh index 37380993c64..f77dad71bdb 100755 --- a/ln-or-cp.sh +++ b/ln-or-cp.sh @@ -1,5 +1,6 @@ #!/bin/sh +install_fallback_ln_cp= install_symlinks= no_install_hardlinks= no_cross_directory_hardlinks= @@ -7,6 +8,10 @@ symlink_target= while test $# != 0 do case "$1" in + --install-fallback-ln-cp) + install_fallback_ln_cp="$2" + shift + ;; --install-symlinks) install_symlinks="$2" shift @@ -61,4 +66,26 @@ main_with_fallbacks () { fi } -main_with_fallbacks +main_no_fallbacks () { + if test -n "$no_install_hardlinks" -a -z "$install_symlinks" + then + cp "$target" "$link" + elif test -n "$install_symlinks" -o -n "$no_cross_directory_hardlinks" + then + ln -f -s "$symlink_target" "$link" + elif test -n "$no_install_hardlinks" + then + cp "$target" "$link" + else + ln -f "$target" "$link" + fi +} + +if test -z "$install_fallback_ln_cp" +then + # The stricter mode, where we know what we want + main_no_fallbacks +else + main_with_fallbacks + +fi
Change the default behavior on "make install" where we fallback through a chain of "ln || ln -s || cp" to instead error out when we can't symlink or hardlink, and not then fallback on a "cp" (or from a symlink to hardlink etc.). The fallback behavior was introduced in 3e073dc5611 (Makefile: always provide a fallback when hardlinks fail, 2008-08-25), since then we've gained the ability to specify e.g. that we'd like symlinks via the INSTALL_SYMLINKS setting. It doesn't make sense as a default to silently fall back if we can't satisfy those settings. We should instead error out, at which point the developer/builder/sysadmin can set one or more of the relevant hardlink or symlink settings. This also changes the behavior for the build (not install!) to always use the new strict mode. This will theoretically break things for someone who can't make symlinks or hardlinks in their git checkout when building. That part of this change would break building on Windows and other platforms that don't support symlinks if INSTALL_SYMLINKS were to become the default, but it's not on by default, so we should be fine here. If and when INSTALL_SYMLINKS ever becomes the default the right way to deal with this would be to tweak config.mak.uname appropriately, not to silently fall back on a copy. Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com> --- Makefile | 11 +++++++++++ ln-or-cp.sh | 29 ++++++++++++++++++++++++++++- 2 files changed, 39 insertions(+), 1 deletion(-)