Makefile: Reorder linker flags in the git executable rule
PATCH 2/3 Makefile: Reorder linker flags in the git executable rule
The XL C compiler can fail due to mixing library path and object file arguments, for example when linking git while building with "gmake LDFLAGS=-L$prefix/lib". This moves the ALL_LDFLAGS variable expansion in the git executable rule to be consistent with all the other linking rules.
--- Makefile | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)
Makefile +2 -2
@@ -1610,8 +1610,8 @@ 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 $@ git.o \ - $(BUILTIN_OBJS) $(ALL_LDFLAGS) $(LIBS) + $(QUIET_LINK)$(CC) $(ALL_CFLAGS) -o $@ $(ALL_LDFLAGS) git.o \ + $(BUILTIN_OBJS) $(LIBS) help.sp help.s help.o: common-cmds.h
Signature · 1 line
1.9.3
Quoted 1 line · David Michael
The XL C compiler can fail due to mixing library path and object
Can you explain in the commit message the actual nature of the failure so that readers can understand more precisely how this change helps?
Quoted 27 lines
file arguments, for example when linking git while building with "gmake LDFLAGS=-L$prefix/lib". This moves the
ALL_LDFLAGSvariable expansion in the git executable rule to be consistent with all the other linking rules.Signed-off-by David Michael <[email protected]>--- Makefile | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)Makefile +2
-3@@ -1610,8 +1610,8 @@ 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 $@ git.o \ - $(BUILTIN_OBJS) $(ALL_LDFLAGS) $(LIBS) + $(QUIET_LINK)$(CC) $(ALL_CFLAGS) -o $@ $(ALL_LDFLAGS) git.o \ + $(BUILTIN_OBJS) $(LIBS) help.sp help.s help.o: common-cmds.h --1.9.3
Quoted 5 lines · Eric Sunshine
Quoted 1 line · David Michael
The XL C compiler can fail due to mixing library path and object
Can you explain in the commit message the actual nature of the failure so that readers can understand more precisely how this change helps?
Based on past experience, it is probably "the compiler complains and refuses to run" (or optionally "the compiler silently ignores your LDFLAGS" depending on how irritating it wants to be). But it would not hurt to be specific.
Either way, the patch looks good; the whole point of LDFLAGS versus LIBS is to make this distinction in command-line positioning.
-Peff
Quoted 5 lines · Jeff King
Quoted 3 lines · Eric Sunshine
Quoted 1 line · David Michael
The XL C compiler can fail due to mixing library path and object
Quoted 7 lines
Quoted 2 lines
Can you explain in the commit message the actual nature of the failure so that readers can understand more precisely how this change helps?
Based on past experience, it is probably "the compiler complains and refuses to run" (or optionally "the compiler silently ignores your LDFLAGS" depending on how irritating it wants to be). But it would not hurt to be specific.
Yes, the compiler refuses to run by default when a "-L" option occurs after a source/object file. It tries to interpret it as another file name and fails.
I believe I can work around the error with an "export _C89_CCMODE=1", but I thought I'd send the patch since this is the only occurrence of the problem, and the argument order is inconsistent with other linker commands in the file.
IBM documentation has this to say on the noted environment variable:
"The default behavior of the c89/cc/c++ command is to expect all options to precede all operands. Setting this variable allows compatibility with historical implementations (other cc commands). When set to 1, the c89/cc/c++ command operates as follows: Options and operands can be interspersed. [...]"
Do you want me to resend the patch and reference the IBM documentation in the message?
Thanks.
David
Quoted 3 lines · David Michael
Yes, the compiler refuses to run by default when a "-L" option occurs after a source/object file. It tries to interpret it as another file name and fails.
Yeah, I think I have seen similar behavior before, but it has been long enough that I no longer remember the compiler in use.
Quoted 4 lines
I believe I can work around the error with an "export _C89_CCMODE=1", but I thought I'd send the patch since this is the only occurrence of the problem, and the argument order is inconsistent with other linker commands in the file.
I don't think working around it makes sense. That would fix your case, but nobody else's (though given how long it has been that way without complaints, I suspect any other compilers this picky may have died off).
Quoted 2 lines
Do you want me to resend the patch and reference the IBM documentation in the message?
I don't think you need to. More interesting than documentation is the real-world breakage you experienced and the analysis of the situation. I'd be fine taking the patch as-is, or if changing anything, mentioning the failure mode in the commit message.
-Peff
Quoted 9 lines · Jeff King
Quoted 3 lines · David Michael
Yes, the compiler refuses to run by default when a "-L" option occurs after a source/object file. It tries to interpret it as another file name and fails.
Yeah, I think I have seen similar behavior before, but it has been long enough that I no longer remember the compiler in use.
Quoted 8 lines
Quoted 4 lines
I believe I can work around the error with an "export _C89_CCMODE=1", but I thought I'd send the patch since this is the only occurrence of the problem, and the argument order is inconsistent with other linker commands in the file.
I don't think working around it makes sense. That would fix your case, but nobody else's (though given how long it has been that way without complaints, I suspect any other compilers this picky may have died off).
I think you meant s/nobody else's/breaks &/;
With that, I agree with your assessment. The diff itself is probably fine as-is (I didn't look at it for more than 10 seconds, though ;-). And I agree that it needs to be better explained.
Quoted 9 lines
Quoted 2 lines
Do you want me to resend the patch and reference the IBM documentation in the message?
I don't think you need to. More interesting than documentation is the real-world breakage you experienced and the analysis of the situation. I'd be fine taking the patch as-is, or if changing anything, mentioning the failure mode in the commit message.
-Peff
Quoted 10 lines · Junio C Hamano
Quoted 8 lines
Quoted 4 lines
I believe I can work around the error with an "export _C89_CCMODE=1", but I thought I'd send the patch since this is the only occurrence of the problem, and the argument order is inconsistent with other linker commands in the file.
I don't think working around it makes sense. That would fix your case, but nobody else's (though given how long it has been that way without complaints, I suspect any other compilers this picky may have died off).
I think you meant s/nobody else's/breaks &/;
I meant "using the _C_89_CCMODE workaround does not help anybody else, because their compiler will not support it; instead we should fix the Makefile as David originally proposed".
I think we are still agreeing, though. :)
-Peff