Closed
Bug 107976
Opened 23 years ago
Closed 19 years ago
coreconf should allow CC and CCC to be overriden.
Categories
(NSS :: Build, enhancement, P1)
Tracking
(Not tracked)
RESOLVED
FIXED
People
(Reporter: wtc, Assigned: wtc)
References
Details
Attachments
(2 files, 1 obsolete file)
(deleted),
patch
|
Details | Diff | Splinter Review | |
(deleted),
patch
|
netscape
:
review+
|
Details | Diff | Splinter Review |
coreconf should allow CC and CCC (the C and C++
compilers) to be overriden. This will impose
a new restriction on the values of CC and CCC
and require some cleanup in coreconf to ensure
that the new restriction is followed. The
module (for example, PSM) that passes the values
of CC and CCC to coreconf must also follow the
restriction.
The new restriction is that CC and CCC must be
a single word, which is either the full pathname
or the name of the compiler. CC and CC must not
contain any compiler options.
For example, some legal values of CC are:
CC = gcc
CC = /usr/local/bin/gcc
CC = /home/wtc/bin/gcc-2.95.3
Some illegal values of CC are:
CC = cc -Ae
CC = cc -xarch=v9CC = gcc -pthread
Please let me know if it is feasible to follow
this restriction and whether additional restrictions
are required.
Assignee | ||
Comment 1•23 years ago
|
||
Correction: in the "Some illegal values of CC are"
paragraph, the line
CC = cc -xarch=v9CC = gcc -pthread
should be two lines
CC = cc -xarch=v9
CC = gcc -pthread
Status: NEW → ASSIGNED
Priority: -- → P1
Target Milestone: --- → 3.4
Comment 2•23 years ago
|
||
What's the reason for the single word restriction? Building with CC='cc
-xarch=v9' is a valid Mozilla build flag. We currently have a tinderboxes that
building with CC='cc -xarch=v9'. I thought that the patch I attached to bug
93206 handled the multi-word case without a problem. I haven't tested the
multi-word case recently though.
Assignee | ||
Comment 3•23 years ago
|
||
Chris Seawood wrote:
> What's the reason for the single word restriction?
> Building with CC='cc -xarch=v9' is a valid Mozilla
> build flag.
1. The single word restriction must be imposed on the
value of CC set by coreconf. I believe we all
agree this is necessary. For example, if
coreconf says
CC = gcc -pthread
and it gets overriden by
CC = /home/wtc/bin/gcc
on the command line, the critical -pthread flag
will be lost.
2. So the only issue is whether the single word
restriction should also be imposed on the value of
CC passed to coreconf on the command line.
As you pointed out, your patch handled the
multi-word case without a problem, so it is in
fact okay to pass a multi-word CC to coreconf.
On the other hand, this restriction is not as bad
as it seems. For the specific case of
CC='cc -xarch=v9' that you raised, I am asking you
to only pass the first word (that is, CC=cc) to
coreconf. In this case, you will also be passing
USE_64=1 to coreconf, which causes coreconf to add
-xarch=v9 to CFLAGS. So in the end you will get
exactly what you want.
Allowing CC and CCC to be overridden will add a lot
of flexibility to coreconf, but it will also open
a lot of possible ways to break coreconf. I am
concerned about the support cost down the road.
This is why I want us to think this through and
clearly state what can and cannot be done. My
first proposal is intentionally more restrictive
than necessary. I want to ensure that every step
we take is safe.
One question for you: is it more common to say
CC='cc -xarch=v9'
or
CC=cc CFLAGS=-xarch=v9
to specify a 64-bit Solaris build? Does this
latter work too?
Comment 4•23 years ago
|
||
I misread what you said. I thought that you were setting a restriction on the
value of CC passed to make. Yes, I agree that the value set in the
coreconf/*.mk files needs to be a single word, otherwise we'll clobber the
useful flags.
For the specific case that I cited, the restriction works but only because you
have another mechanism to handle that specific case. If I wanted to do a CC='cc
-o32' build on Irix (to get plugin compatibility with 4.x), then we'd be back to
square one.
Right now, it's more common to say CC='cc -xarch=v9' to get a 64-bit build as
it is guaranteed to work across most packages. In theory, either method should
work but I haven't tested the latter. The problem is that some cflags, like
-xarch=v9, change the behavior of the compiler so much that you must make sure
that all binaries are compiled with that flag. Usually, setting CC is the only
way to keep that guarantee.
Assignee | ||
Comment 5•23 years ago
|
||
Chris Seawood wrote:
> I misread what you said. I thought that you were setting
> a restriction on the value of CC passed to make.
Actually I want to set that restriction as well. I know it
will be okay to pass a multi-word value of CC to make, at
least I don't have a real-world counterexample, but I just
feel that I need to think more about it.
For your new example of CC='cc -o32' on IRIX, you can use
the coreconf option USE_N32=, although coreconf won't do
the right thing if the version of cc you are using generates
n32 binaries by default. This can be fixed by modifying
coreconf to use -o32 if USE_N32 is undefined or adding a new
coreconf option USE_O32.
My point is that coreconf usually has an option for each
compiler flag that you want to specify in the value of CC.
For example, coreconf has USE_64=1 for CC='cc -xarch=v9'
and USE_N32= for CC='cc -o32'.
I propose that we first proceed with the restriction that
*both* the value of CC passed to make and the value set in
the coreconf/*.mk files need to be a single word, and get
your patch checked in. Then, I will think it over whether
we can allow passing a multi-word value of CC to make.
How does this sound?
Comment 6•23 years ago
|
||
> My point is that coreconf usually has an option for each
> compiler flag that you want to specify in the value of CC.
And when coreconf doesn't have an equivalent option, then we are blocked waiting
for someone to find the time to:
1) verify that our problem is actually a coreconf problem
and
2) deem it important enough to devote resources to fixing
Historically, neither of those steps has typically been done in a timely manner.
Why make the developers bother going through that timesink when we can just
build flexibility into the system to allow multi-word CCs to be passed in?
Coreconf loses nothing by allowing multi-word CCs to be passed in and developers
gain the flexibility of being able to use their "non-standard" configurations
without being unnecessarily blocked.
Assignee | ||
Comment 7•23 years ago
|
||
I haven't had time to work on this. Sigh. I am moving
the target to NSS 4.0.
Target Milestone: 3.4 → 4.0
Comment 8•22 years ago
|
||
Is something happening to this?
I'd like to gently push for this as well. I'm building mozilla for the iPaq,
and I have to cross-compile, because the iPaq doesn't have enough resources to
compile natively. Being able to override CC and CCC would greatly simplify the
task of cross-compiling security.
I'd say that in general, this is a moderately important bug for getting Mozilla
running on embedded devices.
Comment 10•22 years ago
|
||
Hm. Chris Seawood just pointed me at bug 104541. Is this a dup or otherwise
deserving of abandonment?
Comment 11•22 years ago
|
||
No, this isn't a dupe of 104541. There are valid uses of overriding CC/CXX
other than just for cross-compiling. And according to bug 104541#c2, just
overriding CC/CXX isn't enough to get a proper cross-compilation env setup for NSS.
Comment 12•22 years ago
|
||
*** Bug 166305 has been marked as a duplicate of this bug. ***
Comment 13•22 years ago
|
||
What do you think about the patch I had attached to bug 166305?
Comment 14•22 years ago
|
||
The patch from bug 93206 was more robust.
Assignee | ||
Comment 15•22 years ago
|
||
Yes. We should move that patch (attachment 48743 [details] [diff] [review]) to
this bug.
Comment 16•22 years ago
|
||
Attachment 48743 [details] [diff] applied to the current NSS_CLIENT_TAG pull.
Comment 17•22 years ago
|
||
Comment on attachment 98203 [details] [diff] [review]
Add CXXFLAGS to coreconf
Dauphin asked me to review the patch. It looks good to me, but I suggest that
Wan-Teh should review as well, since I'm not really a NSS person.
Attachment #98203 -
Flags: review+
Assignee | ||
Updated•22 years ago
|
Target Milestone: 4.0 → 3.7
Assignee | ||
Comment 18•22 years ago
|
||
Moved to target milestone 3.8 because the original
NSS 3.7 release has been renamed 3.8.
Target Milestone: 3.7 → 3.8
Updated•22 years ago
|
Attachment #98203 -
Flags: review+ → review?(wtc)
Assignee | ||
Comment 19•22 years ago
|
||
This patch is the absolute minimum changes required
for Mozilla to override the compilers used by NSS.
Note the two simplifications:
1. It only allows the C compiler to be overridden.
This is sufficient for NSS because NSS does not have
any C++ code.
2. It requires that the value of CC be a pathname,
with no command-line options. I understand that
this may turn out to be a limitation, but I suggest
that we check in this patch first because it is
still a step forward.
I will attach the corresponding PSM patch to bug
93206.
Assignee | ||
Updated•22 years ago
|
Attachment #114057 -
Flags: review?(seawood)
Assignee | ||
Comment 20•22 years ago
|
||
I found that GNU make's "basename" function does not
do the same thing as C's "basename" function :-) The
GNU make function we want is "notdir".
Attachment #114057 -
Attachment is obsolete: true
Assignee | ||
Updated•22 years ago
|
Attachment #114057 -
Flags: review?(seawood)
Assignee | ||
Updated•22 years ago
|
Attachment #114058 -
Flags: review?(seawood)
Updated•22 years ago
|
Attachment #114058 -
Flags: review?(seawood) → review+
Updated•22 years ago
|
Attachment #98203 -
Flags: review?(wtc)
Comment 21•22 years ago
|
||
Remove target milestone of 3.8, since these bugs didn't get into that release.
Target Milestone: 3.8 → ---
Comment 22•19 years ago
|
||
A form of this patch has been checked into the NSS tree some time ago. See
http://bonsai.mozilla.org/cvslog.cgi?file=mozilla/security/coreconf/ruleset.mk .
Status: ASSIGNED → RESOLVED
Closed: 19 years ago
Resolution: --- → FIXED
You need to log in
before you can comment on or make changes to this bug.
Description
•