{"thread":{"id":"55269","subject":"[PATCH] Makefile: generate 'git' as 'cc [...] -o git+ && mv git+ git'","startedAt":"2021-03-07T13:21:07Z","lastAt":"2021-03-31T22:29:55Z","messageCount":48,"participants":["Ævar Arnfjörð Bjarmason","Junio C Hamano","Jeff King","Johannes Schindelin"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"418415","messageId":"20210307132001.7485-1-avarab@gmail.com","threadId":"55269","inReplyTo":null,"subject":"[PATCH] Makefile: generate 'git' as 'cc [...] -o git+ && mv git+ git'","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2021-03-07T13:20:01Z","receivedAt":"2021-03-07T13:21:07Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"Change the compilation of the main 'git' binary to not have the CC\nclobber 'git' in-place. This means that e.g. running the test suite\nin-place and recompiling won't fail whatever tests happen to be\nrunning for the duration of the binary being regenerated.\n\nThis is not a complete solution, in particular I'm not doing this for\nany of the git-* binaries, so it's effectively only for\nSKIP_DASHED_BUILT_INS=Y.\n\nI think that's fine for a small best-effort solution to this, even\nthose who rely on dashed binaries for something are likely mostly\nusing 'git' for whatever scripts they have running out of their\ngit.git checkout.\n\nSigned-off-by: Ævar Arnfjörð Bjarmason \n---\n Makefile | 5 +++--\n 1 file changed, 3 insertions(+), 2 deletions(-)\n\ndiff --git a/Makefile b/Makefile\nindex dfb0f1000fa..c5c754a9a12 100644\n--- a/Makefile\n+++ b/Makefile\n@@ -2159,8 +2159,9 @@ git.sp git.s git.o: EXTRA_CPPFLAGS = \\\n \t'-DGIT_INFO_PATH=\"$(infodir_relative_SQ)\"'\n \n git$X: git.o GIT-LDFLAGS $(BUILTIN_OBJS) $(GITLIBS)\n-\t$(QUIET_LINK)$(CC) $(ALL_CFLAGS) -o $@ $(ALL_LDFLAGS) \\\n-\t\t$(filter %.o,$^) $(LIBS)\n+\t$(QUIET_LINK)$(CC) $(ALL_CFLAGS) -o $@+ $(ALL_LDFLAGS) \\\n+\t\t$(filter %.o,$^) $(LIBS) && \\\n+\tmv $@+ $@\n \n help.sp help.s help.o: command-list.h\n \n-- \n2.31.0.rc0.126.g04f22c5b82\n\n"},{"id":"418422","messageId":"xmqq35x68zob.fsf@gitster.c.googlers.com","threadId":"55269","inReplyTo":"20210307132001.7485-1-avarab@gmail.com","subject":"Re: [PATCH] Makefile: generate 'git' as 'cc [...] -o git+ && mv git+ git'","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2021-03-07T20:41:08Z","receivedAt":"2021-03-07T20:41:57Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Ævar Arnfjörð Bjarmason writes:\n\n> Change the compilation of the main 'git' binary to not have the CC\n> clobber 'git' in-place. This means that e.g. running the test suite\n> in-place and recompiling won't fail whatever tests happen to be\n> running for the duration of the binary being regenerated.\n\nI am not sure why I would want to support the workflow this is\ntrying to help.\n\nI do not want to see a patch on this list claiming that \"While the\nwhole test suite is running, I tweaked the source three times and\nrecompiled, so some tests may have used my second iteration while\nothers may have used my third and the final version of the code---in\nany case, all tests passed\". And when a patch that was developed\nthat way came in, I do not want to hear \"Yes, the test suite used\nmixed binaries, but I KNOW the difference between my second and\nthird iteration should not matter the parts of the earlier tests\nthat used the older iteration\".\n\n"},{"id":"418435","messageId":"87ft15kegn.fsf@evledraar.gmail.com","threadId":"55269","inReplyTo":"xmqq35x68zob.fsf@gitster.c.googlers.com","subject":"Re: [PATCH] Makefile: generate 'git' as 'cc [...] -o git+ && mv git+ git'","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2021-03-08T12:38:32Z","receivedAt":"2021-03-08T12:39:33Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"\nOn Sun, Mar 07 2021, Junio C Hamano wrote:\n\n> Ævar Arnfjörð Bjarmason writes:\n>\n>> Change the compilation of the main 'git' binary to not have the CC\n>> clobber 'git' in-place. This means that e.g. running the test suite\n>> in-place and recompiling won't fail whatever tests happen to be\n>> running for the duration of the binary being regenerated.\n>\n> I am not sure why I would want to support the workflow this is\n> trying to help.\n\nBecause it also allows me and others to do more testing on patches to\ngit.git.\n\nIf I'm working on a patch to e.g. git-fsck I might be doing\nedit->save->some-tests, where \"some-tests\" are a subset of the test\nsuite I think is relevant to fsck.\n\nBut after doing N commits with passing tests I might notice that some\nother part of the test suite I didn't expect to have anything to do with\nfsck broke because I wasn't running that test.\n\nI wasn't running that test because I'm not going to wait 10-15 minutes\nfor it to run while actively editing, but will wait 30s-1m for 10-50\ntest files to run.\n\nSo I can also have the full test suite running in a loop in some side\nwindow that'll give me a headsup if the \"while do-full-tests; [...]\"\nloop breaks, at which point I'm likely to investigate it sooner than\notherwise, and not waste time going down the wrong path.\n\nYou can of course do that now, but it requires having a worktree on the\nside or whatever, which isn't always ideal (sometimes I'd like to have\nthese tests on uncommitted changes).\n\nThis change makes it mostly just work.\n\n\n> I do not want to see a patch on this list claiming that \"While the\n> whole test suite is running, I tweaked the source three times and\n> recompiled, so some tests may have used my second iteration while\n> others may have used my third and the final version of the code---in\n> any case, all tests passed\". And when a patch that was developed\n> that way came in, I do not want to hear \"Yes, the test suite used\n> mixed binaries, but I KNOW the difference between my second and\n> third iteration should not matter the parts of the earlier tests\n> that used the older iteration\".\n\nWe have a lot of existing rules in the Makefile that are of the form:\n\n make thing >thing+ &&\n mv thing+ thing\n\nWhere we're not doing the rename dance to avoid clobbering the file\nwe're reading.\n\nAs far as I can see all/most of those rules can just be rewritten like\ne.g. this if this is a use-case we not only don't care about, but would\nlike to go out of our way to break:\n\n diff --git a/Makefile b/Makefile\n index dfb0f1000fa..8b57c3a5e63 100644\n --- a/Makefile\n +++ b/Makefile\n @@ -2190,2 +2190 @@ config-list.h:\n - $(QUIET_GEN)$(SHELL_PATH) ./generate-configlist.sh \\\n - >$@+ && mv $@+ $@\n + $(QUIET_GEN)$(SHELL_PATH) ./generate-configlist.sh >$@\n"},{"id":"418483","messageId":"xmqqpn097e9o.fsf@gitster.c.googlers.com","threadId":"55269","inReplyTo":"87ft15kegn.fsf@evledraar.gmail.com","subject":"Re: [PATCH] Makefile: generate 'git' as 'cc [...] -o git+ && mv git+ git'","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2021-03-08T17:21:07Z","receivedAt":"2021-03-08T17:21:45Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Ævar Arnfjörð Bjarmason writes:\n\n> We have a lot of existing rules in the Makefile that are of the form:\n>\n> make thing >thing+ &&\n> mv thing+ thing\n>\n> Where we're not doing the rename dance to avoid clobbering the file\n> we're reading.\n\nI am afraid that you are totally misreading the intent of that age\nold convention, which is spelled:\n\n\tthing:\n\t\trm -f thing thing+\n\t\tprepare contents for thing >thing+\n\t\tmv thing+ thing\n\nIt protects us from a failure mode where \"prepare contents for\nthing\" step is broken and leaves a \"thing\" that does not work, but\nconfuses make that make does not need to rebuild it, if you wrote it\nas such:\n\n\tthing:\n\t\tprepare contents for thing >thing\n\nI think more recent make actually has some knob you can tweak to\ntell \"if the rule failed, remove it\", but the convention predates\nit.\n\nIn any case, it is not \"we are trying to make thing available while\nit is being rewritten\" at all.\n"},{"id":"418490","messageId":"YEZsON5OxUiDkqPG@coredump.intra.peff.net","threadId":"55269","inReplyTo":"87ft15kegn.fsf@evledraar.gmail.com","subject":"Re: [PATCH] Makefile: generate 'git' as 'cc [...] -o git+ && mv git+ git'","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2021-03-08T18:26:00Z","receivedAt":"2021-03-08T18:26:47Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Mon, Mar 08, 2021 at 01:38:32PM +0100, Ævar Arnfjörð Bjarmason wrote:\n\n> >> Change the compilation of the main 'git' binary to not have the CC\n> >> clobber 'git' in-place. This means that e.g. running the test suite\n> >> in-place and recompiling won't fail whatever tests happen to be\n> >> running for the duration of the binary being regenerated.\n> >\n> > I am not sure why I would want to support the workflow this is\n> > trying to help.\n> \n> Because it also allows me and others to do more testing on patches to\n> git.git.\n> \n> If I'm working on a patch to e.g. git-fsck I might be doing\n> edit->save->some-tests, where \"some-tests\" are a subset of the test\n> suite I think is relevant to fsck.\n> \n> But after doing N commits with passing tests I might notice that some\n> other part of the test suite I didn't expect to have anything to do with\n> fsck broke because I wasn't running that test.\n> \n> I wasn't running that test because I'm not going to wait 10-15 minutes\n> for it to run while actively editing, but will wait 30s-1m for 10-50\n> test files to run.\n> \n> So I can also have the full test suite running in a loop in some side\n> window that'll give me a headsup if the \"while do-full-tests; [...]\"\n> loop breaks, at which point I'm likely to investigate it sooner than\n> otherwise, and not waste time going down the wrong path.\n> \n> You can of course do that now, but it requires having a worktree on the\n> side or whatever, which isn't always ideal (sometimes I'd like to have\n> these tests on uncommitted changes).\n\nI don't always use it, but I have a \"ci\" script[1] that just runs the\ntest on each individual commit in a loop. The interesting things about\nit (beyond a simple loop) are:\n\n - it operates in a worktree (that copies the config.mak from the main\n worktree if necessary).\n\n - it uses Michael Haggerty's git-test[2] to memoize results for a given\n tree. That makes it reasonable to leave running in the background,\n where it will only use CPU when there's something useful to do.\n I also use git-test for \"git rebase -x\", so a final \"is each commit\n OK\" check usually runs instantly, because the results are cached.\n\n - it uses inotifywait to decide when HEAD has been updated. This is\n mostly a fun hack. It could also just poll every 10 seconds or\n whatever.\n\n - it triggers a custom command when the tests fail. I can share my\n sad-trombone.wav with you if you need. ;)\n\nYour mention of 10-15 minutes makes me wonder why your system is so\nslow, though. I generally run the whole suite (minus cvs/svn/p4 bits) in\nunder a minute. I know it's _much_ slower on Windows, but I didn't think\nthat was your platform.\n\n(In general, I'm mildly negative on your patch here. I have definitely\nrun into this myself, but I think having the test suite loudly complain\nis a good way to remind you that you have not in fact run the whole\nsuite on a given build).\n\n[1] https://github.com/peff/git/blob/meta/ci\n[2] https://github.com/mhagger/git-test\n\n-Peff\n"},{"id":"420483","messageId":"cover-0.6-00000000000-20210329T161723Z-avarab@gmail.com","threadId":"55269","inReplyTo":"20210307132001.7485-1-avarab@gmail.com","subject":"[PATCH v2 0/5] Makefile: don't die on AIX with open ./git","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2021-03-29T16:20:07Z","receivedAt":"2021-03-29T16:21:21Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"A v2, much the same, but moreso. But now with entirely different\nreasons for existing.\n\nÆvar Arnfjörð Bjarmason (5):\n Makefile: rename objects in-place, don't clobber\n Makefile: rename scripts in-place, don't clobber\n Makefile: don't needlessly \"rm $@ $@+\" before \"mv $@+ $@\"\n Makefile: add the \".DELETE_ON_ERROR\" flag\n Makefile: don't \"rm configure\" before generating it\n\n Makefile | 102 +++++++++++++++++++++++++++++++++----------------------\n 1 file changed, 61 insertions(+), 41 deletions(-)\n\n-- \n2.31.1.461.gd47399f6574\n\n"},{"id":"420484","messageId":"patch-1.6-3330cdbccc0-20210329T161723Z-avarab@gmail.com","threadId":"55269","inReplyTo":"cover-0.6-00000000000-20210329T161723Z-avarab@gmail.com","subject":"[PATCH v2 1/5] Makefile: rename objects in-place, don't clobber","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2021-03-29T16:20:08Z","receivedAt":"2021-03-29T16:21:21Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"Use the established \"[...] -o $@+ && mv $@+ $@\" pattern in the\nMakefile for those rules that don't use it already.\n\nThis improves portability on OS such as AIX that complain if object\nfiles in-use are clobbered (but not if they're rm'd, or renamed\naway). On e.g. AIX 7.2 running a \"./git log\" in one terminal and\ntrying to clobber the \"git\" object in another will yield:\n\n $ >git\n bash: git: Cannot open or remove a file containing a running program.\n\nAnd trying to recompile \"git\" will likewise fail:\n\n LINK git\n ld: 0711-851 SEVERE ERROR: Output file: git\n The file is in use and cannot be overwritten.\n\nIt's perfectly happy to have the file renamed or removed from under it\nthough:\n\n $ mv git git2\n $ >git2\n bash: git2: Cannot open or remove a file containing a running program.\n $ rm git2\n rm: Remove git2? y\n $ file git2\n file: 0653-900 cannot open git2.\n\nOn AIX the test suite is still a dumpster fire. I'm running into this\nbecause an orphaned \"git-daemon\" is sometimes left running, causing\nsubsequent compilation to fail without manual intervention. It's also\nannoying not to be able to run some ad-hoc command using the built git\nwithout holding up subsequent compilation until any such programs are\nstopped.\n\nThis pattern of not clobbering, but rather generating a \"$@+\" object\nto rename in-place to \"$@\" has been present in the Makefile since\n6a2e50f9dfd (git --version tells which version of git you have.,\n2005-09-07), it just hasn't been consistently used for all the rules\nthat generated objects.\n\nPer the log of changes to the Makfile and Junio's recent comment about\n[1] why that pattern got introduced it was for a different reason\nentirely, i.e. (\"[]\" edits are mine, for brevity):\n\n [T]hat age old convention [...] is spelled [as]:\n\n \tthing:\n \t\trm -f thing thing+\n \t\tprepare contents for thing >thing+\n \t\tmv thing+ thing\n\n It protects us from a failure mode where \"prepare contents for\n thing\" step is broken and leaves a \"thing\" that does not work, but\n confuses make that make does not need to rebuild it, if you wrote it\n as such:\n\n \tthing:\n \t\tprepare contents for thing >thing\n\n [It might leave behind a corrupt 'thing'.] In any case, it is not\n \"we are trying to make thing available while it is being\n rewritten\" at all.\n\nThat makes perfect sense for shellscripts, but as this change shows\nthere's other good reasons to use this age old convention that weren't\nconsidered at the time.\n\n1. http://lore.kernel.org/git/xmqqpn097e9o.fsf@gitster.c.googlers.com\n\nSigned-off-by: Ævar Arnfjörð Bjarmason \n---\n Makefile | 40 +++++++++++++++++++++++++---------------\n 1 file changed, 25 insertions(+), 15 deletions(-)\n\ndiff --git a/Makefile b/Makefile\nindex 55c8035fa80..b0dbf5888b7 100644\n--- a/Makefile\n+++ b/Makefile\n@@ -2166,8 +2166,9 @@ git.sp git.s git.o: EXTRA_CPPFLAGS = \\\n \t'-DGIT_INFO_PATH=\"$(infodir_relative_SQ)\"'\n \n git$X: git.o GIT-LDFLAGS $(BUILTIN_OBJS) $(GITLIBS)\n-\t$(QUIET_LINK)$(CC) $(ALL_CFLAGS) -o $@ $(ALL_LDFLAGS) \\\n-\t\t$(filter %.o,$^) $(LIBS)\n+\t$(QUIET_LINK)$(CC) $(ALL_CFLAGS) -o $@+ $(ALL_LDFLAGS) \\\n+\t\t$(filter %.o,$^) $(LIBS) && \\\n+\tmv $@+ $@\n \n help.sp help.s help.o: command-list.h\n \n@@ -2526,18 +2527,22 @@ compat/nedmalloc/nedmalloc.sp: SP_EXTRA_FLAGS += -Wno-non-pointer-null\n endif\n \n git-%$X: %.o GIT-LDFLAGS $(GITLIBS)\n-\t$(QUIET_LINK)$(CC) $(ALL_CFLAGS) -o $@ $(ALL_LDFLAGS) $(filter %.o,$^) $(LIBS)\n+\t$(QUIET_LINK)$(CC) $(ALL_CFLAGS) -o $@+ $(ALL_LDFLAGS) $(filter %.o,$^) $(LIBS) && \\\n+\tmv $@+ $@\n \n git-imap-send$X: imap-send.o $(IMAP_SEND_BUILDDEPS) GIT-LDFLAGS $(GITLIBS)\n-\t$(QUIET_LINK)$(CC) $(ALL_CFLAGS) -o $@ $(ALL_LDFLAGS) $(filter %.o,$^) \\\n-\t\t$(IMAP_SEND_LDFLAGS) $(LIBS)\n+\t$(QUIET_LINK)$(CC) $(ALL_CFLAGS) -o $@+ $(ALL_LDFLAGS) $(filter %.o,$^) \\\n+\t\t$(IMAP_SEND_LDFLAGS) $(LIBS) && \\\n+\tmv $@+ $@\n \n git-http-fetch$X: http.o http-walker.o http-fetch.o GIT-LDFLAGS $(GITLIBS)\n-\t$(QUIET_LINK)$(CC) $(ALL_CFLAGS) -o $@ $(ALL_LDFLAGS) $(filter %.o,$^) \\\n-\t\t$(CURL_LIBCURL) $(LIBS)\n+\t$(QUIET_LINK)$(CC) $(ALL_CFLAGS) -o $@+ $(ALL_LDFLAGS) $(filter %.o,$^) \\\n+\t\t$(CURL_LIBCURL) $(LIBS) && \\\n+\tmv $@+ $@\n git-http-push$X: http.o http-push.o GIT-LDFLAGS $(GITLIBS)\n-\t$(QUIET_LINK)$(CC) $(ALL_CFLAGS) -o $@ $(ALL_LDFLAGS) $(filter %.o,$^) \\\n-\t\t$(CURL_LIBCURL) $(EXPAT_LIBEXPAT) $(LIBS)\n+\t$(QUIET_LINK)$(CC) $(ALL_CFLAGS) -o $@+ $(ALL_LDFLAGS) $(filter %.o,$^) \\\n+\t\t$(CURL_LIBCURL) $(EXPAT_LIBEXPAT) $(LIBS) && \\\n+\tmv $@+ $@\n \n $(REMOTE_CURL_ALIASES): $(REMOTE_CURL_PRIMARY)\n \t$(QUIET_LNCP)$(RM) $@ && \\\n@@ -2546,14 +2551,17 @@ $(REMOTE_CURL_ALIASES): $(REMOTE_CURL_PRIMARY)\n \tcp $< $@\n \n $(REMOTE_CURL_PRIMARY): remote-curl.o http.o http-walker.o GIT-LDFLAGS $(GITLIBS)\n-\t$(QUIET_LINK)$(CC) $(ALL_CFLAGS) -o $@ $(ALL_LDFLAGS) $(filter %.o,$^) \\\n-\t\t$(CURL_LIBCURL) $(EXPAT_LIBEXPAT) $(LIBS)\n+\t$(QUIET_LINK)$(CC) $(ALL_CFLAGS) -o $@+ $(ALL_LDFLAGS) $(filter %.o,$^) \\\n+\t\t$(CURL_LIBCURL) $(EXPAT_LIBEXPAT) $(LIBS) && \\\n+\tmv $@+ $@\n \n $(LIB_FILE): $(LIB_OBJS)\n-\t$(QUIET_AR)$(RM) $@ && $(AR) $(ARFLAGS) $@ $^\n+\t$(QUIET_AR)$(AR) $(ARFLAGS) $@+ $^ && \\\n+\tmv $@+ $@\n \n $(XDIFF_LIB): $(XDIFF_OBJS)\n-\t$(QUIET_AR)$(RM) $@ && $(AR) $(ARFLAGS) $@ $^\n+\t$(QUIET_AR)$(AR) $(ARFLAGS) $@+ $^&& \\\n+\tmv $@+ $@\n \n export DEFAULT_EDITOR DEFAULT_PAGER\n \n@@ -2834,7 +2842,8 @@ perf: all\n t/helper/test-tool$X: $(patsubst %,t/helper/%,$(TEST_BUILTINS_OBJS))\n \n t/helper/test-%$X: t/helper/test-%.o GIT-LDFLAGS $(GITLIBS)\n-\t$(QUIET_LINK)$(CC) $(ALL_CFLAGS) -o $@ $(ALL_LDFLAGS) $(filter %.o,$^) $(filter %.a,$^) $(LIBS)\n+\t$(QUIET_LINK)$(CC) $(ALL_CFLAGS) -o $@+ $(ALL_LDFLAGS) $(filter %.o,$^) $(filter %.a,$^) $(LIBS) && \\\n+\tmv $@+ $@\n \n check-sha1:: t/helper/test-tool$X\n \tt/helper/test-sha1.sh\n@@ -3333,6 +3342,7 @@ FUZZ_CXXFLAGS ?= $(CFLAGS)\n \n $(FUZZ_PROGRAMS): all\n \t$(QUIET_LINK)$(CXX) $(FUZZ_CXXFLAGS) $(LIB_OBJS) $(BUILTIN_OBJS) \\\n-\t\t$(XDIFF_OBJS) $(EXTLIBS) git.o $@.o $(LIB_FUZZING_ENGINE) -o $@\n+\t\t$(XDIFF_OBJS) $(EXTLIBS) git.o $@.o $(LIB_FUZZING_ENGINE) -o $@+ && \\\n+\tmv $@+ $@\n \n fuzz-all: $(FUZZ_PROGRAMS)\n-- \n2.31.1.461.gd47399f6574\n\n"},{"id":"420485","messageId":"patch-4.6-2ff6359c273-20210329T161723Z-avarab@gmail.com","threadId":"55269","inReplyTo":"cover-0.6-00000000000-20210329T161723Z-avarab@gmail.com","subject":"[PATCH v2 4/5] Makefile: add the \".DELETE_ON_ERROR\" flag","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2021-03-29T16:20:11Z","receivedAt":"2021-03-29T16:21:21Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"Use the GNU make \".DELETE_ON_ERROR\" flag. As discussed in preceding\ncommits we're now less paranoid about \"mv\" failing, let's bring that\nparanoia back in a way that makes more sense, and applies to all rules\nin the Makefile.\n\nNow if a command to make X fails X will be removed, the default\nbehavior of GNU make is to only do so if \"make\" itself is interrupted\nwith a signal.\n\nE.g. if we now intentionally break one of the rules with:\n\n - mv $@+ $@\n + mv $@+ $@ && \\\n + false\n\nWe'll get output like:\n\n $ make git\n CC git.o\n LINK git\n make: *** [Makefile:2179: git] Error 1\n make: *** Deleting file 'git'\n $ file git\n git: cannot open `git' (No such file or directory)\n\nBefore this change we'd leave the file in place in under this\nscenario.\n\nSigned-off-by: Ævar Arnfjörð Bjarmason \n---\n Makefile | 10 ++++++++++\n 1 file changed, 10 insertions(+)\n\ndiff --git a/Makefile b/Makefile\nindex f08635067b3..573bce20357 100644\n--- a/Makefile\n+++ b/Makefile\n@@ -2126,6 +2126,16 @@ shell_compatibility_test: please_set_SHELL_PATH_to_a_more_modern_shell\n strip: $(PROGRAMS) git$X\n \t$(STRIP) $(STRIP_OPTS) $^\n \n+### Flags affecting all rules\n+\n+# A GNU make extension since gmake 3.72 (released in late 1994) to\n+# remove the target of rules if commands in those rules fail. The\n+# default is to only do that if make itself receives a signal. Affects\n+# all targets, see:\n+#\n+# info make --index-search=.DELETE_ON_ERROR\n+.DELETE_ON_ERROR:\n+\n ### Target-specific flags and dependencies\n \n # The generic compilation pattern rule and automatically\n-- \n2.31.1.461.gd47399f6574\n\n"},{"id":"420486","messageId":"patch-3.6-96e2338ed8e-20210329T161723Z-avarab@gmail.com","threadId":"55269","inReplyTo":"cover-0.6-00000000000-20210329T161723Z-avarab@gmail.com","subject":"[PATCH v2 3/5] Makefile: don't needlessly \"rm $@ $@+\" before \"mv $@+ $@\"","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2021-03-29T16:20:10Z","receivedAt":"2021-03-29T16:21:21Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"Now that preceding commits have moved the generation of objects and\nscripts in the Makefile to use the \"[...] -o $@+ && mv $@+ $@\" pattern\nwe can stop removing \"$@\" and \"$@+\" before the rule is run.\n\nI suppose that we could leave this at removing \"$@\" before we start\nout, per the \"age old convention\" comment in[1]. I.e. instead of:\n\n rm -f thing thing+\n prepare contents for thing >thing+\n mv thing+ thing\n\nDo:\n\n rm -f thing\n prepare contents for thing >thing+\n mv thing+ thing\n\nSince the removal of \"thing+\" is redundant as we're about to clobber\nit anyway, but we might be so paranoid as to be guarding against mv(1)\nleaving behind a corrupt \"thing\".\n\nBut I think guarding against \"mv\" failing is a step too far in\nparanoia, let's just rely on the \"[...] -o $@+ && mv $@+ $@\" pattern\nworking instead.\n\nEspecially as we'll see in a follow-up commit, we're just about to use\nthe GNU make \".DELETE_ON_ERROR\" feature, which will reliably do this\nfor us.\n\n1. http://lore.kernel.org/git/xmqqpn097e9o.fsf@gitster.c.googlers.com\n\nSigned-off-by: Ævar Arnfjörð Bjarmason \n---\n Makefile | 16 +++++-----------\n 1 file changed, 5 insertions(+), 11 deletions(-)\n\ndiff --git a/Makefile b/Makefile\nindex ef2d4a9973b..f08635067b3 100644\n--- a/Makefile\n+++ b/Makefile\n@@ -2210,7 +2210,6 @@ SCRIPT_DEFINES = $(SHELL_PATH_SQ):$(DIFF_SQ):$(GIT_VERSION):\\\n \t$(gitwebdir_SQ):$(PERL_PATH_SQ):$(SANE_TEXT_GREP):$(PAGER_ENV):\\\n \t$(perllibdir_SQ)\n define cmd_munge_script\n-$(RM) $@ $@+ && \\\n sed -e '1s|#!.*/sh|#!$(SHELL_PATH_SQ)|' \\\n -e 's|@SHELL_PATH@|$(SHELL_PATH_SQ)|' \\\n -e 's|@@DIFF@@|$(DIFF_SQ)|' \\\n@@ -2278,8 +2277,7 @@ endif\n PERL_DEFINES += $(gitexecdir) $(perllibdir) $(localedir)\n \n $(SCRIPT_PERL_GEN): % : %.perl GIT-PERL-DEFINES GIT-PERL-HEADER GIT-VERSION-FILE\n-\t$(QUIET_GEN)$(RM) $@ $@+ && \\\n-\tsed -e '1{' \\\n+\t$(QUIET_GEN)sed -e '1{' \\\n \t -e '\ts|#!.*perl|#!$(PERL_PATH_SQ)|' \\\n \t -e '\tr GIT-PERL-HEADER' \\\n \t -e '\tG' \\\n@@ -2299,8 +2297,7 @@ GIT-PERL-DEFINES: FORCE\n \t fi\n \n GIT-PERL-HEADER: $(PERL_HEADER_TEMPLATE) GIT-PERL-DEFINES Makefile\n-\t$(QUIET_GEN)$(RM) $@ && \\\n-\tINSTLIBDIR='$(perllibdir_SQ)' && \\\n+\t$(QUIET_GEN)INSTLIBDIR='$(perllibdir_SQ)' && \\\n \tINSTLIBDIR_EXTRA='$(PERLLIB_EXTRA_SQ)' && \\\n \tINSTLIBDIR=\"$$INSTLIBDIR$${INSTLIBDIR_EXTRA:+:$$INSTLIBDIR_EXTRA}\" && \\\n \tsed -e 's=@@PATHSEP@@=$(pathsep)=g' \\\n@@ -2325,8 +2322,7 @@ git-instaweb: git-instaweb.sh GIT-SCRIPT-DEFINES\n \tmv $@+ $@\n else # NO_PERL\n $(SCRIPT_PERL_GEN) git-instaweb: % : unimplemented.sh\n-\t$(QUIET_GEN)$(RM) $@ $@+ && \\\n-\tsed -e '1s|#!.*/sh|#!$(SHELL_PATH_SQ)|' \\\n+\t$(QUIET_GEN)sed -e '1s|#!.*/sh|#!$(SHELL_PATH_SQ)|' \\\n \t -e 's|@@REASON@@|NO_PERL=$(NO_PERL)|g' \\\n \t unimplemented.sh >$@+ && \\\n \tchmod +x $@+ && \\\n@@ -2339,15 +2335,13 @@ $(SCRIPT_PYTHON_GEN): GIT-BUILD-OPTIONS\n ifndef NO_PYTHON\n $(SCRIPT_PYTHON_GEN): GIT-CFLAGS GIT-PREFIX GIT-PYTHON-VARS\n $(SCRIPT_PYTHON_GEN): % : %.py\n-\t$(QUIET_GEN)$(RM) $@ $@+ && \\\n-\tsed -e '1s|#!.*python|#!$(PYTHON_PATH_SQ)|' \\\n+\t$(QUIET_GEN)sed -e '1s|#!.*python|#!$(PYTHON_PATH_SQ)|' \\\n \t $< >$@+ && \\\n \tchmod +x $@+ && \\\n \tmv $@+ $@\n else # NO_PYTHON\n $(SCRIPT_PYTHON_GEN): % : unimplemented.sh\n-\t$(QUIET_GEN)$(RM) $@ $@+ && \\\n-\tsed -e '1s|#!.*/sh|#!$(SHELL_PATH_SQ)|' \\\n+\t$(QUIET_GEN)sed -e '1s|#!.*/sh|#!$(SHELL_PATH_SQ)|' \\\n \t -e 's|@@REASON@@|NO_PYTHON=$(NO_PYTHON)|g' \\\n \t unimplemented.sh >$@+ && \\\n \tchmod +x $@+ && \\\n-- \n2.31.1.461.gd47399f6574\n\n"},{"id":"420487","messageId":"patch-2.6-49dbd54d0ff-20210329T161723Z-avarab@gmail.com","threadId":"55269","inReplyTo":"cover-0.6-00000000000-20210329T161723Z-avarab@gmail.com","subject":"[PATCH v2 2/5] Makefile: rename scripts in-place, don't clobber","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2021-03-29T16:20:09Z","receivedAt":"2021-03-29T16:21:21Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"Change the rules that generate auxiliary files such as\nGIT-BUILD-OPTIONS, *.s and other non-object files to use the same\nin-place move pattern as we use for object files as of the preceding\ncommit.\n\nUnlike the change in the preceding commit, this one isn't isn't needed\nfor AIX portability. I think it makes sense to do this for\nconsistency, we'll now use the same pattern for object and non-object\ngeneration.\n\nIt'll also guard against any weird issues with e.g. a command that\nwants to read GIT-PERL-DEFINES to generate one of the %.perl scripts\nracing with either a concurrent instance of \"make\" that has partially\nupdated the file, or test-lib.sh dying with some particularly weird\nerror because GIT-BUILD-OPTIONS was partway through being updated when\nit ran.\n\nSigned-off-by: Ævar Arnfjörð Bjarmason \n---\n Makefile | 33 ++++++++++++++++++++-------------\n 1 file changed, 20 insertions(+), 13 deletions(-)\n\ndiff --git a/Makefile b/Makefile\nindex b0dbf5888b7..ef2d4a9973b 100644\n--- a/Makefile\n+++ b/Makefile\n@@ -2246,7 +2246,8 @@ git.res: git.rc GIT-VERSION-FILE GIT-PREFIX\n \t$(QUIET_RC)$(RC) \\\n \t $(join -DMAJOR= -DMINOR= -DMICRO= -DPATCHLEVEL=, $(wordlist 1, 4, \\\n \t $(shell echo $(GIT_VERSION) 0 0 0 0 | tr '.a-zA-Z-' ' '))) \\\n-\t -DGIT_VERSION=\"\\\\\\\"$(GIT_VERSION)\\\\\\\"\" -i $< -o $@\n+\t -DGIT_VERSION=\"\\\\\\\"$(GIT_VERSION)\\\\\\\"\" -i $< -o $@+ && \\\n+\tmv $@+ $@\n \n # This makes sure we depend on the NO_PERL setting itself.\n $(SCRIPT_PERL_GEN): GIT-BUILD-OPTIONS\n@@ -2293,7 +2294,8 @@ GIT-PERL-DEFINES: FORCE\n \t@FLAGS='$(PERL_DEFINES)'; \\\n \t if test x\"$$FLAGS\" != x\"`cat $@ 2>/dev/null`\" ; then \\\n \t\techo >&2 \" * new perl-specific parameters\"; \\\n-\t\techo \"$$FLAGS\" >$@; \\\n+\t\techo \"$$FLAGS\" >$@+; \\\n+\t\tmv $@+ $@; \\\n \t fi\n \n GIT-PERL-HEADER: $(PERL_HEADER_TEMPLATE) GIT-PERL-DEFINES Makefile\n@@ -2455,7 +2457,8 @@ $(ASM_OBJ): %.o: %.S GIT-CFLAGS $(missing_dep_dirs) $(missing_compdb_dir)\n \t$(QUIET_CC)$(CC) -o $*.o -c $(dep_args) $(compdb_args) $(ALL_CFLAGS) $(EXTRA_CPPFLAGS) $<\n \n %.s: %.c GIT-CFLAGS FORCE\n-\t$(QUIET_CC)$(CC) -o $@ -S $(ALL_CFLAGS) $(EXTRA_CPPFLAGS) $<\n+\t$(QUIET_CC)$(CC) -o $@+ -S $(ALL_CFLAGS) $(EXTRA_CPPFLAGS) $< && \\\n+\tmv $@+ $@\n \n ifdef USE_COMPUTED_HEADER_DEPENDENCIES\n # Take advantage of gcc's on-the-fly dependency generation\n@@ -2478,9 +2481,8 @@ endif\n ifeq ($(GENERATE_COMPILATION_DATABASE),yes)\n all:: compile_commands.json\n compile_commands.json:\n-\t@$(RM) $@\n-\t$(QUIET_GEN)sed -e '1s/^/[/' -e '$$s/,$$/]/' $(compdb_dir)/*.o.json > $@+\n-\t@if test -s $@+; then mv $@+ $@; else $(RM) $@+; fi\n+\t$(QUIET_GEN)sed -e '1s/^/[/' -e '$$s/,$$/]/' $(compdb_dir)/*.o.json >$@+ && \\\n+\tmv $@+ $@\n endif\n \n exec-cmd.sp exec-cmd.s exec-cmd.o: GIT-PREFIX\n@@ -2673,11 +2675,13 @@ perl/build/lib/%.pm: perl/%.pm\n \t$(QUIET_GEN)mkdir -p $(dir $@) && \\\n \tsed -e 's|@@LOCALEDIR@@|$(perl_localedir_SQ)|g' \\\n \t -e 's|@@NO_PERL_CPAN_FALLBACKS@@|$(NO_PERL_CPAN_FALLBACKS_SQ)|g' \\\n-\t< $< > $@\n+\t< $< >$@+ && \\\n+\tmv $@+ $@\n \n perl/build/man/man3/Git.3pm: perl/Git.pm\n \t$(QUIET_GEN)mkdir -p $(dir $@) && \\\n-\tpod2man $< $@\n+\tpod2man $< $@+ && \\\n+\tmv $@+ $@\n \n FIND_SOURCE_FILES = ( \\\n \tgit ls-files \\\n@@ -2805,7 +2809,8 @@ GIT-PYTHON-VARS: FORCE\n \t@VARS='$(TRACK_PYTHON)'; \\\n \t if test x\"$$VARS\" != x\"`cat $@ 2>/dev/null`\" ; then \\\n \t\techo >&2 \" * new Python interpreter location\"; \\\n-\t\techo \"$$VARS\" >$@; \\\n+\t\techo \"$$VARS\" >$@+; \\\n+\t\tmv $@+ $@; \\\n fi\n endif\n \n@@ -2817,8 +2822,9 @@ bin-wrappers/%: wrap-for-bin.sh\n \t@mkdir -p bin-wrappers\n \t$(QUIET_GEN)sed -e '1s|#!.*/sh|#!$(SHELL_PATH_SQ)|' \\\n \t -e 's|@@BUILD_DIR@@|$(shell pwd)|' \\\n-\t -e 's|@@PROG@@|$(patsubst test-%,t/helper/test-%$(X),$(@F))$(patsubst git%,$(X),$(filter $(@F),$(BINDIR_PROGRAMS_NEED_X)))|' < $< > $@ && \\\n-\tchmod +x $@\n+\t -e 's|@@PROG@@|$(patsubst test-%,t/helper/test-%$(X),$(@F))$(patsubst git%,$(X),$(filter $(@F),$(BINDIR_PROGRAMS_NEED_X)))|' < $< >$@+ && \\\n+\tchmod +x $@+ && \\\n+\tmv $@+ $@\n \n # GNU make supports exporting all variables by \"export\" without parameters.\n # However, the environment gets quite big, and some programs have problems\n@@ -2866,8 +2872,9 @@ HCO = $(patsubst %.h,%.hco,$(CHK_HDRS))\n HCC = $(HCO:hco=hcc)\n \n %.hcc: %.h\n-\t@echo '#include \"git-compat-util.h\"' >$@\n-\t@echo '#include \"$<\"' >>$@\n+\t@echo '#include \"git-compat-util.h\"' >$@+ && \\\n+\t@echo '#include \"$<\"' >>$@+ && \\\n+\tmv $@+ $@\n \n $(HCO): %.hco: %.hcc FORCE\n \t$(QUIET_HDR)$(CC) $(ALL_CFLAGS) -o /dev/null -c -xc $<\n-- \n2.31.1.461.gd47399f6574\n\n"},{"id":"420488","messageId":"patch-5.6-b04c4d29c53-20210329T161723Z-avarab@gmail.com","threadId":"55269","inReplyTo":"cover-0.6-00000000000-20210329T161723Z-avarab@gmail.com","subject":"[PATCH v2 5/5] Makefile: don't \"rm configure\" before generating it","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2021-03-29T16:20:12Z","receivedAt":"2021-03-29T16:21:22Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"Remove a \"rm configure\" before we make a new \"configure\" file via a\n\"configure+\" temporary file. This was partially made redundant via the\nnew \".DELETE_ON_ERROR\" target.\n\n\"Partially\" because if we die partway we'll remove the \"configure\"\nnow, but as before we'll leave the \"configure.ac+\" in place. Ideally\nwe'd have another target for the \"configure.ac+\" so this would all\nwork properly, but due to the logic added in 122650457a6 (build: do\nnot automatically reconfigure unless configure.ac changed, 2013-01-02)\nthat's non-trivial to untangle.\n\nSigned-off-by: Ævar Arnfjörð Bjarmason \n---\n Makefile | 3 +--\n 1 file changed, 1 insertion(+), 2 deletions(-)\n\ndiff --git a/Makefile b/Makefile\nindex 573bce20357..ce76c476a3c 100644\n--- a/Makefile\n+++ b/Makefile\n@@ -2358,8 +2358,7 @@ $(SCRIPT_PYTHON_GEN): % : unimplemented.sh\n \tmv $@+ $@\n endif # NO_PYTHON\n \n-CONFIGURE_RECIPE = $(RM) configure configure.ac+ && \\\n-\t\t sed -e 's/@@GIT_VERSION@@/$(GIT_VERSION)/g' \\\n+CONFIGURE_RECIPE = sed -e 's/@@GIT_VERSION@@/$(GIT_VERSION)/g' \\\n \t\t\tconfigure.ac >configure.ac+ && \\\n \t\t autoconf -o configure configure.ac+ && \\\n \t\t $(RM) configure.ac+\n-- \n2.31.1.461.gd47399f6574\n\n"},{"id":"420489","messageId":"cover-0.7-00000000000-20210329T162327Z-avarab@gmail.com","threadId":"55269","inReplyTo":"cover-0.6-00000000000-20210329T161723Z-avarab@gmail.com","subject":"[PATCH 0/6] Makefile: make non-symlink & non-hardlink install sane","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2021-03-29T16:31:38Z","receivedAt":"2021-03-29T16:32:31Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"This is on top of my just-submitted \"Makefile: don't die on AIX with\nopen ./git\" series: [1]\n\nThis series introduces no \"real\" behavior changes I'd expect anyone to\nnotice, but refactors a lengthy copy/pasted test/if/else in the\nMakefile into a simple helper script.\n\nThe \"real\" behavior change is that we no longer ask the user how\nthey'd like to install (symlinks, hardlinks, neither?) and then\nproceed to ignore what was asked of us and optimistically fallback in\ncase of errors. I.e. the inability to create symlinks or hardlinks.\n\nInstead we'll just die, the old behavior is still available as\nINSTALL_FALLBACK_LN_CP. In practice I think exactly nobody actually\nwanted the existing behavior.\n\nIt's just something that emerged over almost 2 decades of first\nwanting to have the ability to specify such a fallback, and then\nadding e.g. support for INSTALL_SYMLINKS along the way.\n\nThere's also side-discussion of a bug I discovered along the way in\nSKIP_DASHED_BUILT_INS in 4/6. This series doesn't make that bug better\nor worse, but it interacts with the flags being changed here.\n\n1. https://lore.kernel.org/git/cover-0.6-00000000000-20210329T161723Z-avarab@gmail.com/\n In practice they apply independently, but since they're touching\n some very adjacent code I'm saying it's \"on top\" in case a re-roll\n makes it so, and also for ease of local testing.\n\n I'm trying a new thing of splitting my serieses up a bit, so if\n there's outstanding feedback on the later parts, hopefully the\n former part can proceed independently...\n\nÆvar Arnfjörð Bjarmason (6):\n Makefile: symlink the same way under \"symlinks\" and \"no hardlinks\"\n Makefile: begin refactoring out \"ln || ln -s || cp\" pattern\n Makefile: refactor out \"ln || ln -s || cp\" pattern\n Makefile: make INSTALL_SYMLINKS affect the build directory\n Makefile: use \"ln -f\" instead of \"rm && ln\"\n Makefile: add a INSTALL_FALLBACK_LN_CP mode\n\n .gitignore | 1 +\n Makefile | 91 +++++++++++++++++++++++++++++++----------------------\n ln-or-cp.sh | 91 +++++++++++++++++++++++++++++++++++++++++++++++++++++\n 3 files changed, 145 insertions(+), 38 deletions(-)\n create mode 100755 ln-or-cp.sh\n\n-- \n2.31.1.461.gd47399f6574\n\n"},{"id":"420490","messageId":"patch-1.7-39926acb0d1-20210329T162327Z-avarab@gmail.com","threadId":"55269","inReplyTo":"cover-0.7-00000000000-20210329T162327Z-avarab@gmail.com","subject":"[PATCH 1/6] Makefile: symlink the same way under \"symlinks\" and \"no hardlinks\"","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2021-03-29T16:31:39Z","receivedAt":"2021-03-29T16:32:31Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"When the INSTALL_SYMLINKS option was added in ad874608d8c (Makefile:\noptionally symlink libexec/git-core binaries to bin/git, 2018-03-13)\nwe retained bug-for-bug compatibility with how the old\nNO_INSTALL_HARDLINKS=Y would selectively fall back on symlinks.\n\nIn particular INSTALL_SYMLINKS=Y will result in a link tree like:\n\n bin/git\n libexec/git -> ../bin/git\n libexec/git-add -> ../bin/git\n\nWhereas NO_INSTALL_HARDLINKS=Y in cases where the \"ln\" would fail would result in:\n\n bin/git\n libexec/git\n libexec/git-add -> git\n\nI.e. we duplicated the \"git\" between the bin/ and libexec/\ndirectories (by default they're hardlinked), and potentially had had\ne.g. \"git-add\" pointing at the libexec/git hardlink (or more likely if\n\"ln\" is failing, a copy), instead of the bin/git.\n\nNow we'll instead use the same symlinks to point to the bindir. I\ndon't see any reason not to do so, and no breakage related to this has\nbeen reported with INSTALL_SYMLINKS in all this time. I just did it\nthis way to maintain bug-for-bug compatibility at the time.\n\nThere is a behavior change here, if the \"ln -s\" fails we'll no longer\ndirect it to stderr. Supporting that edge case would be painful, and\nas we'll see in subsequent commits that difference is going away\nanyway, so let's proceed for now without retaining it.\n\nSigned-off-by: Ævar Arnfjörð Bjarmason \n---\n Makefile | 18 ++++++++----------\n 1 file changed, 8 insertions(+), 10 deletions(-)\n\ndiff --git a/Makefile b/Makefile\nindex ce76c476a3c..1e59d90a8d2 100644\n--- a/Makefile\n+++ b/Makefile\n@@ -334,15 +334,16 @@ all::\n # Define INSTALL_SYMLINKS if you prefer to have everything that can be\n # symlinked between bin/ and libexec/ to use relative symlinks between\n # the two. This option overrides NO_CROSS_DIRECTORY_HARDLINKS and\n-# NO_INSTALL_HARDLINKS which will also use symlinking by indirection\n-# within the same directory in some cases, INSTALL_SYMLINKS will\n+# NO_INSTALL_HARDLINKS. This will not produce any indirect symlinks, we will\n # always symlink to the final target directly.\n #\n # Define NO_CROSS_DIRECTORY_HARDLINKS if you plan to distribute the installed\n # programs as a tar, where bin/ and libexec/ might be on different file systems.\n #\n-# Define NO_INSTALL_HARDLINKS if you prefer to use either symbolic links or\n-# copies to install built-in git commands e.g. git-cat-file.\n+# Define NO_INSTALL_HARDLINKS if you'd like to have programs in bin/\n+# and libexec/ either symlinked (we try with INSTALL_SYMLINKS first),\n+# or if that fails fall back on a \"cp\" instead of a \"ln\". Useful for\n+# when you don't want hardlinks at all.\n #\n # Define SKIP_DASHED_BUILT_INS if you do not need the dashed versions of the\n # built-ins to be linked/copied at all.\n@@ -3019,33 +3020,30 @@ endif\n \t} && \\\n \tfor p in $(filter $(install_bindir_programs),$(BUILT_INS)); do \\\n \t\t$(RM) \"$$bindir/$$p\" && \\\n-\t\ttest -n \"$(INSTALL_SYMLINKS)\" && \\\n+\t\ttest -n \"$(INSTALL_SYMLINKS)\" -o \"$(NO_INSTALL_HARDLINKS)\" && \\\n \t\tln -s \"git$X\" \"$$bindir/$$p\" || \\\n \t\t{ test -z \"$(NO_INSTALL_HARDLINKS)\" && \\\n \t\t ln \"$$bindir/git$X\" \"$$bindir/$$p\" 2>/dev/null || \\\n-\t\t ln -s \"git$X\" \"$$bindir/$$p\" 2>/dev/null || \\\n \t\t cp \"$$bindir/git$X\" \"$$bindir/$$p\" || exit; }; \\\n \tdone && \\\n \tfor p in $(BUILT_INS); do \\\n \t\t$(RM) \"$$execdir/$$p\" && \\\n \t\tif test -z \"$(SKIP_DASHED_BUILT_INS)\"; \\\n \t\tthen \\\n-\t\t\ttest -n \"$(INSTALL_SYMLINKS)\" && \\\n+\t\t\ttest -n \"$(INSTALL_SYMLINKS)\" -o \"$(NO_INSTALL_HARDLINKS)\" && \\\n \t\t\tln -s \"$$destdir_from_execdir_SQ/$(bindir_relative_SQ)/git$X\" \"$$execdir/$$p\" || \\\n \t\t\t{ test -z \"$(NO_INSTALL_HARDLINKS)\" && \\\n \t\t\t ln \"$$execdir/git$X\" \"$$execdir/$$p\" 2>/dev/null || \\\n-\t\t\t ln -s \"git$X\" \"$$execdir/$$p\" 2>/dev/null || \\\n \t\t\t cp \"$$execdir/git$X\" \"$$execdir/$$p\" || exit; }; \\\n \t\tfi \\\n \tdone && \\\n \tremote_curl_aliases=\"$(REMOTE_CURL_ALIASES)\" && \\\n \tfor p in $$remote_curl_aliases; do \\\n \t\t$(RM) \"$$execdir/$$p\" && \\\n-\t\ttest -n \"$(INSTALL_SYMLINKS)\" && \\\n+\t\ttest -n \"$(INSTALL_SYMLINKS)\" -o \"$(NO_INSTALL_HARDLINKS)\" && \\\n \t\tln -s \"git-remote-http$X\" \"$$execdir/$$p\" || \\\n \t\t{ test -z \"$(NO_INSTALL_HARDLINKS)\" && \\\n \t\t ln \"$$execdir/git-remote-http$X\" \"$$execdir/$$p\" 2>/dev/null || \\\n-\t\t ln -s \"git-remote-http$X\" \"$$execdir/$$p\" 2>/dev/null || \\\n \t\t cp \"$$execdir/git-remote-http$X\" \"$$execdir/$$p\" || exit; } \\\n \tdone && \\\n \t./check_bindir \"z$$bindir\" \"z$$execdir\" \"$$bindir/git-add$X\"\n-- \n2.31.1.461.gd47399f6574\n\n"},{"id":"420491","messageId":"patch-5.7-f81708f6120-20210329T162327Z-avarab@gmail.com","threadId":"55269","inReplyTo":"cover-0.7-00000000000-20210329T162327Z-avarab@gmail.com","subject":"[PATCH 5/6] Makefile: use \"ln -f\" instead of \"rm && ln\"","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2021-03-29T16:31:43Z","receivedAt":"2021-03-29T16:33:04Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"Change the invocations and behavior of \"ln-or-cp.sh\" to not assume\nthat we're going to \"rm\" the file in question beforehand.\n\nThis reduces the complexity of these rules, and as a bonus means it's\nnow safe to \"make install\" on a system that may have running \"git\"\nprograms, before this we'd be racing the \"rm && ln/cp\" and wouldn't\nhave a working \"git\" (or one of the built-ins) in the interim.\n\nSigned-off-by: Ævar Arnfjörð Bjarmason \n---\n Makefile | 10 ++--------\n ln-or-cp.sh | 5 ++---\n 2 files changed, 4 insertions(+), 11 deletions(-)\n\ndiff --git a/Makefile b/Makefile\nindex 29d9bade5a8..466df1a8e90 100644\n--- a/Makefile\n+++ b/Makefile\n@@ -2204,8 +2204,7 @@ version.sp version.s version.o: EXTRA_CPPFLAGS = \\\n \n $(BUILT_INS): GIT-LNCPFLAGS\n $(BUILT_INS): git$X\n-\t$(QUIET_BUILT_IN)$(RM) $@ && \\\n-\t./ln-or-cp.sh \\\n+\t$(QUIET_BUILT_IN)./ln-or-cp.sh \\\n \t\t--install-symlinks \"$(INSTALL_SYMLINKS)\" \\\n \t\t$< $@\n \n@@ -2558,8 +2557,7 @@ git-http-push$X: http.o http-push.o GIT-LDFLAGS $(GITLIBS)\n \n $(REMOTE_CURL_ALIASES): GIT-LNCPFLAGS\n $(REMOTE_CURL_ALIASES): $(REMOTE_CURL_PRIMARY)\n-\t$(QUIET_LNCP)$(RM) $@ && \\\n-\t./ln-or-cp.sh \\\n+\t$(QUIET_LNCP)./ln-or-cp.sh \\\n \t\t--install-symlinks \"$(INSTALL_SYMLINKS)\" \\\n \t\t$< $@\n \n@@ -3021,7 +3019,6 @@ endif\n \tdestdir_from_execdir_SQ=$$(echo '$(gitexecdir_relative_SQ)' | sed -e 's|[^/][^/]*|..|g') && \\\n \t{ test \"$$bindir/\" = \"$$execdir/\" || \\\n \t for p in git$X $(filter $(install_bindir_programs),$(ALL_PROGRAMS)); do \\\n-\t\t$(RM) \"$$execdir/$$p\" && \\\n \t\t./ln-or-cp.sh \\\n \t\t\t--install-symlinks \"$(INSTALL_SYMLINKS)\" \\\n \t\t\t--no-install-hardlinks \"$(NO_INSTALL_HARDLINKS)\" \\\n@@ -3031,7 +3028,6 @@ endif\n \t done; \\\n \t} && \\\n \tfor p in $(filter $(install_bindir_programs),$(BUILT_INS)); do \\\n-\t\t$(RM) \"$$bindir/$$p\" && \\\n \t\t./ln-or-cp.sh \\\n \t\t\t--install-symlinks \"$(INSTALL_SYMLINKS)\" \\\n \t\t\t--no-install-hardlinks \"$(NO_INSTALL_HARDLINKS)\" \\\n@@ -3039,7 +3035,6 @@ endif\n \t\t\t\"$$bindir/git$X\" \"$$bindir/$$p\"; \\\n \tdone && \\\n \tfor p in $(BUILT_INS); do \\\n-\t\t$(RM) \"$$execdir/$$p\" && \\\n \t\tif test -z \"$(SKIP_DASHED_BUILT_INS)\"; \\\n \t\tthen \\\n \t\t\t./ln-or-cp.sh \\\n@@ -3051,7 +3046,6 @@ endif\n \tdone && \\\n \tremote_curl_aliases=\"$(REMOTE_CURL_ALIASES)\" && \\\n \tfor p in $$remote_curl_aliases; do \\\n-\t\t$(RM) \"$$execdir/$$p\" && \\\n \t\t./ln-or-cp.sh \\\n \t\t\t--install-symlinks \"$(INSTALL_SYMLINKS)\" \\\n \t\t\t--no-install-hardlinks \"$(NO_INSTALL_HARDLINKS)\" \\\ndiff --git a/ln-or-cp.sh b/ln-or-cp.sh\nindex 663ffd0489d..37380993c64 100755\n--- a/ln-or-cp.sh\n+++ b/ln-or-cp.sh\n@@ -40,11 +40,10 @@ link=\"$2\"\n hardlink_or_cp () {\n \tif test -z \"$no_install_hardlinks\" -a -z \"$no_cross_directory_hardlinks\"\n \tthen\n-\t\tif ! ln \"$target\" \"$link\"\n+\t\tif ! ln -f \"$target\" \"$link\"\n \t\tthen\n \t\t\tcp \"$target\" \"$link\"\n \t\tfi\n-\n \telse\n \t\tcp \"$target\" \"$link\"\n \tfi\n@@ -53,7 +52,7 @@ hardlink_or_cp () {\n main_with_fallbacks () {\n \tif test -n \"$install_symlinks\" -o -n \"$no_install_hardlinks\"\n \tthen\n-\t\tif ! ln -s \"$symlink_target\" \"$link\"\n+\t\tif ! ln -f -s \"$symlink_target\" \"$link\"\n \t\tthen\n \t\t\thardlink_or_cp\n \t\tfi\n-- \n2.31.1.461.gd47399f6574\n\n"},{"id":"420492","messageId":"patch-4.7-1089ca3d184-20210329T162327Z-avarab@gmail.com","threadId":"55269","inReplyTo":"cover-0.7-00000000000-20210329T162327Z-avarab@gmail.com","subject":"[PATCH 4/6] Makefile: make INSTALL_SYMLINKS affect the build directory","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2021-03-29T16:31:42Z","receivedAt":"2021-03-29T16:33:04Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"Change the INSTALL_SYMLINKS flag to also affect whether a built-in\nlike e.g. git-fetch is a symlink or not in git's build directory.\n\nThis doesn't matter for anything other than as an aid to developers\nwho might be confused about their build not matching the installation,\nand who'd like to be reminded that e.g. \"git-fetch\" is a built-in by\n\"ls\" coloring appropriately it as a symlink.\n\nIn order to make this work we need to rebuild the relevant programs\nwhen the INSTALL_SYMLINKS flag changes. This also ensures that we'll\ninstall the right thing, we don't want a different \"INSTALL_SYMLINKS\"\nsetting during \"make all\" and \"make install\" to result in a broken\ninstallation.\n\nWe will do the wrong thing here if both the SKIP_DASHED_BUILT_INS and\nINSTALL_SYMLINKS are being flipped. But that's not a new bug:\n\nA build with \"INSTALL_SYMLINKS=Y SKIP_DASHED_BUILT_INS=\" will result\nin e.g. \"git-fetch\" being a symlink. When building again with\n\"INSTALL_SYMLINKS= SKIP_DASHED_BUILT_INS=Y\", only unconditionally\nbuilt programs such as \"git-upload-pack\" will correctly be flipped to\na hardlink, but e.g. \"git-fetch\" will be left as a symlink.\n\nThat's an existing bug (or unexpected behavior) in the\nSKIP_DASHED_BUILT_INS flag, not something new being introduced or made\nworse here. It's a bit more noticeable now as we might not expect\nthese now-stale symlinks to be left behind, and \"ls\" (in some\nconfigurations) will color them prominently.\n\nBut we'll still do the right thing on \"make install\" since we'll\nignore the likes of \"git-fetch\" there under \"SKIP_DASHED_BUILT_INS=Y\".\nUnder \"SKIP_DASHED_BUILT_INS=\" we'll correctly flip the symlink to a\nhardlink or vice-versa if needed before installation.\n\nStill, we should get around to fixing that SKIP_DASHED_BUILT_INS. You\ncan't reliably set that flag to \"Y\" for checking whether the tests\nrely on the now-skipped dashed built-ins without first running \"make\nclean\" (or knowing you've always been building with\nSKIP_DASHED_BUILT_INS=Y).\n\nSigned-off-by: Ævar Arnfjörð Bjarmason \n---\n .gitignore | 1 +\n Makefile | 19 +++++++++++++++++--\n 2 files changed, 18 insertions(+), 2 deletions(-)\n\ndiff --git a/.gitignore b/.gitignore\nindex 3dcdb6bb5ab..f90aa21b23b 100644\n--- a/.gitignore\n+++ b/.gitignore\n@@ -5,6 +5,7 @@\n /GIT-BUILD-OPTIONS\n /GIT-CFLAGS\n /GIT-LDFLAGS\n+/GIT-LNCPFLAGS\n /GIT-PREFIX\n /GIT-PERL-DEFINES\n /GIT-PERL-HEADER\ndiff --git a/Makefile b/Makefile\nindex a4784f28f5b..29d9bade5a8 100644\n--- a/Makefile\n+++ b/Makefile\n@@ -337,6 +337,11 @@ all::\n # NO_INSTALL_HARDLINKS. This will not produce any indirect symlinks, we will\n # always symlink to the final target directly.\n #\n+# NO_INSTALL_HARDLINKS which will also use symlinking by indirection\n+# within the same directory in some cases, INSTALL_SYMLINKS will\n+# always symlink to the final target directly. This option also\n+# affects dashed built-ins in the build directory pre-installation.\n+#\n # Define NO_CROSS_DIRECTORY_HARDLINKS if you plan to distribute the installed\n # programs as a tar, where bin/ and libexec/ might be on different file systems.\n #\n@@ -2197,9 +2202,12 @@ version.sp version.s version.o: EXTRA_CPPFLAGS = \\\n \t\tGIT_CEILING_DIRECTORIES=\"$(CURDIR)/..\" \\\n \t\tgit rev-parse -q --verify HEAD 2>/dev/null)\"'\n \n+$(BUILT_INS): GIT-LNCPFLAGS\n $(BUILT_INS): git$X\n \t$(QUIET_BUILT_IN)$(RM) $@ && \\\n-\t./ln-or-cp.sh $< $@\n+\t./ln-or-cp.sh \\\n+\t\t--install-symlinks \"$(INSTALL_SYMLINKS)\" \\\n+\t\t$< $@\n \n config-list.h: generate-configlist.sh\n \n@@ -2548,9 +2556,12 @@ git-http-push$X: http.o http-push.o GIT-LDFLAGS $(GITLIBS)\n \t\t$(CURL_LIBCURL) $(EXPAT_LIBEXPAT) $(LIBS) && \\\n \tmv $@+ $@\n \n+$(REMOTE_CURL_ALIASES): GIT-LNCPFLAGS\n $(REMOTE_CURL_ALIASES): $(REMOTE_CURL_PRIMARY)\n \t$(QUIET_LNCP)$(RM) $@ && \\\n-\t./ln-or-cp.sh $< $@\n+\t./ln-or-cp.sh \\\n+\t\t--install-symlinks \"$(INSTALL_SYMLINKS)\" \\\n+\t\t$< $@\n \n $(REMOTE_CURL_PRIMARY): remote-curl.o http.o http-walker.o GIT-LDFLAGS $(GITLIBS)\n \t$(QUIET_LINK)$(CC) $(ALL_CFLAGS) -o $@+ $(ALL_LDFLAGS) $(filter %.o,$^) \\\n@@ -2742,6 +2753,10 @@ GIT-LDFLAGS: FORCE\n \t\techo \"$$FLAGS\" >GIT-LDFLAGS; \\\n fi\n \n+GIT-LNCPFLAGS: FORCE\n+\t@echo INSTALL_SYMLINKS=\\''$(subst ','\\'',$(subst ','\\'',$(INSTALL_SYMLINKS)))'\\' >$@+\n+\t@if cmp $@+ $@ >/dev/null 2>&1; then $(RM) $@+; else mv $@+ $@; fi\n+\n # We need to apply sq twice, once to protect from the shell\n # that runs GIT-BUILD-OPTIONS, and then again to protect it\n # and the first level quoting from the shell that runs \"echo\".\n-- \n2.31.1.461.gd47399f6574\n\n"},{"id":"420493","messageId":"patch-6.7-9ada8979890-20210329T162327Z-avarab@gmail.com","threadId":"55269","inReplyTo":"cover-0.7-00000000000-20210329T162327Z-avarab@gmail.com","subject":"[PATCH 6/6] Makefile: add a INSTALL_FALLBACK_LN_CP mode","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2021-03-29T16:31:44Z","receivedAt":"2021-03-29T16:33:04Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"Change the default behavior on \"make install\" where we fallback\nthrough a chain of \"ln || ln -s || cp\" to instead error out when we\ncan't symlink or hardlink, and not then fallback on a \"cp\" (or from a\nsymlink to hardlink etc.).\n\nThe fallback behavior was introduced in 3e073dc5611 (Makefile: always\nprovide a fallback when hardlinks fail, 2008-08-25), since then we've\ngained the ability to specify e.g. that we'd like symlinks via the\nINSTALL_SYMLINKS setting.\n\nIt doesn't make sense as a default to silently fall back if we can't\nsatisfy those settings. We should instead error out, at which point\nthe developer/builder/sysadmin can set one or more of the relevant\nhardlink or symlink settings.\n\nThis also changes the behavior for the build (not install!) to always\nuse the new strict mode. This will theoretically break things for\nsomeone who can't make symlinks or hardlinks in their git checkout\nwhen building.\n\nThat part of this change would break building on Windows and other\nplatforms that don't support symlinks if INSTALL_SYMLINKS were to\nbecome the default, but it's not on by default, so we should be fine\nhere. If and when INSTALL_SYMLINKS ever becomes the default the right\nway to deal with this would be to tweak config.mak.uname\nappropriately, not to silently fall back on a copy.\n\nSigned-off-by: Ævar Arnfjörð Bjarmason \n---\n Makefile | 11 +++++++++++\n ln-or-cp.sh | 29 ++++++++++++++++++++++++++++-\n 2 files changed, 39 insertions(+), 1 deletion(-)\n\ndiff --git a/Makefile b/Makefile\nindex 466df1a8e90..ccbded79093 100644\n--- a/Makefile\n+++ b/Makefile\n@@ -350,6 +350,13 @@ all::\n # or if that fails fall back on a \"cp\" instead of a \"ln\". Useful for\n # when you don't want hardlinks at all.\n #\n+# Define INSTALL_FALLBACK_LN_CP if you'd like your\n+# \"INSTALL_SYMLINKS=Y\" to fall back on hardlinks if we can't run \"ln\n+# -s\", and for \"ln\" to fall back on \"NO_INSTALL_HARDLINKS=Y\" if we\n+# can't perform a \"ln\" and need to fall-back to a \"cp\". This used to\n+# be the default behavior, but we'll now error if we can't satisfy\n+# your INSTALL_SYMLINKS, NO_INSTALL_HARDLINKS etc. settings.\n+#\n # Define SKIP_DASHED_BUILT_INS if you do not need the dashed versions of the\n # built-ins to be linked/copied at all.\n #\n@@ -3020,6 +3027,7 @@ endif\n \t{ test \"$$bindir/\" = \"$$execdir/\" || \\\n \t for p in git$X $(filter $(install_bindir_programs),$(ALL_PROGRAMS)); do \\\n \t\t./ln-or-cp.sh \\\n+\t\t\t--install-fallback-ln-cp \"$(INSTALL_FALLBACK_LN_CP)\" \\\n \t\t\t--install-symlinks \"$(INSTALL_SYMLINKS)\" \\\n \t\t\t--no-install-hardlinks \"$(NO_INSTALL_HARDLINKS)\" \\\n \t\t\t--no-cross-directory-hardlinks \"$(NO_CROSS_DIRECTORY_HARDLINKS)\" \\\n@@ -3029,6 +3037,7 @@ endif\n \t} && \\\n \tfor p in $(filter $(install_bindir_programs),$(BUILT_INS)); do \\\n \t\t./ln-or-cp.sh \\\n+\t\t\t--install-fallback-ln-cp \"$(INSTALL_FALLBACK_LN_CP)\" \\\n \t\t\t--install-symlinks \"$(INSTALL_SYMLINKS)\" \\\n \t\t\t--no-install-hardlinks \"$(NO_INSTALL_HARDLINKS)\" \\\n \t\t\t--symlink-target \"git$X\" \\\n@@ -3038,6 +3047,7 @@ endif\n \t\tif test -z \"$(SKIP_DASHED_BUILT_INS)\"; \\\n \t\tthen \\\n \t\t\t./ln-or-cp.sh \\\n+\t\t\t\t--install-fallback-ln-cp \"$(INSTALL_FALLBACK_LN_CP)\" \\\n \t\t\t\t--install-symlinks \"$(INSTALL_SYMLINKS)\" \\\n \t\t\t\t--no-install-hardlinks \"$(NO_INSTALL_HARDLINKS)\" \\\n \t\t\t\t--symlink-target \"$$destdir_from_execdir_SQ/$(bindir_relative_SQ)/git$X\" \\\n@@ -3047,6 +3057,7 @@ endif\n \tremote_curl_aliases=\"$(REMOTE_CURL_ALIASES)\" && \\\n \tfor p in $$remote_curl_aliases; do \\\n \t\t./ln-or-cp.sh \\\n+\t\t\t--install-fallback-ln-cp \"$(INSTALL_FALLBACK_LN_CP)\" \\\n \t\t\t--install-symlinks \"$(INSTALL_SYMLINKS)\" \\\n \t\t\t--no-install-hardlinks \"$(NO_INSTALL_HARDLINKS)\" \\\n \t\t\t--symlink-target \"git-remote-http$X\" \\\ndiff --git a/ln-or-cp.sh b/ln-or-cp.sh\nindex 37380993c64..f77dad71bdb 100755\n--- a/ln-or-cp.sh\n+++ b/ln-or-cp.sh\n@@ -1,5 +1,6 @@\n #!/bin/sh\n \n+install_fallback_ln_cp=\n install_symlinks=\n no_install_hardlinks=\n no_cross_directory_hardlinks=\n@@ -7,6 +8,10 @@ symlink_target=\n while test $# != 0\n do\n \tcase \"$1\" in\n+\t--install-fallback-ln-cp)\n+\t\tinstall_fallback_ln_cp=\"$2\"\n+\t\tshift\n+\t\t;;\n \t--install-symlinks)\n \t\tinstall_symlinks=\"$2\"\n \t\tshift\n@@ -61,4 +66,26 @@ main_with_fallbacks () {\n \tfi\n }\n \n-main_with_fallbacks\n+main_no_fallbacks () {\n+\tif test -n \"$no_install_hardlinks\" -a -z \"$install_symlinks\"\n+\tthen\n+\t\tcp \"$target\" \"$link\"\n+\telif test -n \"$install_symlinks\" -o -n \"$no_cross_directory_hardlinks\"\n+\tthen\n+\t\tln -f -s \"$symlink_target\" \"$link\"\n+\telif test -n \"$no_install_hardlinks\"\n+\tthen\n+\t\tcp \"$target\" \"$link\"\n+\telse\n+\t\tln -f \"$target\" \"$link\"\n+\tfi\n+}\n+\n+if test -z \"$install_fallback_ln_cp\"\n+then\n+\t# The stricter mode, where we know what we want\n+\tmain_no_fallbacks\n+else\n+\tmain_with_fallbacks\n+\n+fi\n-- \n2.31.1.461.gd47399f6574\n\n"},{"id":"420494","messageId":"patch-3.7-bde9de756b4-20210329T162327Z-avarab@gmail.com","threadId":"55269","inReplyTo":"cover-0.7-00000000000-20210329T162327Z-avarab@gmail.com","subject":"[PATCH 3/6] Makefile: refactor out \"ln || ln -s || cp\" pattern","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2021-03-29T16:31:41Z","receivedAt":"2021-03-29T16:33:04Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"Refactor out the hard-to-read and maintain \"ln || ln -s || cp\"\npattern.\n\nThis was initially added in 3e073dc5611 (Makefile: always provide a\nfallback when hardlinks fail, 2008-08-25), but since then it's become\na lot more complex as we've added:\n\n * 3426e34fedd (Add NO_CROSS_DIRECTORY_HARDLINKS support to the\n Makefile, 2009-05-11)\n\n * NO_INSTALL_HARDLINKS in 70de5e65e8c (Makefile:\n NO_INSTALL_HARDLINKS, 2012-05-02)\n\n * INSTALL_SYMLINKS in ad874608d8c (Makefile: optionally symlink\n libexec/git-core binaries to bin/git, 2018-03-13)\n\n * SKIP_DASHED_BUILT_INS 179227d6e21 (Optionally skip linking/copying\n the built-ins, 2020-09-21)\n\nEach of those commits had to add a new special-case to this code,\nresulting in quite an unmaintainable mess for adding any sort of new\noption.\n\nLet's use the newly introduced ln-or-cp.sh script instead, note that\nwe only sometimes pass the --no-cross-directory-hardlinks option, per\nthe previous behavior. The target of the \"ln -s\" is also another\nspecial snowflake, but we're careful to carry that forward.\n\nAs in an earlier commit this also changes the behavior to emit any\nerrors to stdout. In that earlier case that was done to simplify the\nscript so that it can use one \"ln -s\" instead of the two, likewise\nwe're now unconditionally emitting to stderr if ln (without -s, to\ncreate the hardlink) fails. We always emitted to stderr if \"cp\"\nfailed.\n\nAs in that earlier commit let's let that pass for now, yes it might be\nvery verbose in some scenarios, but we're working our way towards a\nsimpler end-state here.\n\nSigned-off-by: Ævar Arnfjörð Bjarmason \n---\n Makefile | 41 +++++++++++++++++-----------------\n ln-or-cp.sh | 63 ++++++++++++++++++++++++++++++++++++++++++++++++++---\n 2 files changed, 81 insertions(+), 23 deletions(-)\n\ndiff --git a/Makefile b/Makefile\nindex cfc87d7734d..a4784f28f5b 100644\n--- a/Makefile\n+++ b/Makefile\n@@ -3007,40 +3007,41 @@ endif\n \t{ test \"$$bindir/\" = \"$$execdir/\" || \\\n \t for p in git$X $(filter $(install_bindir_programs),$(ALL_PROGRAMS)); do \\\n \t\t$(RM) \"$$execdir/$$p\" && \\\n-\t\ttest -n \"$(INSTALL_SYMLINKS)\" && \\\n-\t\tln -s \"$$destdir_from_execdir_SQ/$(bindir_relative_SQ)/$$p\" \"$$execdir/$$p\" || \\\n-\t\t{ test -z \"$(NO_INSTALL_HARDLINKS)$(NO_CROSS_DIRECTORY_HARDLINKS)\" && \\\n-\t\t ln \"$$bindir/$$p\" \"$$execdir/$$p\" 2>/dev/null || \\\n-\t\t cp \"$$bindir/$$p\" \"$$execdir/$$p\" || exit; } \\\n+\t\t./ln-or-cp.sh \\\n+\t\t\t--install-symlinks \"$(INSTALL_SYMLINKS)\" \\\n+\t\t\t--no-install-hardlinks \"$(NO_INSTALL_HARDLINKS)\" \\\n+\t\t\t--no-cross-directory-hardlinks \"$(NO_CROSS_DIRECTORY_HARDLINKS)\" \\\n+\t\t\t--symlink-target \"$$destdir_from_execdir_SQ/$(bindir_relative_SQ)/$$p\" \\\n+\t\t\t\"$$bindir/$$p\" \"$$execdir/$$p\"; \\\n \t done; \\\n \t} && \\\n \tfor p in $(filter $(install_bindir_programs),$(BUILT_INS)); do \\\n \t\t$(RM) \"$$bindir/$$p\" && \\\n-\t\ttest -n \"$(INSTALL_SYMLINKS)\" -o \"$(NO_INSTALL_HARDLINKS)\" && \\\n-\t\tln -s \"git$X\" \"$$bindir/$$p\" || \\\n-\t\t{ test -z \"$(NO_INSTALL_HARDLINKS)\" && \\\n-\t\t ln \"$$bindir/git$X\" \"$$bindir/$$p\" 2>/dev/null || \\\n-\t\t cp \"$$bindir/git$X\" \"$$bindir/$$p\" || exit; }; \\\n+\t\t./ln-or-cp.sh \\\n+\t\t\t--install-symlinks \"$(INSTALL_SYMLINKS)\" \\\n+\t\t\t--no-install-hardlinks \"$(NO_INSTALL_HARDLINKS)\" \\\n+\t\t\t--symlink-target \"git$X\" \\\n+\t\t\t\"$$bindir/git$X\" \"$$bindir/$$p\"; \\\n \tdone && \\\n \tfor p in $(BUILT_INS); do \\\n \t\t$(RM) \"$$execdir/$$p\" && \\\n \t\tif test -z \"$(SKIP_DASHED_BUILT_INS)\"; \\\n \t\tthen \\\n-\t\t\ttest -n \"$(INSTALL_SYMLINKS)\" -o \"$(NO_INSTALL_HARDLINKS)\" && \\\n-\t\t\tln -s \"$$destdir_from_execdir_SQ/$(bindir_relative_SQ)/git$X\" \"$$execdir/$$p\" || \\\n-\t\t\t{ test -z \"$(NO_INSTALL_HARDLINKS)\" && \\\n-\t\t\t ln \"$$execdir/git$X\" \"$$execdir/$$p\" 2>/dev/null || \\\n-\t\t\t cp \"$$execdir/git$X\" \"$$execdir/$$p\" || exit; }; \\\n+\t\t\t./ln-or-cp.sh \\\n+\t\t\t\t--install-symlinks \"$(INSTALL_SYMLINKS)\" \\\n+\t\t\t\t--no-install-hardlinks \"$(NO_INSTALL_HARDLINKS)\" \\\n+\t\t\t\t--symlink-target \"$$destdir_from_execdir_SQ/$(bindir_relative_SQ)/git$X\" \\\n+\t\t\t\t\"$$execdir/git$X\" \"$$execdir/$$p\"; \\\n \t\tfi \\\n \tdone && \\\n \tremote_curl_aliases=\"$(REMOTE_CURL_ALIASES)\" && \\\n \tfor p in $$remote_curl_aliases; do \\\n \t\t$(RM) \"$$execdir/$$p\" && \\\n-\t\ttest -n \"$(INSTALL_SYMLINKS)\" -o \"$(NO_INSTALL_HARDLINKS)\" && \\\n-\t\tln -s \"git-remote-http$X\" \"$$execdir/$$p\" || \\\n-\t\t{ test -z \"$(NO_INSTALL_HARDLINKS)\" && \\\n-\t\t ln \"$$execdir/git-remote-http$X\" \"$$execdir/$$p\" 2>/dev/null || \\\n-\t\t cp \"$$execdir/git-remote-http$X\" \"$$execdir/$$p\" || exit; } \\\n+\t\t./ln-or-cp.sh \\\n+\t\t\t--install-symlinks \"$(INSTALL_SYMLINKS)\" \\\n+\t\t\t--no-install-hardlinks \"$(NO_INSTALL_HARDLINKS)\" \\\n+\t\t\t--symlink-target \"git-remote-http$X\" \\\n+\t\t\t\"$$execdir/git-remote-http$X\" \"$$execdir/$$p\"; \\\n \tdone && \\\n \t./check_bindir \"z$$bindir\" \"z$$execdir\" \"$$bindir/git-add$X\"\n \ndiff --git a/ln-or-cp.sh b/ln-or-cp.sh\nindex de79cd85a81..663ffd0489d 100755\n--- a/ln-or-cp.sh\n+++ b/ln-or-cp.sh\n@@ -1,8 +1,65 @@\n #!/bin/sh\n \n+install_symlinks=\n+no_install_hardlinks=\n+no_cross_directory_hardlinks=\n+symlink_target=\n+while test $# != 0\n+do\n+\tcase \"$1\" in\n+\t--install-symlinks)\n+\t\tinstall_symlinks=\"$2\"\n+\t\tshift\n+\t\t;;\n+\t--no-install-hardlinks)\n+\t\tno_install_hardlinks=\"$2\"\n+\t\tshift\n+\t\t;;\n+\t--no-cross-directory-hardlinks)\n+\t\tno_cross_directory_hardlinks=\"$2\"\n+\t\tshift\n+\t\t;;\n+\t--symlink-target)\n+\t\tsymlink_target=\"$2\"\n+\t\tshift\n+\t\t;;\n+\t*)\n+\t\tbreak\n+\t\t;;\n+\tesac\n+\tshift\n+done\n+\n target=\"$1\"\n+if test -z \"$symlink_target\"\n+then\n+\tsymlink_target=\"$target\"\n+fi\n link=\"$2\"\n \n-ln \"$target\" \"$link\" 2>/dev/null ||\n-ln -s \"$target\" \"$link\" 2>/dev/null ||\n-cp \"$target\" \"$link\"\n+hardlink_or_cp () {\n+\tif test -z \"$no_install_hardlinks\" -a -z \"$no_cross_directory_hardlinks\"\n+\tthen\n+\t\tif ! ln \"$target\" \"$link\"\n+\t\tthen\n+\t\t\tcp \"$target\" \"$link\"\n+\t\tfi\n+\n+\telse\n+\t\tcp \"$target\" \"$link\"\n+\tfi\n+}\n+\n+main_with_fallbacks () {\n+\tif test -n \"$install_symlinks\" -o -n \"$no_install_hardlinks\"\n+\tthen\n+\t\tif ! ln -s \"$symlink_target\" \"$link\"\n+\t\tthen\n+\t\t\thardlink_or_cp\n+\t\tfi\n+\telse\n+\t\thardlink_or_cp\n+\tfi\n+}\n+\n+main_with_fallbacks\n-- \n2.31.1.461.gd47399f6574\n\n"},{"id":"420495","messageId":"patch-2.7-5f59121dd0f-20210329T162327Z-avarab@gmail.com","threadId":"55269","inReplyTo":"cover-0.7-00000000000-20210329T162327Z-avarab@gmail.com","subject":"[PATCH 2/6] Makefile: begin refactoring out \"ln || ln -s || cp\" pattern","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2021-03-29T16:31:40Z","receivedAt":"2021-03-29T16:34:40Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"Begin refactoring out the \"ln || ln -s || cp\" pattern in the Makefile\ninto a script. For now this is trivial, but in subsequent commits\nit'll simplify things a lot.\n\nSigned-off-by: Ævar Arnfjörð Bjarmason \n---\n Makefile | 8 ++------\n ln-or-cp.sh | 8 ++++++++\n 2 files changed, 10 insertions(+), 6 deletions(-)\n create mode 100755 ln-or-cp.sh\n\ndiff --git a/Makefile b/Makefile\nindex 1e59d90a8d2..cfc87d7734d 100644\n--- a/Makefile\n+++ b/Makefile\n@@ -2199,9 +2199,7 @@ version.sp version.s version.o: EXTRA_CPPFLAGS = \\\n \n $(BUILT_INS): git$X\n \t$(QUIET_BUILT_IN)$(RM) $@ && \\\n-\tln $< $@ 2>/dev/null || \\\n-\tln -s $< $@ 2>/dev/null || \\\n-\tcp $< $@\n+\t./ln-or-cp.sh $< $@\n \n config-list.h: generate-configlist.sh\n \n@@ -2552,9 +2550,7 @@ git-http-push$X: http.o http-push.o GIT-LDFLAGS $(GITLIBS)\n \n $(REMOTE_CURL_ALIASES): $(REMOTE_CURL_PRIMARY)\n \t$(QUIET_LNCP)$(RM) $@ && \\\n-\tln $< $@ 2>/dev/null || \\\n-\tln -s $< $@ 2>/dev/null || \\\n-\tcp $< $@\n+\t./ln-or-cp.sh $< $@\n \n $(REMOTE_CURL_PRIMARY): remote-curl.o http.o http-walker.o GIT-LDFLAGS $(GITLIBS)\n \t$(QUIET_LINK)$(CC) $(ALL_CFLAGS) -o $@+ $(ALL_LDFLAGS) $(filter %.o,$^) \\\ndiff --git a/ln-or-cp.sh b/ln-or-cp.sh\nnew file mode 100755\nindex 00000000000..de79cd85a81\n--- /dev/null\n+++ b/ln-or-cp.sh\n@@ -0,0 +1,8 @@\n+#!/bin/sh\n+\n+target=\"$1\"\n+link=\"$2\"\n+\n+ln \"$target\" \"$link\" 2>/dev/null ||\n+ln -s \"$target\" \"$link\" 2>/dev/null ||\n+cp \"$target\" \"$link\"\n-- \n2.31.1.461.gd47399f6574\n\n"},{"id":"420498","messageId":"xmqqy2e5kegv.fsf@gitster.g","threadId":"55269","inReplyTo":"patch-1.6-3330cdbccc0-20210329T161723Z-avarab@gmail.com","subject":"Re: [PATCH v2 1/5] Makefile: rename objects in-place, don't clobber","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2021-03-29T18:21:52Z","receivedAt":"2021-03-29T18:22:45Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Ævar Arnfjörð Bjarmason writes:\n\n> Per the log of changes to the Makfile and Junio's recent comment about\n> [1] why that pattern got introduced it was for a different reason\n> entirely, i.e. (\"[]\" edits are mine, for brevity):\n>\n> [T]hat age old convention [...] is spelled [as]:\n>\n> \tthing:\n> \t\trm -f thing thing+\n> \t\tprepare contents for thing >thing+\n\nDid I say that? I recall I specifically avoided the \"redirection\"\nbecause this is *NOT* shell-script only principle. If a command is\nso broken that \"cmd -o thing\" that fails to work correctly leaves a\nbroken output in thing, then the pattern should be applied and made\nto \"cmd -o thing+ && mv thing+ thing\".\n\nOn the other hand, if \"cmd\" refrains from leaving a half-baked\nresult in \"-o thing\" (and I believe $(CC) is well-behaved even on\nAIX), I do not think it is a good idea to use that pattern. One\nversion of AIX may refuse to overwrite a file in use because\ncreat(\"thing\") that is necessary for \"-o thing\" may fail while\n\"thing\" is in use), but another system may refuse to rename over a\nfile in use (i.e. \"-o thing+\" into a brand new \"thing+\" may be OK\nbut the final \"mv thing+ thing\" may fail). So pretending that it is\na cure for broken use case is cluttering Makefile for no real\nbenefit and leading readers into confused thinking.\n\n> \t\tmv thing+ thing\n>\n> It protects us from a failure mode where \"prepare contents for\n> thing\" step is broken and leaves a \"thing\" that does not work, but\n> confuses make that make does not need to rebuild it, if you wrote it\n> as such:\n>\n> \tthing:\n> \t\tprepare contents for thing >thing\n>\n> [It might leave behind a corrupt 'thing'.] In any case, it is not\n> \"we are trying to make thing available while it is being\n> rewritten\" at all.\n>\n> That makes perfect sense for shellscripts, but as this change shows\n> there's other good reasons to use this age old convention that weren't\n> considered at the time.\n\nSo, no, the age old convention may have considered and does not\napply to such a use case.\n\n> git$X: git.o GIT-LDFLAGS $(BUILTIN_OBJS) $(GITLIBS)\n> -\t$(QUIET_LINK)$(CC) $(ALL_CFLAGS) -o $@ $(ALL_LDFLAGS) \\\n> -\t\t$(filter %.o,$^) $(LIBS)\n> +\t$(QUIET_LINK)$(CC) $(ALL_CFLAGS) -o $@+ $(ALL_LDFLAGS) \\\n> +\t\t$(filter %.o,$^) $(LIBS) && \\\n> +\tmv $@+ $@\n\nReally, does anybody else use \"$(CC) -o $@\" in such a way in their\nMakefile? Having to do this smells simply crazy (I am not saying\nyou are crazy---the platform that forces you to write such a thing\nis crazy).\n\nSo, while I do not think the end result would break the build (other\nthan it probably would leave crufts \"make clean\" would not notice\nbehind when interrupted in the middle), I am moderately negative on\nthis change.\n\nThanks.\n"},{"id":"420499","messageId":"xmqqr1jxke8h.fsf@gitster.g","threadId":"55269","inReplyTo":"xmqqy2e5kegv.fsf@gitster.g","subject":"Re: [PATCH v2 1/5] Makefile: rename objects in-place, don't clobber","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2021-03-29T18:26:54Z","receivedAt":"2021-03-29T18:27:33Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Junio C Hamano writes:\n\n>> \tthing:\n>> \t\trm -f thing thing+\n>> \t\tprepare contents for thing >thing+\n>\n> Did I say that? I recall I specifically avoided the \"redirection\"\n> because this is *NOT* shell-script only principle.\n\nAh, I did say that in the response to the previous iteration. But\nthe same principle applied to your other patch for [ce]tags which\ntook \"-o output\", and I was recalling my response to that thread.\n\nIn any case, whether \"cmd >thing\" or \"cmd -o thing\", if cmd leaves a\nbroken output in thing when it fails, it needs the \"into thing+ and\nrename to thing\" dance. Redirecion always need it, but a well behaved\ncommand like $(CC) should not need it.\n\nThanks.\n"},{"id":"420500","messageId":"xmqqk0ppkdmd.fsf@gitster.g","threadId":"55269","inReplyTo":"patch-2.6-49dbd54d0ff-20210329T161723Z-avarab@gmail.com","subject":"Re: [PATCH v2 2/5] Makefile: rename scripts in-place, don't clobber","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2021-03-29T18:40:10Z","receivedAt":"2021-03-29T18:40:53Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Ævar Arnfjörð Bjarmason writes:\n\n> It'll also guard against any weird issues with e.g. a command that\n> wants to read GIT-PERL-DEFINES to generate one of the %.perl scripts\n> racing with either a concurrent instance of \"make\" that has partially\n> updated the file, or test-lib.sh dying with some particularly weird\n> error because GIT-BUILD-OPTIONS was partway through being updated when\n> it ran.\n\nIf something like that happens, doesn't that indicate a bug in the\ndependency graph in the Makefile or the implementation of \"make\"?\nThe generated file is depended on for the consumer to be able to use\na non-stale version---so the consumer should not start before the\ngenerator finishes.\n\nYou may be able to hide the breakage coming from \"partially written\nfile is easily recognizable and the consumer would barf\". But I am\nafraid that you are introducing a problem harder to diagnose, i.e.\n\"the consumer reads from a complete file so there is no syntax error\nor other things that easily tells you there is a breakage, but what\nis used is stale and not up to date\".\n\nThe same comment applies to this step. I do not think it would\nbreak (other than adding intermediate crufts) the result if you\ngenerate into temporary and rename to final, but it is not clear\nto me what the point is in doing so.\n"},{"id":"420501","messageId":"xmqqft0dkdch.fsf@gitster.g","threadId":"55269","inReplyTo":"patch-3.6-96e2338ed8e-20210329T161723Z-avarab@gmail.com","subject":"Re: [PATCH v2 3/5] Makefile: don't needlessly \"rm $@ $@+\" before \"mv $@+ $@\"","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2021-03-29T18:46:06Z","receivedAt":"2021-03-29T18:46:49Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Ævar Arnfjörð Bjarmason writes:\n\n> rm -f thing thing+\n> prepare contents for thing >thing+\n> mv thing+ thing\n> ...\n> But I think guarding against \"mv\" failing is a step too far in\n> paranoia,...\n\nIf mv fails the $(MAKE) rule would fail, so it is OK. It may leave\nthing+ behind, but there is no reason to expect why you would be\nable to rm it the next time, so from that point of view, it is OK to\ndrop the first \"rm -f thing+\", I would think. The only case I can\nthing of that would help is when you are sharing the working tree\nwith your team member, the directories are writable to both of you,\nbut somehow the other person creates thing+ with 0644 or 0755 mode\nbits. You cannot redirect into thing+ the other person left behind,\nbut you can \"rm -f thing+ && cmd >$thing+\" (or \"cmd -o $thing+\") in\nsuch a situation, and that is probably where the pattern comes from\n---i.e. simple hygiene.\n"},{"id":"420502","messageId":"xmqqblb1kd47.fsf@gitster.g","threadId":"55269","inReplyTo":"patch-4.6-2ff6359c273-20210329T161723Z-avarab@gmail.com","subject":"Re: [PATCH v2 4/5] Makefile: add the \".DELETE_ON_ERROR\" flag","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2021-03-29T18:51:04Z","receivedAt":"2021-03-29T18:52:14Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Ævar Arnfjörð Bjarmason writes:\n\n> Use the GNU make \".DELETE_ON_ERROR\" flag.\n\nYay!\n\nWith versions of $(MAKE) where this feature is available, it is far\nmore preferrable to use it than \"generate into temporary and rename\nto the final\" dance.\n\nWe do require / depend on GNU but I do not offhand know if this is\navailable in all versions that still matter. If we know we can\nassume the presence of this feature the I do not mind if we jump\ndirectly to this step without those \"do the same for $(CC)\" steps\n(which I deem crazy) before it.\n\n\n"},{"id":"420526","messageId":"xmqq1rbxiozl.fsf@gitster.g","threadId":"55269","inReplyTo":"patch-1.7-39926acb0d1-20210329T162327Z-avarab@gmail.com","subject":"Re: [PATCH 1/6] Makefile: symlink the same way under \"symlinks\" and \"no hardlinks\"","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2021-03-29T22:17:34Z","receivedAt":"2021-03-29T22:18:28Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Ævar Arnfjörð Bjarmason writes:\n\n> In particular INSTALL_SYMLINKS=Y will result in a link tree like:\n>\n> bin/git\n> libexec/git -> ../bin/git\n> libexec/git-add -> ../bin/git\n>\n> Whereas NO_INSTALL_HARDLINKS=Y in cases where the \"ln\" would fail would result in:\n>\n> bin/git\n> libexec/git\n> libexec/git-add -> git\n>\n> I.e. we duplicated the \"git\" between the bin/ and libexec/\n> directories (by default they're hardlinked), and potentially had had\n> e.g. \"git-add\" pointing at the libexec/git hardlink (or more likely if\n> \"ln\" is failing, a copy), instead of the bin/git.\n>\n> Now we'll instead use the same symlinks to point to the bindir. I\n> don't see any reason not to do so, and no breakage related to this has\n> been reported with INSTALL_SYMLINKS in all this time. I just did it\n> this way to maintain bug-for-bug compatibility at the time.\n\nMakes sense. I do not see a reason why libexec/git-add that points\nat ../bin/git would cause problems, either.\n\n> +# Define NO_INSTALL_HARDLINKS if you'd like to have programs in bin/\n> +# and libexec/ either symlinked (we try with INSTALL_SYMLINKS first),\n> +# or if that fails fall back on a \"cp\" instead of a \"ln\". Useful for\n> +# when you don't want hardlinks at all.\n\nSo without INSTALL_SYMLINKS and with NO_INSTALL_HARDLINKS, the only\nremaining choice is \"cp\". With both set, we favour \"ln -s\" over\n\"cp\" and do not allow \"ln\". With INSTALL_SYMLINKS and without\nNO_INSTALL_HARDLINKS, we try \"ln -s\", \"ln\" and finally \"cp\". With\nneither, we try \"ln\" and fall back to \"cp\"?\n\nThat precedence order does make sense.\n\n> @@ -3019,33 +3020,30 @@ endif\n> \t} && \\\n> \tfor p in $(filter $(install_bindir_programs),$(BUILT_INS)); do \\\n> \t\t$(RM) \"$$bindir/$$p\" && \\\n> -\t\ttest -n \"$(INSTALL_SYMLINKS)\" && \\\n> +\t\ttest -n \"$(INSTALL_SYMLINKS)\" -o \"$(NO_INSTALL_HARDLINKS)\" && \\\n\nI had an impression that we avoid -o/-a used with \"test\" (instead\nwe'd use \"test && test\" or \"test || test\")?\n\nIn either case, if we are told to favor \"ln -s\", or told not to use\n\"ln\", we try \"ln -s\"? That does not make much sense to me, though.\n\nWhat do I need to do if I do not ever want symbolic links? Is it\nimpossible now?\n\n> \t\tln -s \"git$X\" \"$$bindir/$$p\" || \\\n> \t\t{ test -z \"$(NO_INSTALL_HARDLINKS)\" && \\\n> \t\t ln \"$$bindir/git$X\" \"$$bindir/$$p\" 2>/dev/null || \\\n> -\t\t ln -s \"git$X\" \"$$bindir/$$p\" 2>/dev/null || \\\n> \t\t cp \"$$bindir/git$X\" \"$$bindir/$$p\" || exit; }; \\\n> \tdone && \\\n> \tfor p in $(BUILT_INS); do \\\n> \t\t$(RM) \"$$execdir/$$p\" && \\\n> \t\tif test -z \"$(SKIP_DASHED_BUILT_INS)\"; \\\n> \t\tthen \\\n> -\t\t\ttest -n \"$(INSTALL_SYMLINKS)\" && \\\n> +\t\t\ttest -n \"$(INSTALL_SYMLINKS)\" -o \"$(NO_INSTALL_HARDLINKS)\" && \\\n\nPerhaps the same comment applies here and to the next hunk, too?\n\n> \t\t\tln -s \"$$destdir_from_execdir_SQ/$(bindir_relative_SQ)/git$X\" \"$$execdir/$$p\" || \\\n> \t\t\t{ test -z \"$(NO_INSTALL_HARDLINKS)\" && \\\n> \t\t\t ln \"$$execdir/git$X\" \"$$execdir/$$p\" 2>/dev/null || \\\n> -\t\t\t ln -s \"git$X\" \"$$execdir/$$p\" 2>/dev/null || \\\n> \t\t\t cp \"$$execdir/git$X\" \"$$execdir/$$p\" || exit; }; \\\n> \t\tfi \\\n> \tdone && \\\n> \tremote_curl_aliases=\"$(REMOTE_CURL_ALIASES)\" && \\\n> \tfor p in $$remote_curl_aliases; do \\\n> \t\t$(RM) \"$$execdir/$$p\" && \\\n> -\t\ttest -n \"$(INSTALL_SYMLINKS)\" && \\\n> +\t\ttest -n \"$(INSTALL_SYMLINKS)\" -o \"$(NO_INSTALL_HARDLINKS)\" && \\\n> \t\tln -s \"git-remote-http$X\" \"$$execdir/$$p\" || \\\n> \t\t{ test -z \"$(NO_INSTALL_HARDLINKS)\" && \\\n> \t\t ln \"$$execdir/git-remote-http$X\" \"$$execdir/$$p\" 2>/dev/null || \\\n> -\t\t ln -s \"git-remote-http$X\" \"$$execdir/$$p\" 2>/dev/null || \\\n> \t\t cp \"$$execdir/git-remote-http$X\" \"$$execdir/$$p\" || exit; } \\\n> \tdone && \\\n> \t./check_bindir \"z$$bindir\" \"z$$execdir\" \"$$bindir/git-add$X\"\n"},{"id":"420527","messageId":"xmqqwntpha9p.fsf@gitster.g","threadId":"55269","inReplyTo":"patch-2.7-5f59121dd0f-20210329T162327Z-avarab@gmail.com","subject":"Re: [PATCH 2/6] Makefile: begin refactoring out \"ln || ln -s || cp\" pattern","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2021-03-29T22:20:50Z","receivedAt":"2021-03-29T22:21:41Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Ævar Arnfjörð Bjarmason writes:\n\n> Begin refactoring out the \"ln || ln -s || cp\" pattern in the Makefile\n> into a script. For now this is trivial, but in subsequent commits\n> it'll simplify things a lot.\n\nI agree with the approach. The precedence glitch I commented on in\nmy review of [1/6] (e.g. how would I say \"I never ever want symbolic\nlinks---just use hardlink and fall back to copying\") would be much\neasier to solve if a single helper is used consistenly---that would\ngive us a single place to centrally fix.\n\n"},{"id":"420528","messageId":"xmqqsg4dha3i.fsf@gitster.g","threadId":"55269","inReplyTo":"patch-3.7-bde9de756b4-20210329T162327Z-avarab@gmail.com","subject":"Re: [PATCH 3/6] Makefile: refactor out \"ln || ln -s || cp\" pattern","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2021-03-29T22:24:33Z","receivedAt":"2021-03-29T22:27:39Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Ævar Arnfjörð Bjarmason writes:\n\n> Refactor out the hard-to-read and maintain \"ln || ln -s || cp\"\n> pattern.\n>\n> This was initially added in 3e073dc5611 (Makefile: always provide a\n> fallback when hardlinks fail, 2008-08-25), but since then it's become\n> a lot more complex as we've added:\n>\n> * 3426e34fedd (Add NO_CROSS_DIRECTORY_HARDLINKS support to the\n> Makefile, 2009-05-11)\n>\n> * NO_INSTALL_HARDLINKS in 70de5e65e8c (Makefile:\n> NO_INSTALL_HARDLINKS, 2012-05-02)\n>\n> * INSTALL_SYMLINKS in ad874608d8c (Makefile: optionally symlink\n> libexec/git-core binaries to bin/git, 2018-03-13)\n>\n> * SKIP_DASHED_BUILT_INS 179227d6e21 (Optionally skip linking/copying\n> the built-ins, 2020-09-21)\n>\n> Each of those commits had to add a new special-case to this code,\n> resulting in quite an unmaintainable mess for adding any sort of new\n> option.\n>\n> Let's use the newly introduced ln-or-cp.sh script instead, note that\n> we only sometimes pass the --no-cross-directory-hardlinks option, per\n> the previous behavior. The target of the \"ln -s\" is also another\n> special snowflake, but we're careful to carry that forward.\n\nNice. These explicit command-line options to the helper may be\nharder to initially write and maintain than just exporting the\nrelevant $(MAKE) macros and using it from the helper, but once\nit works correctly, it is much easier to see what is going on.\n\nAnd obviously, if we want to fix the \"I do not ever want to see any\nsymlinks\", it is very clear that main_with_fallbacks is where the\nchange needs to go.\n\nRaelly nice.\n"},{"id":"420529","messageId":"xmqqo8f1h9ra.fsf@gitster.g","threadId":"55269","inReplyTo":"patch-4.7-1089ca3d184-20210329T162327Z-avarab@gmail.com","subject":"Re: [PATCH 4/6] Makefile: make INSTALL_SYMLINKS affect the build directory","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2021-03-29T22:31:53Z","receivedAt":"2021-03-29T22:34:48Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Ævar Arnfjörð Bjarmason writes:\n\n> Change the INSTALL_SYMLINKS flag to also affect whether a built-in\n> like e.g. git-fetch is a symlink or not in git's build directory.\n>\n> This doesn't matter for anything other than as an aid to developers\n> who might be confused about their build not matching the installation,\n> and who'd like to be reminded that e.g. \"git-fetch\" is a built-in by\n> \"ls\" coloring appropriately it as a symlink.\n\nI am not with the cause and hence not very interested in this\n\"feature\".\n\nWhen there are multiple possible reasons why something is made into\na symbolic link, the symlink-ness in the build directory cannot\nfundamentally mirror the symlink-ness in the installation, no?\n\"git\" and \"git-fetch\" may be in the same directory in the build, but\ntheir installation directories are different, so they may be\nhardlinked in the former but they may be turned into symlinks in the\nlatter.\n\n"},{"id":"420532","messageId":"xmqqim59h93j.fsf@gitster.g","threadId":"55269","inReplyTo":"patch-5.7-f81708f6120-20210329T162327Z-avarab@gmail.com","subject":"Re: [PATCH 5/6] Makefile: use \"ln -f\" instead of \"rm && ln\"","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2021-03-29T22:46:08Z","receivedAt":"2021-03-29T22:47:05Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Ævar Arnfjörð Bjarmason writes:\n\n> Change the invocations and behavior of \"ln-or-cp.sh\" to not assume\n> that we're going to \"rm\" the file in question beforehand.\n>\n> This reduces the complexity of these rules, and as a bonus means it's\n> now safe to \"make install\" on a system that may have running \"git\"\n> programs, before this we'd be racing the \"rm && ln/cp\" and wouldn't\n> have a working \"git\" (or one of the built-ins) in the interim.\n\nNeither link(2) nor symlink(2) has the equivalent of the -f flag, so\n\"ln [-s] -f\" has to be implemented as an unlink(2) followed by\nlink(2) or symlink(2) anyway, so you didn't solve the \"racing\"\nproblem (if that is a problem in the first place, that is), did you?\n\nThe only reason why \"rm -f t && ln s t\" makes sense over \"ln -f s t\"\nis because there could be a leftover 't' directory from a previous\nbuild or rogue testing process or whatever. It avoids creating a\nhardlink at t/s, unlike \"ln -f s t\" which would happily do so. It\nwould let us notice there is something fishy going on by failing to\nremove the stray directory that should not exist.\n\nI do not object to replace \"rm then ln\" with \"ln -f\", as the \"be\ncautious against somebody mistakenly making a directory there\" is\nnot something I find valuable all that much.\n\nBut I do not want to be associated with a commit that claims that\n\"ln -f\" avoids race in \"rm && ln\" ;-).\n\nThanks.\n\n"},{"id":"420533","messageId":"xmqqeefxh8rl.fsf@gitster.g","threadId":"55269","inReplyTo":"patch-6.7-9ada8979890-20210329T162327Z-avarab@gmail.com","subject":"Re: [PATCH 6/6] Makefile: add a INSTALL_FALLBACK_LN_CP mode","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2021-03-29T22:53:18Z","receivedAt":"2021-03-29T22:54:12Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Ævar Arnfjörð Bjarmason writes:\n\n> Change the default behavior on \"make install\" where we fallback\n> through a chain of \"ln || ln -s || cp\" to instead error out when we\n> can't symlink or hardlink, and not then fallback on a \"cp\" (or from a\n> symlink to hardlink etc.).\n>\n> The fallback behavior was introduced in 3e073dc5611 (Makefile: always\n> provide a fallback when hardlinks fail, 2008-08-25), since then we've\n> gained the ability to specify e.g. that we'd like symlinks via the\n> INSTALL_SYMLINKS setting.\n\nHmph, I am not so sure. \"Use hardlink if we can, as that would not\nconsume inode, but where hardlinks cannot be used, it is OK to use\nsymlink, and I do not want to waste disk blocks with cp\" is probably\none of the sensible wishes, but at least without \"ln || ln -s\" fallback,\nyou cannot do that, no?\n\nSo I would understand if there are two orthogonal knobs\n\n - the order of preference (e.g. hardlink > symlink > copy)\n - which ones are allowed (e.g. \"no symlinks please\")\n\nbut I cannot quite imagine how a system without any fallback would\nbe useful.\n\n> +main_no_fallbacks () {\n> +\tif test -n \"$no_install_hardlinks\" -a -z \"$install_symlinks\"\n\nAs the values of these variables are (presumably) tightly under our\ncontrol, the use of -a/-o with test may be safe in these examples,\nbut to avoid letting clueless shell script newbies to cargo cult\nthis code, let's use the safer \"test -n A && test -z B\" form.\n"},{"id":"420535","messageId":"xmqq7dlph823.fsf@gitster.g","threadId":"55269","inReplyTo":"cover-0.7-00000000000-20210329T162327Z-avarab@gmail.com","subject":"Re: [PATCH 0/6] Makefile: make non-symlink & non-hardlink install sane","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2021-03-29T23:08:36Z","receivedAt":"2021-03-29T23:09:42Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Ævar Arnfjörð Bjarmason writes:\n\n> This is on top of my just-submitted \"Makefile: don't die on AIX with\n> open ./git\" series: [1]\n>\n> This series introduces no \"real\" behavior changes I'd expect anyone to\n> notice, but refactors a lengthy copy/pasted test/if/else in the\n> Makefile into a simple helper script.\n>\n> The \"real\" behavior change is that we no longer ask the user how\n> they'd like to install (symlinks, hardlinks, neither?) and then\n> proceed to ignore what was asked of us and optimistically fallback in\n> case of errors. I.e. the inability to create symlinks or hardlinks.\n>\n> Instead we'll just die, the old behavior is still available as\n> INSTALL_FALLBACK_LN_CP. In practice I think exactly nobody actually\n> wanted the existing behavior.\n\nI read most of the patches in the series.\n\nWhile I like the general direction to allow builders/installers more\nprecisely to specify what methods of installation are preferred, I\ndo not know if I agree with the exact execution. It does not help\nthat the series is built on top of that \"generate in temporary and\nmove to the final, even for $(CC)\" series, most of which I do not\nthink I agree with.\n\nThanks.\n"},{"id":"420537","messageId":"87ft0dmtkw.fsf@evledraar.gmail.com","threadId":"55269","inReplyTo":"xmqqy2e5kegv.fsf@gitster.g","subject":"Re: [PATCH v2 1/5] Makefile: rename objects in-place, don't clobber","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2021-03-29T23:24:47Z","receivedAt":"2021-03-29T23:25:25Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"\nOn Mon, Mar 29 2021, Junio C Hamano wrote:\n\n> Ævar Arnfjörð Bjarmason writes:\n>\n>> Per the log of changes to the Makfile and Junio's recent comment about\n>> [1] why that pattern got introduced it was for a different reason\n>> entirely, i.e. (\"[]\" edits are mine, for brevity):\n>>\n>> [T]hat age old convention [...] is spelled [as]:\n>>\n>> \tthing:\n>> \t\trm -f thing thing+\n>> \t\tprepare contents for thing >thing+\n>\n> Did I say that? I recall I specifically avoided the \"redirection\"\n> because this is *NOT* shell-script only principle. If a command is\n> so broken that \"cmd -o thing\" that fails to work correctly leaves a\n> broken output in thing, then the pattern should be applied and made\n> to \"cmd -o thing+ && mv thing+ thing\".\n\n[I see this was already noted downthread...]\n\n> On the other hand, if \"cmd\" refrains from leaving a half-baked\n> result in \"-o thing\" (and I believe $(CC) is well-behaved even on\n> AIX), I do not think it is a good idea to use that pattern. One\n> version of AIX may refuse to overwrite a file in use because\n> creat(\"thing\") that is necessary for \"-o thing\" may fail while\n> \"thing\" is in use), but another system may refuse to rename over a\n> file in use (i.e. \"-o thing+\" into a brand new \"thing+\" may be OK\n> but the final \"mv thing+ thing\" may fail). So pretending that it is\n> a cure for broken use case is cluttering Makefile for no real\n> benefit and leading readers into confused thinking.\n\nIt does fix this issue entirely on AIX. So it's a cure for the broken\ncase.\n\nI can assure you that not having to regularly log in to the GCC farm AIX\nbox and remember how to deal with IBM's ps/kill etc. just to do another\nbuild/test is a real benefit :)\n\nWhereas maybe there's another system we're not fixing with this patch,\nbut does it matter? I don't see how it would make things worse for that\nOS, if it exists. But it sounds like it's just a hypothetical.\n\n>> \t\tmv thing+ thing\n>>\n>> It protects us from a failure mode where \"prepare contents for\n>> thing\" step is broken and leaves a \"thing\" that does not work, but\n>> confuses make that make does not need to rebuild it, if you wrote it\n>> as such:\n>>\n>> \tthing:\n>> \t\tprepare contents for thing >thing\n>>\n>> [It might leave behind a corrupt 'thing'.] In any case, it is not\n>> \"we are trying to make thing available while it is being\n>> rewritten\" at all.\n>>\n>> That makes perfect sense for shellscripts, but as this change shows\n>> there's other good reasons to use this age old convention that weren't\n>> considered at the time.\n>\n> So, no, the age old convention may have considered and does not\n> apply to such a use case.\n\nThe reason I mentioned this was to specifically address the implied \"why\nwould we need this?\" part of your E-Mail that you're quoting.\n\nI think that started out as us talking past one another, so let me try\nto summarize.\n\nI was basically saying \"here's a well-known command idiom\" that can be\nused for XYZ.\n\nI think you're basically saying \"in git.git, I introduced this idiom to\ndeal with problem ABC\".\n\nABC and XYZ aren't incompatible. I'm just saying this can and does solve\nboth problems[continued below].\n\n>> git$X: git.o GIT-LDFLAGS $(BUILTIN_OBJS) $(GITLIBS)\n>> -\t$(QUIET_LINK)$(CC) $(ALL_CFLAGS) -o $@ $(ALL_LDFLAGS) \\\n>> -\t\t$(filter %.o,$^) $(LIBS)\n>> +\t$(QUIET_LINK)$(CC) $(ALL_CFLAGS) -o $@+ $(ALL_LDFLAGS) \\\n>> +\t\t$(filter %.o,$^) $(LIBS) && \\\n>> +\tmv $@+ $@\n>\n> Really, does anybody else use \"$(CC) -o $@\" in such a way in their\n> Makefile? Having to do this smells simply crazy (I am not saying\n> you are crazy---the platform that forces you to write such a thing\n> is crazy).\n\nYes, if you do say a Google search for \"Cannot open or remove a file\ncontaining a running program\" you'll find that there's 15k results of\npeople basically (re)discovering this problem in porting their software\nto AIX, and the solutions being some variant of \"yes AIX sucks, just use\nthis 'cmd >x+ && mv x+ x' trick\".\n\nYou can also invoke a \"slibclean\" program to reticulate AIX's splines,\nbut I thought this sucked less.\n"},{"id":"420538","messageId":"87czvhmtfi.fsf@evledraar.gmail.com","threadId":"55269","inReplyTo":"xmqqk0ppkdmd.fsf@gitster.g","subject":"Re: [PATCH v2 2/5] Makefile: rename scripts in-place, don't clobber","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2021-03-29T23:28:01Z","receivedAt":"2021-03-29T23:29:09Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"\nOn Mon, Mar 29 2021, Junio C Hamano wrote:\n\n> Ævar Arnfjörð Bjarmason writes:\n>\n>> It'll also guard against any weird issues with e.g. a command that\n>> wants to read GIT-PERL-DEFINES to generate one of the %.perl scripts\n>> racing with either a concurrent instance of \"make\" that has partially\n>> updated the file, or test-lib.sh dying with some particularly weird\n>> error because GIT-BUILD-OPTIONS was partway through being updated when\n>> it ran.\n>\n> If something like that happens, doesn't that indicate a bug in the\n> dependency graph in the Makefile or the implementation of \"make\"?\n> The generated file is depended on for the consumer to be able to use\n> a non-stale version---so the consumer should not start before the\n> generator finishes.\n\nIf everything we were building in the Makefile would be invoked via\nother Makefile rules, yes. But if I run say (cd t && ./t0000-basic.sh)\nI'm having test-lib.sh pick up one of those (possibly partial) files,\nthis guarantees they'll all be atomically updated.\n\n> You may be able to hide the breakage coming from \"partially written\n> file is easily recognizable and the consumer would barf\". But I am\n> afraid that you are introducing a problem harder to diagnose, i.e.\n> \"the consumer reads from a complete file so there is no syntax error\n> or other things that easily tells you there is a breakage, but what\n> is used is stale and not up to date\".\n>\n> The same comment applies to this step. I do not think it would\n> break (other than adding intermediate crufts) the result if you\n> generate into temporary and rename to final, but it is not clear\n> to me what the point is in doing so.\n\nI just think it makes sense to do this for consistency. So on \"master\"\nwe've got 65 hits for $@+ in the Makefile, at the end of this saga we've\ngot 100.\n\nI think being consistent across the board makes things easier to read,\nand in combination with the later \".DELETE_ON_ERROR\" we can also drop\nother copy/paste cruft.\n"},{"id":"420539","messageId":"87a6qlmt9f.fsf@evledraar.gmail.com","threadId":"55269","inReplyTo":"xmqqblb1kd47.fsf@gitster.g","subject":"Re: [PATCH v2 4/5] Makefile: add the \".DELETE_ON_ERROR\" flag","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2021-03-29T23:31:40Z","receivedAt":"2021-03-29T23:32:21Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"\nOn Mon, Mar 29 2021, Junio C Hamano wrote:\n\n> Ævar Arnfjörð Bjarmason writes:\n>\n>> Use the GNU make \".DELETE_ON_ERROR\" flag.\n>\n> Yay!\n>\n> With versions of $(MAKE) where this feature is available, it is far\n> more preferrable to use it than \"generate into temporary and rename\n> to the final\" dance.\n>\n> We do require / depend on GNU but I do not offhand know if this is\n> available in all versions that still matter. If we know we can\n> assume the presence of this feature the I do not mind if we jump\n> directly to this step without those \"do the same for $(CC)\" steps\n> (which I deem crazy) before it.\n\nEven if it's not available in all versions that's OK. You just won't get\nthe nicer removal behavior on on error on such a jurassic gmake, but\nyou'll probably have way bigger issues with your late-90s-era software.\n\nIt's not a syntax error on a gmake or other make that doesn't know about\nit either, i.e. you can also add a target like:\n\n .THIS_DOES_NOT_EXIST_AS_A_GMAKE_FEATURE:\n\nAnd gmake willl happily ignore it.\n"},{"id":"420543","messageId":"xmqqpmzhfr65.fsf@gitster.g","threadId":"55269","inReplyTo":"87a6qlmt9f.fsf@evledraar.gmail.com","subject":"Re: [PATCH v2 4/5] Makefile: add the \".DELETE_ON_ERROR\" flag","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2021-03-29T23:58:42Z","receivedAt":"2021-03-29T23:59:42Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Ævar Arnfjörð Bjarmason writes:\n\n> Even if it's not available in all versions that's OK. You just won't get\n> the nicer removal behavior on on error on such a jurassic gmake, but\n> you'll probably have way bigger issues with your late-90s-era software.\n>\n> It's not a syntax error on a gmake or other make that doesn't know about\n> it either, i.e. you can also add a target like:\n>\n> .THIS_DOES_NOT_EXIST_AS_A_GMAKE_FEATURE:\n>\n> And gmake willl happily ignore it.\n\nWhat I meant was that I would not tolerate \"cc -o x+ && mv x+ x\",\nbut I do not mind DELETE_ON_ERROR with \"cc -o x\" at all.\n"},{"id":"420550","messageId":"xmqqh7ktfq3y.fsf@gitster.g","threadId":"55269","inReplyTo":"87ft0dmtkw.fsf@evledraar.gmail.com","subject":"Re: [PATCH v2 1/5] Makefile: rename objects in-place, don't clobber","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2021-03-30T00:21:37Z","receivedAt":"2021-03-30T00:22:32Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Ævar Arnfjörð Bjarmason writes:\n\n>> Really, does anybody else use \"$(CC) -o $@\" in such a way in their\n>> Makefile? Having to do this smells simply crazy (I am not saying\n>> you are crazy---the platform that forces you to write such a thing\n>> is crazy).\n>\n> Yes, if you do say a Google search for \"Cannot open or remove a file\n> containing a running program\" you'll find that there's 15k results of\n> people basically (re)discovering this problem in porting their software\n> to AIX, and the solutions being some variant of \"yes AIX sucks, just use\n> this 'cmd >x+ && mv x+ x' trick\".\n\nWhat I meant was if there are well known upstream projects whose\nMakefile actually use\n\n\t$(CC) -o $@+ ...\n\tmv $@+ $@\n\nI wouldn't be surprised if AIX community maintained collections of\npatches to many projects to turn\n\n\t$(CC) -o $@ ...\n\nin the Makefiles taken from upstream projects into\n\n\t$(CC) -o $@+ ...\n\tmv $@+ $@\n\nto work AIX around. As an upstream, however, I am not interested in\nforcing that pattern on users of other platforms.\n\nIn any case, I do not care too much about the \"I am building a new\nbinary while running, without installing, the one I built\" use case\nand do not agree with the idea of making the Makefile ugly only to\nsupport such a use case. That is where my comments are coming from\non this topic. FWIW, AIX developers who do not do the \"build, run\nwithout installing, and rebuild while the old one is still running\"\nwill not need the \"$(CC) -o $@+ && mv $@+ $@\" either, right?\n\n\n\n"},{"id":"420621","messageId":"YGM/i+k4r0q8FKg2@coredump.intra.peff.net","threadId":"55269","inReplyTo":"87a6qlmt9f.fsf@evledraar.gmail.com","subject":"Re: [PATCH v2 4/5] Makefile: add the \".DELETE_ON_ERROR\" flag","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2021-03-30T15:11:07Z","receivedAt":"2021-03-30T15:11:50Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Tue, Mar 30, 2021 at 01:31:40AM +0200, Ævar Arnfjörð Bjarmason wrote:\n\n> >> Use the GNU make \".DELETE_ON_ERROR\" flag.\n> >\n> > Yay!\n> >\n> > With versions of $(MAKE) where this feature is available, it is far\n> > more preferrable to use it than \"generate into temporary and rename\n> > to the final\" dance.\n> >\n> > We do require / depend on GNU but I do not offhand know if this is\n> > available in all versions that still matter. If we know we can\n> > assume the presence of this feature the I do not mind if we jump\n> > directly to this step without those \"do the same for $(CC)\" steps\n> > (which I deem crazy) before it.\n> \n> Even if it's not available in all versions that's OK. You just won't get\n> the nicer removal behavior on on error on such a jurassic gmake, but\n> you'll probably have way bigger issues with your late-90s-era software.\n> \n> It's not a syntax error on a gmake or other make that doesn't know about\n> it either, i.e. you can also add a target like:\n> \n> .THIS_DOES_NOT_EXIST_AS_A_GMAKE_FEATURE:\n> \n> And gmake willl happily ignore it.\n\nYes, I think it's fine to treat it as \"nice if we have it, but OK\notherwise\". But also, .DELETE_ON_ERROR first shows up in the GNU make\ngit repository in version 3.71.5 from 1994. So I think we can probably\njust assume it's everywhere.\n\n-Peff\n"},{"id":"420622","messageId":"YGNBoljqknW+Kpif@coredump.intra.peff.net","threadId":"55269","inReplyTo":"xmqqsg4dha3i.fsf@gitster.g","subject":"Re: [PATCH 3/6] Makefile: refactor out \"ln || ln -s || cp\" pattern","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2021-03-30T15:20:02Z","receivedAt":"2021-03-30T15:20:59Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Mon, Mar 29, 2021 at 03:24:33PM -0700, Junio C Hamano wrote:\n\n> Ævar Arnfjörð Bjarmason writes:\n> \n> > Refactor out the hard-to-read and maintain \"ln || ln -s || cp\"\n> > pattern.\n> >\n> > This was initially added in 3e073dc5611 (Makefile: always provide a\n> > fallback when hardlinks fail, 2008-08-25), but since then it's become\n> > a lot more complex as we've added:\n> >\n> > * 3426e34fedd (Add NO_CROSS_DIRECTORY_HARDLINKS support to the\n> > Makefile, 2009-05-11)\n> >\n> > * NO_INSTALL_HARDLINKS in 70de5e65e8c (Makefile:\n> > NO_INSTALL_HARDLINKS, 2012-05-02)\n> >\n> > * INSTALL_SYMLINKS in ad874608d8c (Makefile: optionally symlink\n> > libexec/git-core binaries to bin/git, 2018-03-13)\n> >\n> > * SKIP_DASHED_BUILT_INS 179227d6e21 (Optionally skip linking/copying\n> > the built-ins, 2020-09-21)\n> >\n> > Each of those commits had to add a new special-case to this code,\n> > resulting in quite an unmaintainable mess for adding any sort of new\n> > option.\n> >\n> > Let's use the newly introduced ln-or-cp.sh script instead, note that\n> > we only sometimes pass the --no-cross-directory-hardlinks option, per\n> > the previous behavior. The target of the \"ln -s\" is also another\n> > special snowflake, but we're careful to carry that forward.\n> \n> Nice. These explicit command-line options to the helper may be\n> harder to initially write and maintain than just exporting the\n> relevant $(MAKE) macros and using it from the helper, but once\n> it works correctly, it is much easier to see what is going on.\n\nAnother option is to make \"ln-or-cp\" itself a target that \"make\" knows\nhow to build, and bake the values into its \"built\" version. Besides\nmaking the invocations a bit shorter, it also means that the dependency\ngraph is more correct. If a rule invokes ln-or-cp, its behavior will\nchange if $NO_INSTALL_HARDLINKS changes, for example. Telling that to\nmake requires depending on a sentinel file in each such caller (like\nGIT-BUILD-OPTIONS). Whereas we could do that once for the \"ln-or-cp\"\ntarget, and then everyone who uses it just depends on it.\n\nI had a series a long time ago that moved the whole Makefile in that\ndirection, but I got nervous that it was too disruptive and too\nnon-idiomatic to be worth pursuing. So I offer the alternative here\nmostly as food for thought. It may not be a good direction (and we\nalready have good-enough solutions, like depending on\nGIT-BUILD-OPTIONS).\n\n-Peff\n"},{"id":"420635","messageId":"xmqqblb0eben.fsf@gitster.g","threadId":"55269","inReplyTo":"YGNBoljqknW+Kpif@coredump.intra.peff.net","subject":"Re: [PATCH 3/6] Makefile: refactor out \"ln || ln -s || cp\" pattern","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2021-03-30T18:36:48Z","receivedAt":"2021-03-30T18:38:29Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jeff King writes:\n\n>> Nice. These explicit command-line options to the helper may be\n>> harder to initially write and maintain than just exporting the\n>> relevant $(MAKE) macros and using it from the helper, but once\n>> it works correctly, it is much easier to see what is going on.\n>\n> Another option is to make \"ln-or-cp\" itself a target that \"make\" knows\n> how to build, and bake the values into its \"built\" version.\n\nAha. That is even nicer ;-).\n"},{"id":"420636","messageId":"xmqqft0cebft.fsf@gitster.g","threadId":"55269","inReplyTo":"YGM/i+k4r0q8FKg2@coredump.intra.peff.net","subject":"Re: [PATCH v2 4/5] Makefile: add the \".DELETE_ON_ERROR\" flag","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2021-03-30T18:36:06Z","receivedAt":"2021-03-30T18:43:48Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jeff King writes:\n\n> On Tue, Mar 30, 2021 at 01:31:40AM +0200, Ævar Arnfjörð Bjarmason wrote:\n>\n>> >> Use the GNU make \".DELETE_ON_ERROR\" flag.\n>> >\n>> > Yay!\n>> >\n>> > With versions of $(MAKE) where this feature is available, it is far\n>> > more preferrable to use it than \"generate into temporary and rename\n>> > to the final\" dance.\n>> >\n>> > We do require / depend on GNU but I do not offhand know if this is\n>> > available in all versions that still matter. If we know we can\n>> > assume the presence of this feature the I do not mind if we jump\n>> > directly to this step without those \"do the same for $(CC)\" steps\n>> > (which I deem crazy) before it.\n>> \n>> Even if it's not available in all versions that's OK. You just won't get\n>> the nicer removal behavior on on error on such a jurassic gmake, but\n>> you'll probably have way bigger issues with your late-90s-era software.\n>> \n>> It's not a syntax error on a gmake or other make that doesn't know about\n>> it either, i.e. you can also add a target like:\n>> \n>> .THIS_DOES_NOT_EXIST_AS_A_GMAKE_FEATURE:\n>> \n>> And gmake willl happily ignore it.\n>\n> Yes, I think it's fine to treat it as \"nice if we have it, but OK\n> otherwise\". But also, .DELETE_ON_ERROR first shows up in the GNU make\n> git repository in version 3.71.5 from 1994. So I think we can probably\n> just assume it's everywhere.\n\nOK. That raises my hopes up that we may be able to simplify things\nlike this\n\n config-list.h:\n $(QUIET_GEN)$(SHELL_PATH) ./generate-configlist.sh \\\n >$@+ && mv $@+ $@\n\ninto\n\n config-list.h:\n $(QUIET_GEN)$(SHELL_PATH) ./generate-configlist.sh >$@\n\n"},{"id":"420691","messageId":"YGQdpkHAcFR/zNOx@coredump.intra.peff.net","threadId":"55269","inReplyTo":"xmqqft0cebft.fsf@gitster.g","subject":"Re: [PATCH v2 4/5] Makefile: add the \".DELETE_ON_ERROR\" flag","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2021-03-31T06:58:46Z","receivedAt":"2021-03-31T06:59:48Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Tue, Mar 30, 2021 at 11:36:06AM -0700, Junio C Hamano wrote:\n\n> > Yes, I think it's fine to treat it as \"nice if we have it, but OK\n> > otherwise\". But also, .DELETE_ON_ERROR first shows up in the GNU make\n> > git repository in version 3.71.5 from 1994. So I think we can probably\n> > just assume it's everywhere.\n> \n> OK. That raises my hopes up that we may be able to simplify things\n> like this\n> \n> config-list.h:\n> $(QUIET_GEN)$(SHELL_PATH) ./generate-configlist.sh \\\n> >$@+ && mv $@+ $@\n> \n> into\n> \n> config-list.h:\n> $(QUIET_GEN)$(SHELL_PATH) ./generate-configlist.sh >$@\n\nYes, I think .DELETE_ON_ERROR would accomplish roughly the same thing.\nThough note that it's essentially a \"rollback\" strategy. We create the\nbroken file, realize there was an error, and the roll it back (as\nopposed to the \"mv\" strategy, which confirms everything is good before\ncommitting it into place). This is worse than a \"commit\" strategy in a\nfew ways:\n\n - somebody else may racily see the broken state. I'm not too concerned\n with this, and from the rest of the thread I don't think you are\n either.\n\n - we may be left in the broken state if the rollback fails. Plausible\n reasons are: somebody kills \"make\" (I'd hope on the obvious ^C that\n it catches the signal and deletes before exiting), bug in make,\n transient OS error, power failure, etc.\n\nI don't know how paranoid we want to be about this, especially the\nlatter. My general inclination is to prefer \"commit\" systems as more\nrobust, but it is just a Makefile.\n\n-Peff\n"},{"id":"420706","messageId":"87v997l8sl.fsf@evledraar.gmail.com","threadId":"55269","inReplyTo":"xmqqeefxh8rl.fsf@gitster.g","subject":"Re: [PATCH 6/6] Makefile: add a INSTALL_FALLBACK_LN_CP mode","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2021-03-31T14:03:38Z","receivedAt":"2021-03-31T14:04:14Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"\nOn Tue, Mar 30 2021, Junio C Hamano wrote:\n\n> Ævar Arnfjörð Bjarmason writes:\n>\n>> Change the default behavior on \"make install\" where we fallback\n>> through a chain of \"ln || ln -s || cp\" to instead error out when we\n>> can't symlink or hardlink, and not then fallback on a \"cp\" (or from a\n>> symlink to hardlink etc.).\n>>\n>> The fallback behavior was introduced in 3e073dc5611 (Makefile: always\n>> provide a fallback when hardlinks fail, 2008-08-25), since then we've\n>> gained the ability to specify e.g. that we'd like symlinks via the\n>> INSTALL_SYMLINKS setting.\n>\n> Hmph, I am not so sure. \"Use hardlink if we can, as that would not\n> consume inode, but where hardlinks cannot be used, it is OK to use\n> symlink, and I do not want to waste disk blocks with cp\" is probably\n> one of the sensible wishes, but at least without \"ln || ln -s\" fallback,\n> you cannot do that, no?\n>\n> So I would understand if there are two orthogonal knobs\n>\n> - the order of preference (e.g. hardlink > symlink > copy)\n> - which ones are allowed (e.g. \"no symlinks please\")\n>\n> but I cannot quite imagine how a system without any fallback would\n> be useful.\n\nBecause with explicit knobs I'd like to tell it what to do and not have\nit auto-guess. E.g. if I say I want openssl I don't want it to see it's\nnot there and auto-fallback on gnutls or whatever.\n\nThe same for \"I want hardlinks/symlinks\", usually someone picking one is\nbuilding a package, and under a lot of packaging formats that difference\nreally matters, and either won't be notinced in time or will break\nfurther down the chain.\n\n>> +main_no_fallbacks () {\n>> +\tif test -n \"$no_install_hardlinks\" -a -z \"$install_symlinks\"\n>\n> As the values of these variables are (presumably) tightly under our\n> control, the use of -a/-o with test may be safe in these examples,\n> but to avoid letting clueless shell script newbies to cargo cult\n> this code, let's use the safer \"test -n A && test -z B\" form.\n\n*nod*\n"},{"id":"420707","messageId":"87sg4bl8qn.fsf@evledraar.gmail.com","threadId":"55269","inReplyTo":"xmqqo8f1h9ra.fsf@gitster.g","subject":"Re: [PATCH 4/6] Makefile: make INSTALL_SYMLINKS affect the build directory","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2021-03-31T14:04:48Z","receivedAt":"2021-03-31T14:05:52Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"\nOn Tue, Mar 30 2021, Junio C Hamano wrote:\n\n> Ævar Arnfjörð Bjarmason writes:\n>\n>> Change the INSTALL_SYMLINKS flag to also affect whether a built-in\n>> like e.g. git-fetch is a symlink or not in git's build directory.\n>>\n>> This doesn't matter for anything other than as an aid to developers\n>> who might be confused about their build not matching the installation,\n>> and who'd like to be reminded that e.g. \"git-fetch\" is a built-in by\n>> \"ls\" coloring appropriately it as a symlink.\n>\n> I am not with the cause and hence not very interested in this\n> \"feature\".\n>\n> When there are multiple possible reasons why something is made into\n> a symbolic link, the symlink-ness in the build directory cannot\n> fundamentally mirror the symlink-ness in the installation, no?\n> \"git\" and \"git-fetch\" may be in the same directory in the build, but\n> their installation directories are different, so they may be\n> hardlinked in the former but they may be turned into symlinks in the\n> latter.\n\nThis won't be the case after 6/6 in INSTALL_FALLBACK_LN_CP. I'll make\nsome note of it here.\n\nIn practice I think this fallback mode is useful to almost nobody, so\nbeing able to have the build directory mirror the install for\ndevelopment purposes makes things much more obvious.\n"},{"id":"420708","messageId":"87pmzfl861.fsf@evledraar.gmail.com","threadId":"55269","inReplyTo":"xmqqh7ktfq3y.fsf@gitster.g","subject":"Re: [PATCH v2 1/5] Makefile: rename objects in-place, don't clobber","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2021-03-31T14:17:10Z","receivedAt":"2021-03-31T14:17:48Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"\nOn Tue, Mar 30 2021, Junio C Hamano wrote:\n\n> Ævar Arnfjörð Bjarmason writes:\n>\n>>> Really, does anybody else use \"$(CC) -o $@\" in such a way in their\n>>> Makefile? Having to do this smells simply crazy (I am not saying\n>>> you are crazy---the platform that forces you to write such a thing\n>>> is crazy).\n>>\n>> Yes, if you do say a Google search for \"Cannot open or remove a file\n>> containing a running program\" you'll find that there's 15k results of\n>> people basically (re)discovering this problem in porting their software\n>> to AIX, and the solutions being some variant of \"yes AIX sucks, just use\n>> this 'cmd >x+ && mv x+ x' trick\".\n>\n> What I meant was if there are well known upstream projects whose\n> Makefile actually use\n>\n> \t$(CC) -o $@+ ...\n> \tmv $@+ $@\n>\n> I wouldn't be surprised if AIX community maintained collections of\n> patches to many projects to turn\n>\n> \t$(CC) -o $@ ...\n>\n> in the Makefiles taken from upstream projects into\n>\n> \t$(CC) -o $@+ ...\n> \tmv $@+ $@\n>\n> to work AIX around. As an upstream, however, I am not interested in\n> forcing that pattern on users of other platforms.\n\nWho's going to notice or care? We have some mixture of clobbering, \"mv\n$@+ $@\" etc. now in our Makefile for various rules and I think unless\nyou're debugging those specific rules you won't notice.\n\nThe case of the $@+ being left behind is quite obscure, and with *+ in\nour .gitignore won't be noticed (e.g. by a \"git add .\" or something).\n\n> In any case, I do not care too much about the \"I am building a new\n> binary while running, without installing, the one I built\" use case\n> and do not agree with the idea of making the Makefile ugly only to\n> support such a use case. That is where my comments are coming from\n> on this topic. FWIW, AIX developers who do not do the \"build, run\n> without installing, and rebuild while the old one is still running\"\n> will not need the \"$(CC) -o $@+ && mv $@+ $@\" either, right?\n\nI daresay that uses cases of:\n\n * The tests break, you login to the CI to run gdb, fix a small bug,\n compile (doesn't work), but being forced to close that gdb session\n would be annoying (e.g. maybe I'm just looking at a data in a struct\n I didn't change).\n\n * Ditto, but maybe debugging two things at the same time, having an\n open \"cat-file --batch\" session etc.\n\nAren't something obscure to someone wanting to work on a codebase. I'm\nsubmitting these because this is an active impediment to me submitting\nportability patches on AIX, of which I already have some:\n\n git log --grep=AIX --author=Ævar origin/master\n\nAnything that makes that less painful is a win, and the tiny amount of\nMakefile complexity seems worth it to me.\n"},{"id":"420719","messageId":"xmqq35wb891a.fsf@gitster.g","threadId":"55269","inReplyTo":"YGQdpkHAcFR/zNOx@coredump.intra.peff.net","subject":"Re: [PATCH v2 4/5] Makefile: add the \".DELETE_ON_ERROR\" flag","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2021-03-31T18:36:49Z","receivedAt":"2021-03-31T18:37:24Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jeff King writes:\n\n> I don't know how paranoid we want to be about this, especially the\n> latter. My general inclination is to prefer \"commit\" systems as more\n> robust, but it is just a Makefile.\n\n;-)\n\nAs an old timer, I've written \"gen >$@+ && mv $@+ $@\" all the time\nmyself, but it is ugly and felt a bit too conservative. I do not\nthink it is wise to overnight remove all the existing \"generate in\ntemporary, move to the final\" patterns and delegate $(MAKE) to take\ncare of failed generator with this mechanism, but I actually would\nfeel it probably gives us a cleaner Makefile in the longer term. At\nleast \"bugs in $(MAKE)\" won't be our sole problem (i.e. all other\nprojects that rely on the feature would share the incentives to see\nthem fixed).\n\n"},{"id":"420721","messageId":"xmqqr1jv6u2u.fsf@gitster.g","threadId":"55269","inReplyTo":"87v997l8sl.fsf@evledraar.gmail.com","subject":"Re: [PATCH 6/6] Makefile: add a INSTALL_FALLBACK_LN_CP mode","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2021-03-31T18:45:13Z","receivedAt":"2021-03-31T18:46:02Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Ævar Arnfjörð Bjarmason writes:\n\n>> So I would understand if there are two orthogonal knobs\n>>\n>> - the order of preference (e.g. hardlink > symlink > copy)\n>> - which ones are allowed (e.g. \"no symlinks please\")\n>>\n>> but I cannot quite imagine how a system without any fallback would\n>> be useful.\n>\n> Because with explicit knobs I'd like to tell it what to do and not have\n> it auto-guess.\n\nSo how would I explicitly tell \"I want hardlinks for everything\nelse, but use cp when going between /usr/bin and /usr/libexec\"\n(because /usr/bin and /usr/libexec is not on the same filesystem\non this particular box---I'll tell you to use hardlink everywhere\non another box of mine where they reside on the same filesystem)?\n\nYour argument or analogy with openssl does not make much sense to me\nin this case.\n"},{"id":"420722","messageId":"xmqqmtuj6ttn.fsf@gitster.g","threadId":"55269","inReplyTo":"87pmzfl861.fsf@evledraar.gmail.com","subject":"Re: [PATCH v2 1/5] Makefile: rename objects in-place, don't clobber","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2021-03-31T18:50:44Z","receivedAt":"2021-03-31T18:51:24Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Ævar Arnfjörð Bjarmason writes:\n\n>> What I meant was if there are well known upstream projects whose\n>> Makefile actually use\n>>\n>> \t$(CC) -o $@+ ...\n>> \tmv $@+ $@\n\n... or can you find a popular build generator or two that write such\nrules for the final linkage stage (or individual object file\ngeneration step) and argue that all the projects that use the tool\ndo benefit from such a rule because they can run without installing\nand rebuild while the old one is running?\n\nThen I may have found a good reason to believe that some projects\nthought hard and long and came to the same conclusion as you did.\n\nI started from the place where I didn't find your rationale for\nwanting such a construct sensible, and such a finding may tempt me\nto think again. But otherwise...\n\n"},{"id":"420725","messageId":"87h7krkv02.fsf@evledraar.gmail.com","threadId":"55269","inReplyTo":"xmqqr1jv6u2u.fsf@gitster.g","subject":"Re: [PATCH 6/6] Makefile: add a INSTALL_FALLBACK_LN_CP mode","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2021-03-31T19:01:33Z","receivedAt":"2021-03-31T19:02:41Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"\nOn Wed, Mar 31 2021, Junio C Hamano wrote:\n\n> Ævar Arnfjörð Bjarmason writes:\n>\n>>> So I would understand if there are two orthogonal knobs\n>>>\n>>> - the order of preference (e.g. hardlink > symlink > copy)\n>>> - which ones are allowed (e.g. \"no symlinks please\")\n>>>\n>>> but I cannot quite imagine how a system without any fallback would\n>>> be useful.\n>>\n>> Because with explicit knobs I'd like to tell it what to do and not have\n>> it auto-guess.\n>\n> So how would I explicitly tell \"I want hardlinks for everything\n> else, but use cp when going between /usr/bin and /usr/libexec\"\n> (because /usr/bin and /usr/libexec is not on the same filesystem\n> on this particular box---I'll tell you to use hardlink everywhere\n> on another box of mine where they reside on the same filesystem)?\n\nJust as you would now:\n\n make NO_CROSS_DIRECTORY_HARDLINKS=Y install\n\nThat'll get you hardlinks within bin/ and libexec, but copies between\nthem, and no symlinks.\n\nThe behavior change in this series is if your \"ln\" errors out we'll no\nlonger silently plow ahead and e.g. not hardlink *within* bin and\nlibexec in that case.\n\n> Your argument or analogy with openssl does not make much sense to me\n> in this case.\n\nThe point is the Makefile shouldn't be second-guessing explicit\nrequests. That's what defaults are for.\n"},{"id":"420753","messageId":"nycvar.QRO.7.76.6.2104010026440.54@tvgsbejvaqbjf.bet","threadId":"55269","inReplyTo":"xmqq35wb891a.fsf@gitster.g","subject":"Re: [PATCH v2 4/5] Makefile: add the \".DELETE_ON_ERROR\" flag","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2021-03-31T22:29:03Z","receivedAt":"2021-03-31T22:29:55Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi,\n\nOn Wed, 31 Mar 2021, Junio C Hamano wrote:\n\n> Jeff King writes:\n>\n> > I don't know how paranoid we want to be about this, especially the\n> > latter. My general inclination is to prefer \"commit\" systems as more\n> > robust, but it is just a Makefile.\n>\n> ;-)\n>\n> As an old timer, I've written \"gen >$@+ && mv $@+ $@\" all the time\n> myself, but it is ugly and felt a bit too conservative. I do not\n> think it is wise to overnight remove all the existing \"generate in\n> temporary, move to the final\" patterns and delegate $(MAKE) to take\n> care of failed generator with this mechanism, but I actually would\n> feel it probably gives us a cleaner Makefile in the longer term. At\n> least \"bugs in $(MAKE)\" won't be our sole problem (i.e. all other\n> projects that rely on the feature would share the incentives to see\n> them fixed).\n\nAnother point in favor of removing that hack is this: `+` is not a valid\ncharacter in Windows' filenames. It just so _happens_ that Cygwin (and\ntherefore the MSYS2 Bash/`make` we use to compile Git for Windows)\n_pretends_ that it is a valid filename character, but regular Win32\nprograms cannot read/write such files, and it would preclude Git from\nbeing compiled with more native versions of a POSIX shell or `make`.\n\nCiao,\nDscho\n"}]}