Closed Bug 173122 Opened 22 years ago Closed 22 years ago

Installer build script fails with Cygwin perl

Categories

(SeaMonkey :: Installer, defect)

x86
Windows 2000
defect
Not set
normal

Tracking

(Not tracked)

RESOLVED FIXED

People

(Reporter: koreth-mozilla, Assigned: ssu0262)

References

Details

Attachments

(2 files, 1 obsolete file)

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.
I thought that dprice already had a bug covering this.
Assignee: seawood → dveditz
Status: UNCONFIRMED → NEW
Component: Build Config → Installer
Ever confirmed: true
QA Contact: granrose → bugzilla
QA Contact: bugzilla → ktrina
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 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.
Attachment #104566 - Flags: superreview+
*** Bug 164941 has been marked as a duplicate of this bug. ***
Attached patch v1.1 (deleted) — Splinter Review
Updated the comments, removed the packages-win chunk and added a chdir() test for -depth.
Attachment #104566 - Attachment is obsolete: true
Attachment #104566 - Flags: review+
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
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!).
What about accepting cygwin paths in the environment on any perl?
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.
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.
taking
Assignee: dveditz → ssu
marking as fixed since bug 186703 has been fixed.
Status: NEW → RESOLVED
Closed: 22 years ago
Resolution: --- → FIXED
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.
So building on win32 now requires either cygwin Perl /or/ ActiveState Perl? (re: bug 235802)
(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
Product: Browser → Seamonkey
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: