Closed Bug 480680 Opened 15 years ago Closed 15 years ago

nsprpub/configure often gets this unwanted(!?) local modification

Categories

(Firefox Build System :: General, defect)

defect
Not set
minor

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)

(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 :-/
(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.)
(Fwiw, it's this bug which lead me to file bug 475026...)
Blocks: 475026
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: 15 years ago
Resolution: --- → INVALID
(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 ?
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!
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.
Status: RESOLVED → REOPENED
Depends on: 104286
Resolution: INVALID → ---
> 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!
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.
Assignee: wtc → nobody
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.
(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?
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.
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
Attached patch (Av1) No more autoconf (obsolete) — Splinter Review
Per comment 12.
(Untested but trivial.)
Attachment #365333 - Flags: review?(wtc)
Attachment #365333 - Flags: review?(ted.mielczarek)
Attachment #365333 - Flags: review?(wtc) → review+
Comment on attachment 365333 [details] [diff] [review]
(Av1) No more autoconf

r=wtc.
Assignee: nobody → sgautherie.bz
Component: NSPR → Build Config
Product: NSPR → Core
QA Contact: nspr → build-config
Version: other → Trunk
Attachment #365333 - Flags: review?(ted.mielczarek) → review+
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)
Attachment #365341 - Flags: review?(ted.mielczarek) → review+
(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
Attachment #365341 - Attachment description: (Av1a) No more autoconf → (Av1a) No more autoconf [Checkin: Comment 18]
Attachment #365341 - Flags: approval1.9.1?
Status: REOPENED → ASSIGNED
Flags: in-testsuite-
Whiteboard: [Keep open until comment 17 answers]
(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.
(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.
(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: 15 years ago15 years ago
Resolution: --- → FIXED
Whiteboard: [Keep open until comment 17 answers]
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+
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]
Keywords: fixed1.9.1
Whiteboard: [fixed1.9.1b4]
http://hg.mozilla.org/tracemonkey/rev/20bbcdfa984c to update comment in testing/sisyphus/bin/tester.sh.
/cvsroot/mozilla/js/tests/bisect.sh,v  <--  bisect.sh
new revision: 1.8; previous revision: 1.7
Product: Core → Firefox Build System
You need to log in before you can comment on or make changes to this bug.