284810 - Typo in perl script regexp causes platform to be incorrectly identified as Unix when building installer under Windows/Cygwin/OS2

Status: RESOLVED WORKSFORME

(Toolkit Graveyard :: Build Config, defect)

Description

[dave]

User-Agent:       Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.7.5) Gecko/20050211
Build Identifier: N/A (FIREFOX_1_0_1_RELEASE tag)

There is an extraneous leading space (presumably a typo) in the perl regexp used
to identify the OS under which the installer is being built in the following
script file: mozilla/toolkit/mozapps/installer/build_static.pl

Quite simply, this line:
$win32 = ($^O =~ / ((MS)?win32)|cygwin|os2/i) ? 1 : 0;
should read:
$win32 = ($^O =~ /((MS)?win32)|cygwin|os2/i) ? 1 : 0;

Although it may seem like a bit of an overkill, I nonetheless created a patch
file in the format preferred by Mozilla.org, as specified in the guildelines for
patch file submission.  I had never used CVS before, so it was worth the
learning experience. :-)


Reproducible: Always

Steps to Reproduce:
N/A
'MSWin32' !~ / MSWin32/

Comment 1

[dave]

Attachment: patch to mozilla/toolkit/mozapps/installer/build_static.pl

Comment 2

[dave]

Oops!  I failed to explicitly state that the result of incorrectly detecting the
platform in $(MOZ_SRC)/toolkit/mozapps/installer/build_static.pl is that the
installer _WILL NOT_ build under Windows (i.e., this is not merely a cosmetic
issue).  CVS tag: FIREFOX_1_0_1_RELEASE

Comment 3

[u88484]

you have to ask for review on patches so that they have a chance to get checked in.

Comment 4

[Reed Loden [:reed]]

Comment on attachment 176306 [details] [diff] [review]
patch to mozilla/toolkit/mozapps/installer/build_static.pl

Since the reporter never responding, I'm taking the liberty to request first-review from bsmedberg since this code is still unchanged in trunk.

Comment 5

[Benjamin Smedberg]

Comment on attachment 176306 [details] [diff] [review]
patch to mozilla/toolkit/mozapps/installer/build_static.pl

But obviously the installer *does* build, so I'm not sure what the issue is.

Comment 6

[dave]

(In reply to comment #5)
> (From update of attachment 176306 [details] [diff] [review])
> But obviously the installer *does* build, so I'm not sure what the issue is.
> 

My dated response is that it did not build on Windows when I reported this.  I don't know whether this has been fixed as I'm no longer using Windows.  By installer, I meant the self-extracting installer.

Comment 7

[Matthias Versen [:Matti]]

since the installer builds marking wfm