300913 - MSVC tools version number detection is broken for non-English MSVC

Status: RESOLVED FIXED

(Firefox Build System :: General, defect)

Description

[Giorgio Maone [:ma1]]

If I try to compile the tree using an Italian MSVC, build process fails because
MSVC version number recognition routines expect the "Version" word to be found
in the tool (compiler/linker/...) output.
This is a wrong assumption for non-English versions ofg MSVC tools: Italian
ones, for instance, spit out "Versione <Maj>.<Min>.<Build>", and I don't dare to
imagine what a Russian output looks like.
In short, version detection should be language independent.
I'm going to attach a patch immediately.

Comment 1

[Giorgio Maone [:ma1]]

Attachment: MSVC tools version detection done searching for language-independent numeric pattern

I've replaced the grep and sed regular expressions looking for the
language-dependent hardcoded "Version" string with other patterns looking for
the <maj>.<min>.<build> version number pattern on which the configure script
already depends.

Comment 2

[cls]

Comment on attachment 189422 [details] [diff] [review]
MSVC tools version detection done searching for language-independent numeric pattern

That works but the regexes could be a bit less confusing.   Try  's|.*
\([0-9]\+\.[0-9]\+\.[0-9]\+\).*|\1|'

Comment 3

[Giorgio Maone [:ma1]]

Attachment: Simplified sed regexp

V2 with more readable regexp (see comment #2)

Comment 4

[cls]

Comment on attachment 190575 [details] [diff] [review]
Simplified sed regexp

Unless there's a good reason to use a different regex for grep, they should be
the same.  r=cls with regexes the same (either way)

Comment 5

[Giorgio Maone [:ma1]]

Attachment: Same regexp for grep and sed

Comment 6

[Giorgio Maone [:ma1]]

Attachment: Same as V3 but with autoconf escaping

Well, apparently nobody (including reviewers & myself) realized that autoconf
was unescaping this section when building configure from configure.in (while
*every* other section containing grep or sed regexp was kept untouched).

Here is the correct patch for branch landing, diffed from MOZILLA_1_8_BRANCH
and escaped as timeless did to extinguish the trunk on fire tonight
(http://tinderbox.mozilla.org/bonsai/cvsview2.cgi?diff_mode=context&whitespace_mode=show&root=/cvsroot&subdir=mozilla&command=DIFF_FRAMESET&root=/cvsroot&file=configure.in&rev1=1.1505&rev2=1.1506)

Comment 7

[[e-mail]]

Neither my sed nor grep accept anyhing more than basic regexps i.e. . ? and *
Would anyone object to me changing the \+s to *s?

I'd also like to point out that since the regexps are now the same you can use
| sed -n -e 's| ... |\1|p' avoiding the grep.

Comment 8

[cls]

Since changing the \+s to \*s will change the meaning of the expression, I would
object.  What sed & grep are you using?

Comment 9

[[e-mail]]

(In reply to comment #8)
>Since changing the \+s to \*s will change the meaning of the expression,
>I would object.
OK, so how about [[0-9]][[0-9]]*

>What sed & grep are you using?
grep version 1.0 for SFU 3.5 (no sed version appears to be listed)

Comment 10

[Giorgio Maone [:ma1]]

(In reply to comment #9)
> (In reply to comment #8)
> >Since changing the \+s to \*s will change the meaning of the expression,
> >I would object.
> OK, so how about [[0-9]][[0-9]]*

LOL, this was my first patch for this bug...
https://bugzilla.mozilla.org/attachment.cgi?id=189422

I used that verbose syntax because I was XP concerned and I had read somewhere
the "+" quantifier being not part of "basic regular expression" as originally
conceived.
I didn't know if grep or sed version which didn't support it actually were in
the wild, though, hence I accepted Chris Seawood criticisms trusting his widsom ;)

> >What sed & grep are you using?
> grep version 1.0 for SFU 3.5 (no sed version appears to be listed)

Final compatible patch following in minutes...

Comment 11

[Giorgio Maone [:ma1]]

Attachment: autoconf esc. + compat. regexp (1.8 branch)

Modularized regexp, to avoid too much hassle if someone decides to change it
again...

Comment 12

[Giorgio Maone [:ma1]]

Attachment: autoconf esc. + compat. regexp (trunk)

diff from trunk to undo previous checkin

Comment 13

[cls]

Comment on attachment 192928 [details] [diff] [review]
autoconf esc. + compat. regexp (trunk)

Fine, fine.  Note that I actually did approve the use of the *s regex earlier
(see comment #4).  The more complicated sed regex from the original patch had
to go though.

As long as we're not bending over backwards for a non-standard build setup that
we don't officially support, I'm fine with the change.

Comment 14

[Giorgio Maone [:ma1]]

Attachment: autoconf esc. + compat. regexp (trunk)

-	 unset $_MSVC_VER_FILTER
+	 unset _MSVC_VER_FILTER

Comment 15

[Giorgio Maone [:ma1]]

Attachment: autoconf esc. + compat. regexp (1.8 branch)

Really sorry for the bugspam, but this abusive "$" just slipped out from my
finger tips :(

Comment 16

[[e-mail]]

Giorgio, do you need these checked in now?

Comment 17

[Giorgio Maone [:ma1]]

(In reply to comment #16)
> Giorgio, do you need these checked in now?

Yes, thanks - maybe time to apply for a CVS account ;)

Comment 18

[[e-mail]]

Comment on attachment 192930 [details] [diff] [review]
autoconf esc. + compat. regexp (trunk)

I checked this in on the trunk but I can't get my autoconf-2.13 to work so
you'll need someone else to do the branch.

Comment 19

[[e-mail]]

(In reply to comment #18)
>I can't get my autoconf-2.13 to work
Fortunately cltbld's autoconf works ;-)

Comment 20

[Chase Phillips]

(In reply to comment #19)
> (In reply to comment #18)
> >I can't get my autoconf-2.13 to work
> Fortunately cltbld's autoconf works ;-)

Yeah, I fixed cltbld's autoconf check-in to work on the Mozilla 1.8 branch
yesterday.  Let me know if you see any weirdness.

Comment 21

[Benjamin Smedberg]

*** Bug 284571 has been marked as a duplicate of this bug. ***