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

Status: RESOLVED FIXED

(Firefox Build System :: General, defect)

Description

[Serge Gautherie (:sgautherie)]

(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 :-/

Comment 1

[Serge Gautherie (:sgautherie)]

(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.)

Comment 2

[Serge Gautherie (:sgautherie)]

(Fwiw, it's this bug which lead me to file bug 475026...)

Comment 3

[Wan-Teh Chang]

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.

Comment 4

[Serge Gautherie (:sgautherie)]

(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

[Nelson Bolyard (seldom reads bugmail)]

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

[Wan-Teh Chang]

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.

Comment 7

[Nelson Bolyard (seldom reads bugmail)]

> 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

[Wan-Teh Chang]

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.

Comment 9

[:Gavin Sharp [email: [e-mail]]]

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

[Nelson Bolyard (seldom reads bugmail)]

(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

[Wan-Teh Chang]

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

[(not currently active) Ted Mielczarek]

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

Comment 13

[Serge Gautherie (:sgautherie)]

Attachment: (Av1) No more autoconf

Per comment 12.
(Untested but trivial.)

Comment 14

[Wan-Teh Chang]

Comment on attachment 365333 [details] [diff] [review]
(Av1) No more autoconf

r=wtc.

Comment 15

[Serge Gautherie (:sgautherie)]

Attachment: (Av1a) No more autoconf [Checkin: Comment 18 & 23]

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 ?)

Comment 17

[Serge Gautherie (:sgautherie)]

(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.

Comment 18

[Serge Gautherie (:sgautherie)]

Comment on attachment 365341 [details] [diff] [review]
(Av1a) No more autoconf
[Checkin: Comment 18 & 23]


http://hg.mozilla.org/mozilla-central/rev/146ea3a52cf9

Comment 19

[(not currently active) Ted Mielczarek]

(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

[Bob Clary [:bc] (inactive)]

(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.

Comment 21

[Serge Gautherie (:sgautherie)]

(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...

Comment 22

[Mike Beltzner [:beltzner, not reading bugmail]]

Comment on attachment 365341 [details] [diff] [review]
(Av1a) No more autoconf
[Checkin: Comment 18 & 23]

a191=beltzner

Comment 23

[Serge Gautherie (:sgautherie)]

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

Comment 24

[Bob Clary [:bc] (inactive)]

http://hg.mozilla.org/tracemonkey/rev/20bbcdfa984c to update comment in testing/sisyphus/bin/tester.sh.

Comment 25

[Bob Clary [:bc] (inactive)]

/cvsroot/mozilla/js/tests/bisect.sh,v  <--  bisect.sh
new revision: 1.8; previous revision: 1.7