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@xxxxxxxxxxxxxxxxxxxxxx
Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@xxxxxxxxx>
---
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
