Closed
Bug 304785
Opened 20 years ago
Closed 20 years ago
Replacing NSPR variables in config/autoconf.mk is slow
Categories
(Firefox Build System :: General, defect)
Tracking
(Not tracked)
RESOLVED
FIXED
People
(Reporter: mozilla, Unassigned)
Details
(Keywords: fixed1.8)
Attachments
(1 file, 1 obsolete file)
1.50 KB,
patch
|
chase
:
review+
asa
:
approval1.8b5+
|
Details | Diff | Splinter Review |
Near the end of the configure run (line 7135 of mozilla/configure.in) a message
appears
configure: warning: Recreating autoconf.mk with updated nspr-config output
after which the configure process seems to hang for some time while using 100%
CPU. On my relatively fast machine this takes about 1 minute but others said (in
netscape.public.mozilla.os2) that it takes tens of minutes on their machines, so
this is worth improving on.
The "hang" is due to a perl loop used to replace the variables NSPR_LIBS and
NSPR_CFLAGS in config/autoconf.mk. This loop is very slow on platforms that are
not as effective at spawning shells (like OS/2) as nspr-config is called for
every line in autoconf.mk.
Reporter | ||
Comment 1•20 years ago
|
||
This alleviates the problem by using the nspr-config call just once, before the
perl loop, to input it into a variable that in turn is used in the perl
one-liner.
I didn't find any other instance of this export + perl $ENV hack so there
probabl y is a better way, but the patch doesn't break anything on Linux while
on OS/2 the time it takes to do this variable replacement is down from one
minute to a fraction of a second.
Updated•20 years ago
|
Attachment #192807 -
Flags: review?(chase)
Updated•20 years ago
|
Attachment #192807 -
Flags: review?(chase) → review+
Comment 2•20 years ago
|
||
Regarding your hack comment, there is a better way if you get the urge. Unix
shells do sane quoting. This means that
v='d'
echo 'abc'$v'def'
will do what you expect, so you can get rid of the export and replace the perl
invocation with
$PERL -pi.bak -e 's {^NSPR_LIBS\s*=.*} {NSPR_LIBS = '"$NSPR_LIBS_REPL"' }'
config/autoconf.mk
I suspect autoconf style would prefer the variable to be lowercased too.
I'd need to test but the double quotes are unlikely to be required.
Comment 3•20 years ago
|
||
Peter, do you want to try Steven's suggestion before I check this in?
Reporter | ||
Comment 4•20 years ago
|
||
Now I tried it but it doesn't work. With normall commands Steven's trick works
but perl somehow interprets the $\(DIST\) and whatever I try with escaping and
quoting it always gives me CFLAGS like "-I430 430DIST)/include/nspr". So, I
would vote to check in the patch as is.
Comment 5•20 years ago
|
||
I neglected to avoid excess variable interpolation. The revised patch should
handle all cases as long as nspr_config does not output a value containing one
or more single quotes.
FWIW, $( is a variable that represents the read gid of the current process.
Since OS/2 doesn't do gids, perl always supplies a 0 value.
Reporter | ||
Comment 6•20 years ago
|
||
Comment on attachment 193250 [details] [diff] [review]
Updated autoconf.mk edit patch
Yes, that works for me both on OS/2 and Linux with bash or ash as shell. chase,
do you like this one better, too?
Attachment #193250 -
Flags: review?(chase)
Comment 7•20 years ago
|
||
Comment on attachment 193250 [details] [diff] [review]
Updated autoconf.mk edit patch
Yeah, even better.
Attachment #193250 -
Flags: review?(chase) → review+
Comment 8•20 years ago
|
||
Fix checked in.
Status: NEW → RESOLVED
Closed: 20 years ago
Resolution: --- → FIXED
Reporter | ||
Updated•20 years ago
|
Attachment #192807 -
Attachment is obsolete: true
Reporter | ||
Comment 9•20 years ago
|
||
Comment on attachment 193250 [details] [diff] [review]
Updated autoconf.mk edit patch
Mike, thanks for the checkin.
We probably have to do quite a lot of builds on the 1.8 branch, so it would be
nice to get this in there, too.
Attachment #193250 -
Flags: approval1.8b5?
Updated•20 years ago
|
Attachment #193250 -
Flags: approval1.8b5? → approval1.8b5+
Comment 10•20 years ago
|
||
Mike or Peter, time is running out on 1.8b5 and I suspect we're soon going to
start withdrawing the approvals for non-blocking bugs to minimize risk. If this
still needs to land on the branch, please do so ASAP and add the "fixed1.8"
keyword. Thanks.
Updated•7 years ago
|
Product: Core → Firefox Build System
You need to log in
before you can comment on or make changes to this bug.
Description
•