git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: "Ævar Arnfjörð Bjarmason" <avarab@gmail.com>
To: git@vger.kernel.org
Cc: "Junio C Hamano" <gitster@pobox.com>, "Jeff King" <peff@peff.net>,
	"Johannes Schindelin" <johannes.schindelin@gmx.de>,
	"Jonathan Nieder" <jrnieder@gmail.com>,
	"Ævar Arnfjörð Bjarmason" <avarab@gmail.com>
Subject: [PATCH v2 1/5] Makefile: rename objects in-place, don't clobber
Date: Mon, 29 Mar 2021 18:20:08 +0200	[thread overview]
Message-ID: <patch-1.6-3330cdbccc0-20210329T161723Z-avarab@gmail.com> (raw)
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


  reply	other threads:[~2021-03-29 16:21 UTC|newest]

Thread overview: 48+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-03-07 13:20 [PATCH] Makefile: generate 'git' as 'cc [...] -o git+ && mv git+ git' Ævar Arnfjörð Bjarmason
2021-03-07 20:41 ` Junio C Hamano
2021-03-08 12:38   ` Ævar Arnfjörð Bjarmason
2021-03-08 17:21     ` Junio C Hamano
2021-03-08 18:26     ` Jeff King
2021-03-29 16:20 ` [PATCH v2 0/5] Makefile: don't die on AIX with open ./git Ævar Arnfjörð Bjarmason
2021-03-29 16:20   ` Ævar Arnfjörð Bjarmason [this message]
2021-03-29 18:21     ` [PATCH v2 1/5] Makefile: rename objects in-place, don't clobber Junio C Hamano
2021-03-29 18:26       ` Junio C Hamano
2021-03-29 23:24       ` Ævar Arnfjörð Bjarmason
2021-03-30  0:21         ` Junio C Hamano
2021-03-31 14:17           ` Ævar Arnfjörð Bjarmason
2021-03-31 18:50             ` Junio C Hamano
2021-03-29 16:20   ` [PATCH v2 2/5] Makefile: rename scripts " Ævar Arnfjörð Bjarmason
2021-03-29 18:40     ` Junio C Hamano
2021-03-29 23:28       ` Ævar Arnfjörð Bjarmason
2021-03-29 16:20   ` [PATCH v2 3/5] Makefile: don't needlessly "rm $@ $@+" before "mv $@+ $@" Ævar Arnfjörð Bjarmason
2021-03-29 18:46     ` Junio C Hamano
2021-03-29 16:20   ` [PATCH v2 4/5] Makefile: add the ".DELETE_ON_ERROR" flag Ævar Arnfjörð Bjarmason
2021-03-29 18:51     ` Junio C Hamano
2021-03-29 23:31       ` Ævar Arnfjörð Bjarmason
2021-03-29 23:58         ` Junio C Hamano
2021-03-30 15:11         ` Jeff King
2021-03-30 18:36           ` Junio C Hamano
2021-03-31  6:58             ` Jeff King
2021-03-31 18:36               ` Junio C Hamano
2021-03-31 22:29                 ` Johannes Schindelin
2021-03-29 16:20   ` [PATCH v2 5/5] Makefile: don't "rm configure" before generating it Ævar Arnfjörð Bjarmason
2021-03-29 16:31   ` [PATCH 0/6] Makefile: make non-symlink & non-hardlink install sane Ævar Arnfjörð Bjarmason
2021-03-29 16:31     ` [PATCH 1/6] Makefile: symlink the same way under "symlinks" and "no hardlinks" Ævar Arnfjörð Bjarmason
2021-03-29 22:17       ` Junio C Hamano
2021-03-29 16:31     ` [PATCH 2/6] Makefile: begin refactoring out "ln || ln -s || cp" pattern Ævar Arnfjörð Bjarmason
2021-03-29 22:20       ` Junio C Hamano
2021-03-29 16:31     ` [PATCH 3/6] Makefile: refactor " Ævar Arnfjörð Bjarmason
2021-03-29 22:24       ` Junio C Hamano
2021-03-30 15:20         ` Jeff King
2021-03-30 18:36           ` Junio C Hamano
2021-03-29 16:31     ` [PATCH 4/6] Makefile: make INSTALL_SYMLINKS affect the build directory Ævar Arnfjörð Bjarmason
2021-03-29 22:31       ` Junio C Hamano
2021-03-31 14:04         ` Ævar Arnfjörð Bjarmason
2021-03-29 16:31     ` [PATCH 5/6] Makefile: use "ln -f" instead of "rm && ln" Ævar Arnfjörð Bjarmason
2021-03-29 22:46       ` Junio C Hamano
2021-03-29 16:31     ` [PATCH 6/6] Makefile: add a INSTALL_FALLBACK_LN_CP mode Ævar Arnfjörð Bjarmason
2021-03-29 22:53       ` Junio C Hamano
2021-03-31 14:03         ` Ævar Arnfjörð Bjarmason
2021-03-31 18:45           ` Junio C Hamano
2021-03-31 19:01             ` Ævar Arnfjörð Bjarmason
2021-03-29 23:08     ` [PATCH 0/6] Makefile: make non-symlink & non-hardlink install sane Junio C Hamano

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=patch-1.6-3330cdbccc0-20210329T161723Z-avarab@gmail.com \
    --to=avarab@gmail.com \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    --cc=johannes.schindelin@gmx.de \
    --cc=jrnieder@gmail.com \
    --cc=peff@peff.net \
    --subject='Re: [PATCH v2 1/5] Makefile: rename objects in-place, don'\''t clobber' \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).