[PATCH v2 1/5] Makefile: rename objects in-place, don't clobber
- From
Ævar Arnfjörð Bjarmason <avarab@gmail.com>
- Date
- Mar 29, 2021, 16:20 UTC
- Message-ID
- <patch-1.6-3330cdbccc0-20210329T161723Z-avarab@gmail.com>
- In-Reply-To
- <cover-0.6-00000000000-20210329T161723Z-avarab@gmail.com>
Use the established "[...] -o $@+ && mv $@+ $@" pattern in the Makefile for those rules that don't use it already.
This improves portability on OS such as AIX that complain if object files in-use are clobbered (but not if they're rm'd, or renamed away). On e.g. AIX 7.2 running a "./git log" in one terminal and trying to clobber the "git" object in another will yield:
$ >git
bash: git: Cannot open or remove a file containing a running program.And trying to recompile "git" will likewise fail:
LINK git
ld: 0711-851 SEVERE ERROR: Output file: git
The file is in use and cannot be overwritten.It's perfectly happy to have the file renamed or removed from under it though:
$ mv git git2
$ >git2
bash: git2: Cannot open or remove a file containing a running program.
$ rm git2
rm: Remove git2? y
$ file git2
file: 0653-900 cannot open git2.On AIX the test suite is still a dumpster fire. I'm running into this because an orphaned "git-daemon" is sometimes left running, causing subsequent compilation to fail without manual intervention. It's also annoying not to be able to run some ad-hoc command using the built git without holding up subsequent compilation until any such programs are stopped.
This pattern of not clobbering, but rather generating a "$@+" object to rename in-place to "$@" has been present in the Makefile since 6a2e50f9dfd (git --version tells which version of git you have., 2005-09-07), it just hasn't been consistently used for all the rules that generated objects.
Per the log of changes to the Makfile and Junio's recent comment about [1] why that pattern got introduced it was for a different reason entirely, i.e. ("[]" edits are mine, for brevity):
[T]hat age old convention [...] is spelled [as]:
thing:
rm -f thing thing+
prepare contents for thing >thing+
mv thing+ thing It protects us from a failure mode where "prepare contents for
thing" step is broken and leaves a "thing" that does not work, but
confuses make that make does not need to rebuild it, if you wrote it
as such: thing:
prepare contents for thing >thing [It might leave behind a corrupt 'thing'.] In any case, it is not
"we are trying to make thing available while it is being
rewritten" at all.That makes perfect sense for shellscripts, but as this change shows there's other good reasons to use this age old convention that weren't considered at the time.
1. http://lore.kernel.org/git/xmqqpn097e9o.fsf@gitster.c.googlers.com
Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com> --- Makefile | 40 +++++++++++++++++++++++++--------------- 1 file changed, 25 insertions(+), 15 deletions(-)
diff --git a/Makefile b/Makefile index 55c8035fa80..b0dbf5888b7 100644 --- a/Makefile +++ b/Makefile @@ -2166,8 +2166,9 @@ git.sp git.s git.o: EXTRA_CPPFLAGS = \ '-DGIT_INFO_PATH="$(infodir_relative_SQ)"' git$X: git.o GIT-LDFLAGS $(BUILTIN_OBJS) $(GITLIBS) - $(QUIET_LINK)$(CC) $(ALL_CFLAGS) -o $@ $(ALL_LDFLAGS) \ - $(filter %.o,$^) $(LIBS) + $(QUIET_LINK)$(CC) $(ALL_CFLAGS) -o $@+ $(ALL_LDFLAGS) \ + $(filter %.o,$^) $(LIBS) && \ + mv $@+ $@ help.sp help.s help.o: command-list.h @@ -2526,18 +2527,22 @@ compat/nedmalloc/nedmalloc.sp: SP_EXTRA_FLAGS += -Wno-non-pointer-null endif git-%$X: %.o GIT-LDFLAGS $(GITLIBS) - $(QUIET_LINK)$(CC) $(ALL_CFLAGS) -o $@ $(ALL_LDFLAGS) $(filter %.o,$^) $(LIBS) + $(QUIET_LINK)$(CC) $(ALL_CFLAGS) -o $@+ $(ALL_LDFLAGS) $(filter %.o,$^) $(LIBS) && \ + mv $@+ $@ git-imap-send$X: imap-send.o $(IMAP_SEND_BUILDDEPS) GIT-LDFLAGS $(GITLIBS) - $(QUIET_LINK)$(CC) $(ALL_CFLAGS) -o $@ $(ALL_LDFLAGS) $(filter %.o,$^) \ - $(IMAP_SEND_LDFLAGS) $(LIBS) + $(QUIET_LINK)$(CC) $(ALL_CFLAGS) -o $@+ $(ALL_LDFLAGS) $(filter %.o,$^) \ + $(IMAP_SEND_LDFLAGS) $(LIBS) && \ + mv $@+ $@ git-http-fetch$X: http.o http-walker.o http-fetch.o GIT-LDFLAGS $(GITLIBS) - $(QUIET_LINK)$(CC) $(ALL_CFLAGS) -o $@ $(ALL_LDFLAGS) $(filter %.o,$^) \ - $(CURL_LIBCURL) $(LIBS) + $(QUIET_LINK)$(CC) $(ALL_CFLAGS) -o $@+ $(ALL_LDFLAGS) $(filter %.o,$^) \ + $(CURL_LIBCURL) $(LIBS) && \ + mv $@+ $@ git-http-push$X: http.o http-push.o GIT-LDFLAGS $(GITLIBS) - $(QUIET_LINK)$(CC) $(ALL_CFLAGS) -o $@ $(ALL_LDFLAGS) $(filter %.o,$^) \ - $(CURL_LIBCURL) $(EXPAT_LIBEXPAT) $(LIBS) + $(QUIET_LINK)$(CC) $(ALL_CFLAGS) -o $@+ $(ALL_LDFLAGS) $(filter %.o,$^) \ + $(CURL_LIBCURL) $(EXPAT_LIBEXPAT) $(LIBS) && \ + mv $@+ $@ $(REMOTE_CURL_ALIASES): $(REMOTE_CURL_PRIMARY) $(QUIET_LNCP)$(RM) $@ && \ @@ -2546,14 +2551,17 @@ $(REMOTE_CURL_ALIASES): $(REMOTE_CURL_PRIMARY) cp $< $@ $(REMOTE_CURL_PRIMARY): remote-curl.o http.o http-walker.o GIT-LDFLAGS $(GITLIBS) - $(QUIET_LINK)$(CC) $(ALL_CFLAGS) -o $@ $(ALL_LDFLAGS) $(filter %.o,$^) \ - $(CURL_LIBCURL) $(EXPAT_LIBEXPAT) $(LIBS) + $(QUIET_LINK)$(CC) $(ALL_CFLAGS) -o $@+ $(ALL_LDFLAGS) $(filter %.o,$^) \ + $(CURL_LIBCURL) $(EXPAT_LIBEXPAT) $(LIBS) && \ + mv $@+ $@ $(LIB_FILE): $(LIB_OBJS) - $(QUIET_AR)$(RM) $@ && $(AR) $(ARFLAGS) $@ $^ + $(QUIET_AR)$(AR) $(ARFLAGS) $@+ $^ && \ + mv $@+ $@ $(XDIFF_LIB): $(XDIFF_OBJS) - $(QUIET_AR)$(RM) $@ && $(AR) $(ARFLAGS) $@ $^ + $(QUIET_AR)$(AR) $(ARFLAGS) $@+ $^&& \ + mv $@+ $@ export DEFAULT_EDITOR DEFAULT_PAGER @@ -2834,7 +2842,8 @@ perf: all t/helper/test-tool$X: $(patsubst %,t/helper/%,$(TEST_BUILTINS_OBJS)) t/helper/test-%$X: t/helper/test-%.o GIT-LDFLAGS $(GITLIBS) - $(QUIET_LINK)$(CC) $(ALL_CFLAGS) -o $@ $(ALL_LDFLAGS) $(filter %.o,$^) $(filter %.a,$^) $(LIBS) + $(QUIET_LINK)$(CC) $(ALL_CFLAGS) -o $@+ $(ALL_LDFLAGS) $(filter %.o,$^) $(filter %.a,$^) $(LIBS) && \ + mv $@+ $@ check-sha1:: t/helper/test-tool$X t/helper/test-sha1.sh @@ -3333,6 +3342,7 @@ FUZZ_CXXFLAGS ?= $(CFLAGS) $(FUZZ_PROGRAMS): all $(QUIET_LINK)$(CXX) $(FUZZ_CXXFLAGS) $(LIB_OBJS) $(BUILTIN_OBJS) \ - $(XDIFF_OBJS) $(EXTLIBS) git.o $@.o $(LIB_FUZZING_ENGINE) -o $@ + $(XDIFF_OBJS) $(EXTLIBS) git.o $@.o $(LIB_FUZZING_ENGINE) -o $@+ && \ + mv $@+ $@ fuzz-all: $(FUZZ_PROGRAMS)
-- 2.31.1.461.gd47399f6574