Re: [PATCH] Makefile: remove archives before manipulating them with 'ar'

[Junio C Hamano <gitster@pobox.com>]

SZEDER Gábor <szeder.dev@gmail.com> writes:

> The rules creating the $(LIB_FILE) and $(XDIFF_LIB) archives used to
> be:
>
>   $(QUIET_AR)$(RM) $@ && $(AR) $(ARFLAGS) $@ $^
>
> until commit 7b76d6bf22 (Makefile: add and use the ".DELETE_ON_ERROR"
> flag, 2021-06-29) removed the '$(RM) $@' part, claiming that "we can
> rely on the "c" (create) being present in ARFLAGS", and (I presume)
> assuming that it means that the named archive is created from scratch.
>
> Unfortunately, that's not what the 'c' flag does, it merely "Suppress
> the diagnostic message that is written to standard error by default
> when the archive is created" [1].  Consequently, all object files that
> are already present in an existing archive and are not replaced will
> remain there.  This leads to linker errors in back-to-back builds of
> different revisions without a 'make clean' between them if source
> files going into these archives are renamed in between:
>
>   # The last commit renaming files that go into 'libgit.a':
>   # bc62692757 (hash-lookup: rename from sha1-lookup, 2020-12-31)
>   #  sha1-lookup.c => hash-lookup.c | 14 +++++++-------
>   #  sha1-lookup.h => hash-lookup.h | 12 ++++++------
>   $ git checkout bc62692757^
>   HEAD is now at 7a7d992d0d sha1-lookup: rename `sha1_pos()` as `hash_pos()`
>   $ make
>   [...]
>   $ git checkout 7b76d6bf22
>   HEAD is now at 7b76d6bf22 Makefile: add and use the ".DELETE_ON_ERROR" flag
>   $ make
>   [...]
>       AR libgit.a
>       LINK git
>   /usr/bin/ld: libgit.a(hash-lookup.o): in function `bsearch_hash':
>   /home/szeder/src/git/hash-lookup.c:105: multiple definition of `bsearch_hash'; libgit.a(sha1-lookup.o):/home/szeder/src/git/sha1-lookup.c:105: first defined here
>   collect2: error: ld returned 1 exit status
>   make: *** [Makefile:2213: git] Error 1
>
> Restore the original make rules to first remove $(LIB_FILE) and
> $(XDIFF_LIB) and then create them from scratch to avoid these build
> errors.

Thanks.  I think I've seen a similar breakage around hook.o but
didn't dig into it.

Will queue.

>
> [1] Quoting POSIX at:
>     https://pubs.opengroup.org/onlinepubs/9699919799/utilities/ar.html
>
> Signed-off-by: SZEDER Gábor <szeder.dev@gmail.com>
> ---
>  Makefile | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/Makefile b/Makefile
> index 157293b555..20a0fe6e88 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -2594,10 +2594,10 @@ $(REMOTE_CURL_PRIMARY): remote-curl.o http.o http-walker.o GIT-LDFLAGS $(GITLIBS
>  		$(CURL_LIBCURL) $(EXPAT_LIBEXPAT) $(LIBS)
>  
>  $(LIB_FILE): $(LIB_OBJS)
> -	$(QUIET_AR)$(AR) $(ARFLAGS) $@ $^
> +	$(QUIET_AR)$(RM) $@ && $(AR) $(ARFLAGS) $@ $^
>  
>  $(XDIFF_LIB): $(XDIFF_OBJS)
> -	$(QUIET_AR)$(AR) $(ARFLAGS) $@ $^
> +	$(QUIET_AR)$(RM) $@ && $(AR) $(ARFLAGS) $@ $^
>  
>  export DEFAULT_EDITOR DEFAULT_PAGER