173122 - Installer build script fails with Cygwin perl

Status: RESOLVED FIXED

(SeaMonkey :: Installer, defect)

Description

[Steven Grimm]

The Windows installer build script
(mozilla/xpinstall/wizard/windows/builder/build.pl) only works with ActiveState
perl at the moment.  When I run it with Cygwin perl, it loses backslashes in my
paths in the system() calls.  As a quick test I tried quoting the filenames in a
couple of those calls, e.g.

if(system("perl makeall.pl $ver $DEPTH\\stage $cwdDistWin\\install -aurl
$inXpiURL -rurl $inRedirIniURL"))

becomes

if(system("perl makeall.pl $ver \"$DEPTH\\stage\" \"$cwdDistWin\\install\" -aurl
$inXpiURL -rurl $inRedirIniURL"))

and it seemed to work -- but there are a ton of calls like that all over the
build scripts and I'm not familiar enough with what the scripts are doing to
feel safe tweaking them in so many places.

Comment 1

[hacker formerly known as [e-mail]]

I thought that dprice already had a bug covering this.

Comment 2

[hacker formerly known as [e-mail]]

Attachment: add cygwin perl & objdir support to build.pl & friends

Cygwin perl was almost a drop in replacement after changing most of the \\ to /
and system(copy()) to File::Copy::copy.  Since File::Copy::copy doesn't work on
wildcards, I had to resort to an opendir, readdir-copy loop to avoid the system
copy call.   For some reason, the windows system directory isn't in cygwin
perl's default path so it can't find copy.exe.

Adding objdir support was a bit more difficult as there were a number of places
that looked for MOZ_SRC & MOZ_TOOLS in the environment.  The build.pl script
now requires a -topsrcdir <path> argument and takes an optional -depth argument
which specifies the location of the objdir.  Most of the helper scripts now
take $topsrcdir as their first argument.  

Oh, and it adds moz_art_lgpl.dll to packages-win.

Comment 3

[Daniel Veditz [:dveditz]]

Comment on attachment 104566 [details] [diff] [review]
add cygwin perl & objdir support to build.pl & friends

>Index: xpinstall/packager/packages-win
>===================================================================
>+bin\moz_art_lgpl.dll

What does this have to do with this bug? but OK I guess.

>Index: xpinstall/packager/windows/makeall.pl
>===================================================================
> # Make sure there are at least three arguments
>-if($#ARGV < 2)
>+if($#ARGV < 3)

Change comment to match ("four")

>Index: xpinstall/packager/windows/makejs.pl
>===================================================================
> # Make sure there are at least two arguments
>-if($#ARGV < 2)
>+if($#ARGV < 3)

Same nitpicky comment issue

>Index: xpinstall/packager/windows/makexpi.pl
>===================================================================
> # Make sure there are at least three arguments
>-if($#ARGV < 2)
>+if($#ARGV < 3)

and here...

sr=dveditz with these changes.

Comment 4

[hacker formerly known as [e-mail]]

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

Comment 5

[hacker formerly known as [e-mail]]

Attachment: v1.1

Updated the comments, removed the packages-win chunk and added a chdir() test
for -depth.

Comment 6

[Sean Su]

Comment on attachment 104566 [details] [diff] [review]
add cygwin perl & objdir support to build.pl & friends

please fix the usage() in makeall.pl, and do not check in until you coordinate
this with release engineering.

you'll break their daily build process if you don't

Also the commercial tree should be updated as well.   They have their own set
(almost the same) of .pl scripts.

r=ssu

Comment 7

[Curt Patrick (gone)]

Seawood, before you spend time on the commercial tree or having the release team
make any changes I am making a different significant overhaul of the installer
build scripts right now so that we don't have to maintain seperate scripts for
mozilla and for commercial and for gre and for mfcembed, etc., etc.  I believe
that these changes are very close to being ready for prime time, i.e. this week
or next.  If I can get these changes in, then you will not need to duplicate
your work for the commercial tree.  I will move your changes into the new
centralized scripts and we will need only to maintain the work in one set of
code (the main reason I'm making my changes to begin with!).

Comment 8

[Brian 'netdragon' Bober]

What about accepting cygwin paths in the environment on any perl?

Comment 9

[hacker formerly known as [e-mail]]

Why would you accept paths that you know aren't portable?  

I had another fix in mind that didn't require touching all of the helper scripts
but I'll wait for Curt's changes to land first.

Comment 10

[Sean Su]

Chris, I hope you don't mind that I've merged your patch to bug 186703.  I was
already tinkering with the installer build process so I decided apply your patch
to it as well.

Comment 11

[Sean Su]

taking

Comment 12

[Sean Su]

marking as fixed since bug 186703 has been fixed.

Comment 13

[Ben Bucksch (:BenB)]

Attachment: Patch, same as above, for the 1.0 branch, plus a few smaller fixes

I backported the patch to the 1.0.x branch.

I ran into other problems as well when using configure and cygwin's perl, I
hacked around them, and ssu helped me to work around another one (failure of
nsztool.exe to build uninstaller).

It works for me with this patch.

Comment 14

[Robert Mohr]

So building on win32 now requires either cygwin Perl /or/ ActiveState Perl? 
(re: bug 235802)

Comment 15

[cls]

(In reply to comment #14)
> So building on win32 now requires either cygwin Perl /or/ ActiveState Perl? 
> (re: bug 235802)

Building on win32 has always just required one version of perl.  It's just the
installer scripts which were(/are*) broken.

*Bug 228776 - fire* installer scripts are broken