evil use of LCFLAGS in config.mak



Build Config
19 years ago
14 years ago


(Reporter: dveditz, Assigned: dveditz)


Firefox Tracking Flags

(Not tracked)




19 years ago
LCFLAGS is for Local compiler flags -- i.e. local to the individual makefile.
Because config.mak was always included last (or nearly so) the fact that it
added stuff to LCFLAGS didn't break anything. But now that config.mak is first
any makefile that does LCFLAGS=foo will stomp the config.mak settings.
Config.mak should be adding to CFLAGS instead.

(Ultimately having to include config.mak explicitly in local makefiles is wrong
and should be changed as well.)

Comment 1

19 years ago
This bug is mainly a heads up in case someone notices odd behavior in the
post-Necko windows builds--it might be due to LCFLAGS stompage. I expect to
check in a fix for this tonight or tomorrow.

Comment 2

19 years ago
I agree that setting LCFLAGS in config.mak is wrong wrong wrong, but including
config.mak explicitly in a local makefile is a useful thing. It allows you to
get the default settings of variables so that you can augment/override them
before including rules.mak. Otherwise including rules.mak includes config.mak
automatically, overriding _your_ settings.

Comment 3

19 years ago
I'll concede a hypothetical rare case where including config.mak at the top is
useful, but in the Necko-landing case it was not the best thing. If there is
some variable that commonly gets added or overridden we can add them as they
come up -- examples being LLFLAGS and LCFLAGS.

In this case NECKO added ugly conditionals to LLIBS, and then when the sense of
the conditional changed config.mak was slapped on the top. There were three
other options, each of which I think would have been better, with the best
solution something NECKO should have done back when its branch first started.

1. since all the makefiles got edited anyway, change "!if defined(NECKO)" to
"!if !defined(NO_NECKO)"

2. rip out the conditional -- we can't really go back anyway.

3. replace the explicit networking library names with a variable (like NSPR
does), and then define that variable conditionally in config.mak

I think #3 is what should have been done in the first place. It keeps
unnecessary conditional ugliness out of local makefiles, and then any switch
can be thrown in a centralized place. Ditto for libraries like js3250.dll or
other libraries whose names are likely to change -- with a centralized variable
you don't have to edit hundreds of makefiles when we change these things.

I suppose this thread should be in a newsgroup instead, it has little to do
with fixing this bug (which involved simply changing LCFLAGS to CFLAGS)


19 years ago
Assignee: cyeh → dveditz


19 years ago
Last Resolved: 19 years ago
Resolution: --- → FIXED

Comment 4

19 years ago
Fix checked-in. Nobody reported any win-only bustage so I guess it was merely a
potential bug. A lot of local makefiles show extreme paranoia and do
"LCFLAGS=$(LCFLAGS) otherstuff", and the ones that aren't paranoid happened not
to have config.mak added. Dodged *that* bullet :-)

Comment 5

19 years ago
All I can say is that we used to need to pre-include config.mak all the time in
the 4.x days (for java-related stuff). I can't say how often we still really
need it. That's the whole reason for it being separated out from rules.mak.
Product: Browser → Seamonkey
You need to log in before you can comment on or make changes to this bug.