From: Ramsay Jones <hidden> Date: 2016-06-15 22:48:59
The msvc debug build (make MSVC=1 DEBUG=1) issues a warning
on every invocation of the linker:
LINK : warning LNK4044: unrecognized option '/Zi'; ignored
In order to suppress the warning, we refrain from passing the
$(ALL_CFLAGS) macro to the linker.
Note that, should it be necessary in the future, an option
intended for both the (front-end) compiler and the linker can
be included in both CFLAGS and LDFLAGS.
Signed-off-by: Ramsay Jones <redacted>
---
Makefile | 14 +++++++-------
1 files changed, 7 insertions(+), 7 deletions(-)
From: Jonathan Nieder <hidden> Date: 2016-06-15 22:49:00
Hi Ramsay,
Ramsay Jones wrote:
LINK : warning LNK4044: unrecognized option '/Zi'; ignored
In order to suppress the warning, we refrain from passing the
$(ALL_CFLAGS) macro to the linker.
Note that, should it be necessary in the future, an option
intended for both the (front-end) compiler and the linker can
be included in both CFLAGS and LDFLAGS.
I think traditionally CPPFLAGS is meant to be used for the purpose
you are describing (see [1] for example).
I realize that the Makefile does not currently use the terms this way:
making it consistent would require
. s/BASIC_CFLAGS/BASIC_CPPFLAGS/, except that the
BASIC_CFLAGS += -Kthread
settings should probably stay as-is
. Windows BASIC_CFLAGS would probably need to be split:
BASIC_CFLAGS = -nologo
BASIC_CPPFLAGS = -I. -I../zlib ... -DWIN32 ...
. s/COMPAT_CFLAGS/COMPAT_CPPFLAGS
What do you think?
Jonathan
[1] http://www.gnu.org/software/autoconf/manual/html_node/Preset-Output-Variables.html#index-CFLAGS-80
From: Peter Harris <hidden> Date: 2016-06-15 22:49:00
On Wed, Jun 23, 2010 at 3:48 PM, Ramsay Jones wrote:
The msvc debug build (make MSVC=1 DEBUG=1) issues a warning
on every invocation of the linker:
LINK : warning LNK4044: unrecognized option '/Zi'; ignored
In order to suppress the warning, we refrain from passing the
$(ALL_CFLAGS) macro to the linker.
Alternatively, we could leave the makefile alone and fix it up in
compat/vcbuild/scripts/clink.pl
(Apologies for the patch attached instead of inline; I'm not using a
whitespace-clean email client at the moment.)
Peter Harris
From: Ramsay Jones <hidden> Date: 2016-06-15 22:49:00
Jonathan Nieder wrote:
Ramsay Jones wrote:
quoted
LINK : warning LNK4044: unrecognized option '/Zi'; ignored
In order to suppress the warning, we refrain from passing the
$(ALL_CFLAGS) macro to the linker.
Note that, should it be necessary in the future, an option
intended for both the (front-end) compiler and the linker can
be included in both CFLAGS and LDFLAGS.
I think traditionally CPPFLAGS is meant to be used for the purpose
you are describing (see [1] for example).
Really? I thought that the general scheme was something like:
- LDFLAGS is for options which only affects the operation of
the linker (e.g. -L).
- CPPFLAGS is for options which only affects the operation of
the C pre-processor (e.g. -I, -D, -U)
- CFLAGS is for options which only affects the operation of
the compiler proper.
If an option affects multiple phases, then (one option) is to include
it into each of the above macros to which it applies.
In practice, of course, I've yet to see a Makefile which faithfully
implements the above scheme. ;-)
Also, the last time I was forced to use automake (yuck), I noticed that
it passed CPPFLAGS to the linker; I consider this to be a bug in
automake. :-P
[CFLAGS et. al. are also supposed to be user settable ...]
I realize that the Makefile does not currently use the terms this way:
making it consistent would require
. s/BASIC_CFLAGS/BASIC_CPPFLAGS/, except that the
BASIC_CFLAGS += -Kthread
settings should probably stay as-is
. Windows BASIC_CFLAGS would probably need to be split:
BASIC_CFLAGS = -nologo
BASIC_CPPFLAGS = -I. -I../zlib ... -DWIN32 ...
. s/COMPAT_CFLAGS/COMPAT_CPPFLAGS
What do you think?
I think I am missing something, since I don't see how this relates to
my patch!
I'm sure the misunderstanding is mine; sorry to be so dense!
ATB,
Ramsay Jones
From: Ramsay Jones <hidden> Date: 2016-06-15 22:49:00
Peter Harris wrote:
On Wed, Jun 23, 2010 at 3:48 PM, Ramsay Jones wrote:
quoted
The msvc debug build (make MSVC=1 DEBUG=1) issues a warning
on every invocation of the linker:
LINK : warning LNK4044: unrecognized option '/Zi'; ignored
In order to suppress the warning, we refrain from passing the
$(ALL_CFLAGS) macro to the linker.
Alternatively, we could leave the makefile alone and fix it up in
compat/vcbuild/scripts/clink.pl
Hmm, OK ... but it would be much easier if we didn't pass inappropriate
options in the first place. :-P
Also, are you correctly filtering *all* possible inappropriate options?
For example, the very next patch on my branch (not sent to list) adds
a -W3 option ... ;-)
ATB,
Ramsay Jones
From: Jonathan Nieder <hidden> Date: 2016-06-15 22:49:00
Ramsay Jones wrote:
I thought that the general scheme was something like:
- LDFLAGS is for options which only affects the operation of
the linker (e.g. -L).
- CPPFLAGS is for options which only affects the operation of
the C pre-processor (e.g. -I, -D, -U)
- CFLAGS is for options which only affects the operation of
the compiler proper.
If an option affects multiple phases, then (one option) is to include
it into each of the above macros to which it applies.
That does make sense. Perhaps “traditionally” was not the right word;
I was just looking at common practice. I’ve just never seen -O put
into LDFLAGS, for example, and I think most people would expect
setting CFLAGS=-O0 to affect the linker, too.
Jonathan Nieder wrote:
quoted
I realize that the Makefile does not currently use the terms this way:
making it consistent would require
. s/BASIC_CFLAGS/BASIC_CPPFLAGS/, except that the
[etc]
quoted
What do you think?
I think I am missing something, since I don't see how this relates to
my patch!
I suspect the problem comes from my unfamiliarity with MSVC.
By analogy with GCC, I assumed that any parameter is intended for the
preprocessor, the linker, or the frontend/both. In other words, I was
assuming flags for the compiler proper can be safely passed to the
linker, and that /Zi must be for the preprocessor-using stages only.
CFLAGS tends to contain front-end arguments used for all stages of
translation.
What does /Zi do? Searching online reveals:
- it means “enable debugging information”
- weakly implies /debug for the linker
- overrides /Z7 and /Zd, and is overridden by them
- is incompatible with /MP
- is among the prerequisites for /dlp and for /Gm
Maybe clink.pl should use “cl.exe /link” instead of “link.exe” and this
problem would go away.
From: Bernhard R. Link <hidden> Date: 2016-06-15 22:49:00
* Ramsay Jones [off-list ref] [100626 20:02]:
Really? I thought that the general scheme was something like:
- LDFLAGS is for options which only affects the operation of
the linker (e.g. -L).
- CPPFLAGS is for options which only affects the operation of
the C pre-processor (e.g. -I, -D, -U)
- CFLAGS is for options which only affects the operation of
the compiler proper.
I think the problem are options like -g or -m64 and the like which
you also want to give to the linker on some systems, so it is quite
common and I'd say recommended to have the compiling stage get
CPPFLAGS and CFLAGS and the linking stage getting CFLAGS and LDFLAGS
(and link using the compiler, otherwise you might miss libraries anyway).
I do not know if automake started with this, or if it is older.
Also, the last time I was forced to use automake (yuck), I noticed that
it passed CPPFLAGS to the linker; I consider this to be a bug in
automake. :-P
Huh? Never heared of that bug, it must be more than 15 years ago. Are
you sure it was not CFLAGS it passed to the linker?
Bernhard R. Link
No need to CC me, I'm subscribed...
From: Peter Harris <hidden> Date: 2016-06-15 22:49:01
On Fri, Jun 25, 2010 at 4:24 PM, Ramsay Jones wrote:
Peter Harris wrote:
quoted
On Wed, Jun 23, 2010 at 3:48 PM, Ramsay Jones wrote:
quoted
The msvc debug build (make MSVC=1 DEBUG=1) issues a warning
on every invocation of the linker:
LINK : warning LNK4044: unrecognized option '/Zi'; ignored
In order to suppress the warning, we refrain from passing the
$(ALL_CFLAGS) macro to the linker.
Alternatively, we could leave the makefile alone and fix it up in
compat/vcbuild/scripts/clink.pl
Hmm, OK ... but it would be much easier if we didn't pass inappropriate
options in the first place. :-P
Well, yes and no.
The vast majority of git devs work in an environment where compiler
flags and linker flags aren't completely disjoint, and it's not very
polite of us to rework their makefile to fit our view of the world.
Especially when we already have a wrapper that fits their world to
ours.
So I suggest that improving clink.pl is a whole lot easier for a whole
lot more people (compared to reworking the entire makefile being
easier for far fewer people).
Also, are you correctly filtering *all* possible inappropriate options?
For example, the very next patch on my branch (not sent to list) adds
a -W3 option ... ;-)
Heck no. This only filters the options that are used by the git makefile.
But adding a filter for -W* is a trivial one liner (one character-er, even).
Peter Harris
From: Ramsay Jones <hidden> Date: 2016-06-15 22:49:02
Peter Harris wrote:
The vast majority of git devs work in an environment where compiler
flags and linker flags aren't completely disjoint, and it's not very
polite of us to rework their makefile to fit our view of the world.
Especially when we already have a wrapper that fits their world to
ours.
Yep, good point.
quoted
Also, are you correctly filtering *all* possible inappropriate options?
For example, the very next patch on my branch (not sent to list) adds
a -W3 option ... ;-)
Heck no. This only filters the options that are used by the git makefile.
But adding a filter for -W* is a trivial one liner (one character-er, even).
Your patch fixes my problem, and is *much* better than my patch, so could
you send a formal patch to the list (with W added to the the regex as well
as Z)?
Thanks.
ATB,
Ramsay Jones
From: Ramsay Jones <hidden> Date: 2016-06-15 22:49:02
Jonathan Nieder wrote:
That does make sense. Perhaps “traditionally” was not the right word;
I was just looking at common practice. I’ve just never seen -O put
into LDFLAGS, for example, and I think most people would expect
setting CFLAGS=-O0 to affect the linker, too.
[This is now a bit off-topic, see below, but what version of gcc are
you using that passes -O to the linker (well, collect2 anyway)?
Do you have to pass -flto? For example "gcc -v -g -O2 hello.o -o hello"
seems to show (for both gcc 3.4.4 and gcc 4.4.0) that neither -g or -O2
is passed to collect2.
Hmmm, the output from gcc 4.4.0 adds a line like:
COLLECT_GCC_OPTIONS='-v' '-g' '-O2' '-o' 'hello.exe' '-mtune=i386'
so, does this imply this is a env. var exported to collect2?
dunno ;-P ]
I suspect the problem comes from my unfamiliarity with MSVC.
Heh, I'm not an expert user of msvc either!
Maybe clink.pl should use “cl.exe /link” instead of “link.exe” and this
problem would go away.
I tried this *very* quickly and it didn't work. (It's quite possible
that I could spend some time and get it to work, but ...)
The patch that Peter sent seems like the way to go, since it does not
affect the regular (non-msvc) build - and it works! ;-)
So we can drop this patch. Thanks for your time.
ATB,
Ramsay Jones
From: Peter Harris <hidden> Date: 2016-06-15 22:49:04
On Wed, Jun 30, 2010 at 3:52 PM, Ramsay Jones wrote:
Your patch fixes my problem, and is *much* better than my patch, so could
you send a formal patch to the list (with W added to the the regex as well
as Z)?
I'm currently on vacation. I'll try to remember to do that on Monday.
Peter Harris