Last Comment Bug 107976 - coreconf should allow CC and CCC to be overriden.
: coreconf should allow CC and CCC to be overriden.
Status: RESOLVED FIXED
:
Product: NSS
Classification: Components
Component: Build (show other bugs)
: 3.3.1
: All All
: P1 enhancement (vote)
: ---
Assigned To: Wan-Teh Chang
: Wan-Teh Chang
:
Mentors:
: 166305 (view as bug list)
Depends on:
Blocks: 93206 158385
  Show dependency treegraph
 
Reported: 2001-11-01 08:32 PST by Wan-Teh Chang
Modified: 2005-06-02 17:35 PDT (History)
6 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---


Attachments
Add CXXFLAGS to coreconf (6.14 KB, patch)
2002-09-06 14:46 PDT, cls
no flags Details | Diff | Splinter Review
Minimal patch for coreconf (1.16 KB, patch)
2003-02-10 16:37 PST, Wan-Teh Chang
no flags Details | Diff | Splinter Review
Minimal patch for coreconf, v2 (1.04 KB, patch)
2003-02-10 16:49 PST, Wan-Teh Chang
netscape: review+
Details | Diff | Splinter Review

Description Wan-Teh Chang 2001-11-01 08:32:48 PST
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.
Comment 1 Wan-Teh Chang 2001-11-01 08:37:32 PST
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



Comment 2 hacker formerly known as seawood@netscape.com 2001-11-01 09:03:59 PST
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.
Comment 3 Wan-Teh Chang 2001-11-01 13:21:35 PST
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 hacker formerly known as seawood@netscape.com 2001-11-01 14:16:19 PST
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.
  
Comment 5 Wan-Teh Chang 2001-11-01 15:26:08 PST
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 hacker formerly known as seawood@netscape.com 2001-11-04 20:53:20 PST
> 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.



Comment 7 Wan-Teh Chang 2002-01-25 16:23:51 PST
I haven't had time to work on this. Sigh.  I am moving
the target to NSS 4.0.
Comment 8 Dennis Haney 2002-05-20 02:33:18 PDT
Is something happening to this?
Comment 9 Dan Rosen 2002-06-19 17:53:55 PDT
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 Dan Rosen 2002-06-19 18:05:34 PDT
Hm. Chris Seawood just pointed me at bug 104541. Is this a dup or otherwise 
deserving of abandonment?
Comment 11 hacker formerly known as seawood@netscape.com 2002-06-23 22:20:40 PDT
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 hacker formerly known as seawood@netscape.com 2002-09-02 21:51:42 PDT
*** Bug 166305 has been marked as a duplicate of this bug. ***
Comment 13 Kai Engert (:kaie) (on vacation) 2002-09-06 11:28:54 PDT
What do you think about the patch I had attached to bug 166305?
Comment 14 hacker formerly known as seawood@netscape.com 2002-09-06 14:00:18 PDT
The patch from bug 93206 was more robust.
Comment 15 Wan-Teh Chang 2002-09-06 14:04:51 PDT
Yes.  We should move that patch (attachment 48743 [details] [diff] [review]) to
this bug.
Comment 16 cls 2002-09-06 14:46:52 PDT
Created attachment 98203 [details] [diff] [review]
Add CXXFLAGS to coreconf

Attachment 48743 [details] [diff] applied to the current NSS_CLIENT_TAG pull.
Comment 17 Kai Engert (:kaie) (on vacation) 2002-09-12 21:21:49 PDT
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.
Comment 18 Wan-Teh Chang 2002-12-06 15:46:04 PST
Moved to target milestone 3.8 because the original
NSS 3.7 release has been renamed 3.8.
Comment 19 Wan-Teh Chang 2003-02-10 16:37:11 PST
Created attachment 114057 [details] [diff] [review]
Minimal patch for coreconf

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.
Comment 20 Wan-Teh Chang 2003-02-10 16:49:19 PST
Created attachment 114058 [details] [diff] [review]
Minimal patch for coreconf, v2

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".
Comment 21 Nelson Bolyard (seldom reads bugmail) 2003-05-09 21:20:19 PDT
Remove target milestone of 3.8, since these bugs didn't get into that release.
Comment 22 cls 2005-06-02 17:35:58 PDT
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 .

Note You need to log in before you can comment on or make changes to this bug.