Closed
Bug 480680
Opened 14 years ago
Closed 14 years ago
nsprpub/configure often gets this unwanted(!?) local modification
Categories
(Firefox Build System :: General, defect)
Firefox Build System
General
Tracking
(Not tracked)
RESOLVED
FIXED
mozilla1.9.2a1
People
(Reporter: sgautherie, Assigned: sgautherie)
References
Details
(Keywords: fixed1.9.1, Whiteboard: [fixed1.9.1b4])
Attachments
(1 file, 1 obsolete file)
1.33 KB,
patch
|
ted
:
review+
beltzner
:
approval1.9.1+
|
Details | Diff | Splinter Review |
(This is something I have noticed for some time now...) This is the diff I end up with: { diff --git a/nsprpub/configure b/nsprpub/configure --- a/nsprpub/configure +++ b/nsprpub/configure @@ -6066,17 +6066,17 @@ trap 'rm -f $CONFIG_STATUS conftest*; ex # Protect against Makefile macro expansion. cat > conftest.defs <<\EOF s%#define \([A-Za-z_][A-Za-z0-9_]*\) *\(.*\)%-D\1=\2%g s%[ `~#$^&*(){}\\|;'"<>?]%\\&%g s%\[%\\&%g s%\]%\\&%g s%\$%$$%g EOF -DEFS=`sed -f conftest.defs confdefs.h | tr '\012' ' ' | tr '\015' ' '` +DEFS=`sed -f conftest.defs confdefs.h | tr '\012' ' '` rm -f conftest.defs # Without the "./", some shells look in PATH for config.status. : ${CONFIG_STATUS=./config.status} echo creating $CONFIG_STATUS rm -f $CONFIG_STATUS } I think this happens (randomly ?) when I build (mozilla-central) SeaMonkey on my Windows 2000. (Though it might be Firefox too.) I don't know what triggers it :-/
Assignee | ||
Comment 1•14 years ago
|
||
(In reply to comment #0) > I don't know what triggers it :-/ Actually, it may be the obvious condition that configure.in was updated in my working directory :-| (I haven't tried to confirm that, yet.)
Assignee | ||
Comment 2•14 years ago
|
||
(Fwiw, it's this bug which lead me to file bug 475026...)
Blocks: 475026
Comment 3•14 years ago
|
||
I manually add "| tr '\015' ' '" to nsprpub/configure after I generate it with autoconf-2.13. It is necessary to support MKS Korn Shell on Windows. You can safely ignore this if you don't use MKS Korn Shell.
Status: NEW → RESOLVED
Closed: 14 years ago
Resolution: --- → INVALID
Assignee | ||
Comment 4•14 years ago
|
||
(In reply to comment #3) > I manually add "| tr '\015' ' '" to nsprpub/configure > after I generate it with autoconf-2.13. It is necessary > to support MKS Korn Shell on Windows. Could you add a comment to the file too, saying that ? > You can safely ignore this if you don't use MKS Korn Shell. Good to know. Yet, I always notice it (now) because this indirectly adds a '+' to the url reported in FF/SM about:buildconfig :-/ Couldn't your manual patch of the file become automatic/integrated somehow ?
Comment 5•14 years ago
|
||
Is this a difference between the CVS repository and the Hg repostiory? Does the extra tr in the pipeline cause any problems for any platform? I use MKS, and I really dread the thought of having to manually alter nsprpub/configure.in with every update!
Comment 6•14 years ago
|
||
I suspect that it is possible to write a m4 macro to automate the addition of "| tr '\015' ' '"; we could then put the m4 macro in our autoconf auxilliary directory nsprpub/build/autoconf. It is tedious to add "| tr '\015' ' '" manually, so if anyone familiar with autoconf and m4 is willing to do this, I'll greatly appreciate it. See bug 104286 comment 0 and bug 104286 comment 4 for more info on this issue. The extra "| tr '\015' ' '" is harmless if you don't use MKS Korn Shell.
Assignee | ||
Updated•14 years ago
|
Comment 7•14 years ago
|
||
> The extra "| tr '\015' ' '" is harmless if you don't use MKS Korn Shell.
Well, then, why don't we check in the copy of that line that has that added,
so that all builders can build without needing to alter configure.in!
Comment 8•14 years ago
|
||
The "| tr '\015' ' '" is in nsprpub/configure, not nsprpub/configure.in. When I run autoconf-2.13 in nsprpub to generate nsprpub/configure, the generated nsprpub/configure doesn't work with MKS Korn Shell. So I manually add the "| tr '\015' ' '" to nsprpub/configure. And then I cvs commit nsprpub/configure.in and nsprpub/configure. I do this every time I cvs commit nsprpub/configure.in. People who build NSPR do not need to alter the nsprpub/configure from CVS. Serge's confusion came when he ran autoconf-2.13 in nsprpub, and found that nsprpub/configure was modified, because it was missing the "| tr '\015' ' '" I manually added to the version checked in to CVS.
Updated•14 years ago
|
Assignee: wtc → nobody
Comment 9•14 years ago
|
||
The Mozilla build process runs autoconf, so this problem affects more than just Serge. Having the build process create local changes to the source directory is problematic for several reasons, so it would be nice to fix this problem. I think the ideal fix would be for NSPR to remove it's generated configure from CVS. Is there a reason you need it to be checked in? It introduces a dependency on autoconf for building, but the rest of the Mozilla project has been able to live with that dependency. Can NSPR live with it as well? If that is not a suitable solution for upstream NSPR, perhaps we could just avoid including nsprpub/configure when we do an import of NSPR into mozilla-central.
Comment 10•14 years ago
|
||
(In reply to comment #9) > I think the ideal fix would be for NSPR to remove it's generated configure > from CVS. Is there a reason you need it to be checked in? Yes, NSPR is supported on systems that cannot run autoconf. > If that is not a suitable solution for upstream NSPR, perhaps we could just > avoid including nsprpub/configure when we do an import of NSPR into > mozilla-central. I think that would be an acceptable solution. Wan-Teh, what do you think?
Comment 11•14 years ago
|
||
The Mozilla build process should not run autoconf in mozilla/nsprpub. It should know that mozilla/nsprpub has a pre-generated configure script, just like it knows mozilla/security/nss does not use autoconf. But, I'm tired of arguing. I don't care.
Comment 12•14 years ago
|
||
This is an unexpected side effect of switching to Mercurial. We stopped checking in the configure files in Hg, and so we forced RUN_AUTOCONF_LOCALLY to 1 in client.mk. Unfortunately that causes this problem. We can just remove nsprpub/configure from the list of configures to generate in client.mk, since it should always be pre-generated. http://mxr.mozilla.org/mozilla-central/source/client.mk#171
Assignee | ||
Comment 13•14 years ago
|
||
Per comment 12. (Untested but trivial.)
Attachment #365333 -
Flags: review?(wtc)
Attachment #365333 -
Flags: review?(ted.mielczarek)
Updated•14 years ago
|
Attachment #365333 -
Flags: review?(wtc) → review+
Comment 14•14 years ago
|
||
Comment on attachment 365333 [details] [diff] [review] (Av1) No more autoconf r=wtc.
Updated•14 years ago
|
Assignee: nobody → sgautherie.bz
Component: NSPR → Build Config
Product: NSPR → Core
QA Contact: nspr → build-config
Version: other → Trunk
Updated•14 years ago
|
Attachment #365333 -
Flags: review?(ted.mielczarek) → review+
Assignee | ||
Comment 15•14 years ago
|
||
Av1, completed based on comm-central client.mk. (Untested but trivial.) http://mxr.mozilla.org/mozilla-central/search?string=c-sdk&case=on&find=%2Fclient.mk I think this last reference is obsolete now that c-c has it. *** Fwiw, http://mxr.mozilla.org/mozilla-central/search?string=nsprpub%2Fconfigure&case=on Notice the 2 "# nsprpub/configure I'm looking at you!" in '/js/tests/bisect.sh'... (Are these comments right ?) http://mxr.mozilla.org/mozilla-central/find?string=%2Fconfigure%24 5 configure files in the tree... (Should they be listed too ?)
Attachment #365333 -
Attachment is obsolete: true
Attachment #365341 -
Flags: review?(ted.mielczarek)
Updated•14 years ago
|
Attachment #365341 -
Flags: review?(ted.mielczarek) → review+
Assignee | ||
Comment 17•14 years ago
|
||
(In reply to comment #15) > http://mxr.mozilla.org/mozilla-central/search?string=nsprpub%2Fconfigure&case=on > Notice the 2 "# nsprpub/configure I'm looking at you!" in > '/js/tests/bisect.sh'... > (Are these comments right ?) Bob, these come from bug 466951 and bug 476458(!). I think the comments can be removed after patch Av1a here, but the updates might still be needed if there may be other changes to handle (this I'm not aware of). What do you think ? (In reply to comment #15) > http://mxr.mozilla.org/mozilla-central/find?string=%2Fconfigure%24 > 5 configure files in the tree... > (Should they be listed too ?) Ted, could you check these ? Thanks.
OS: Windows 2000 → All
Hardware: x86 → All
Target Milestone: --- → mozilla1.9.2a1
Assignee | ||
Updated•14 years ago
|
Attachment #365341 -
Attachment description: (Av1a) No more autoconf → (Av1a) No more autoconf
[Checkin: Comment 18]
Attachment #365341 -
Flags: approval1.9.1?
Assignee | ||
Comment 18•14 years ago
|
||
Comment on attachment 365341 [details] [diff] [review] (Av1a) No more autoconf [Checkin: Comment 18 & 23] http://hg.mozilla.org/mozilla-central/rev/146ea3a52cf9
Assignee | ||
Updated•14 years ago
|
Status: REOPENED → ASSIGNED
Flags: in-testsuite-
Whiteboard: [Keep open until comment 17 answers]
Comment 19•14 years ago
|
||
(In reply to comment #17) > (In reply to comment #15) > > http://mxr.mozilla.org/mozilla-central/find?string=%2Fconfigure%24 > > 5 configure files in the tree... > > (Should they be listed too ?) > > Ted, could you check these ? Thanks. No, these are random configures from source imports (freetype and breakpad) that we don't use. I don't know what the NSS one is, but NSS doesn't use autoconf, so don't worry about it.
Comment 20•14 years ago
|
||
(In reply to comment #17) > (In reply to comment #15) > > > http://mxr.mozilla.org/mozilla-central/search?string=nsprpub%2Fconfigure&case=on > > Notice the 2 "# nsprpub/configure I'm looking at you!" in > > '/js/tests/bisect.sh'... > > (Are these comments right ?) I can remove the comments as they are childish at best. However the problem will _always_ exist for bisection scripts which build with arbitrary revisions which predate the fix.
Assignee | ||
Comment 21•14 years ago
|
||
(In reply to comment #20) > I can remove the comments as they are childish at best. Yes please, I think they should either be removed or say something about this bug/fix :-) ***** I leave it to you, nspr people, to file a separate bug if you want to document/automate the currently manual step wtc does...
Status: ASSIGNED → RESOLVED
Closed: 14 years ago → 14 years ago
Resolution: --- → FIXED
Whiteboard: [Keep open until comment 17 answers]
Comment 22•14 years ago
|
||
Comment on attachment 365341 [details] [diff] [review] (Av1a) No more autoconf [Checkin: Comment 18 & 23] a191=beltzner
Attachment #365341 -
Flags: approval1.9.1? → approval1.9.1+
Assignee | ||
Comment 23•14 years ago
|
||
Comment on attachment 365341 [details] [diff] [review] (Av1a) No more autoconf [Checkin: Comment 18 & 23] http://hg.mozilla.org/releases/mozilla-1.9.1/rev/afc5fef8b23c
Attachment #365341 -
Attachment description: (Av1a) No more autoconf
[Checkin: Comment 18] → (Av1a) No more autoconf
[Checkin: Comment 18 & 23]
Assignee | ||
Updated•14 years ago
|
Keywords: fixed1.9.1
Whiteboard: [fixed1.9.1b4]
Comment 24•14 years ago
|
||
http://hg.mozilla.org/tracemonkey/rev/20bbcdfa984c to update comment in testing/sisyphus/bin/tester.sh.
Comment 25•14 years ago
|
||
/cvsroot/mozilla/js/tests/bisect.sh,v <-- bisect.sh new revision: 1.8; previous revision: 1.7
Updated•5 years ago
|
Product: Core → Firefox Build System
You need to log in
before you can comment on or make changes to this bug.
Description
•