| Message ID | 20210818213611.3658076-1-szeder.dev@gmail.com (mailing list archive) |
|---|---|
| State | Accepted |
| Commit | 9a3df9ec5640ac61d63b494c4879d8b8da20d90e |
| Headers | show |
| Series | Makefile: remove archives before manipulating them with 'ar' | expand |
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
On Thu, Aug 19 2021, Junio C Hamano wrote: > 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. Thanks both, and sorry for the late reply. This is in "next" already and scheduled for "master", but FWIW this looks good to me. The breakage with hook.o happens when you build "seen", and then "master". Since master..seen contains a change that moves find_hook() between files we'll get an error like: multiple definition of `find_hook'; libgit.a(hook.o):/home/avar/g/git/hook.c:97: first defined here
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
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. [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(-)