Closed Bug 339096 Opened 18 years ago Closed 18 years ago

Building installer should be possible under msys

Categories

(Firefox :: Installer, defect)

2.0 Branch
x86
Windows XP
defect
Not set
normal

Tracking

()

RESOLVED WORKSFORME

People

(Reporter: regis.caspar+bz, Unassigned)

Details

Attachments

(3 files)

After bug 326580 landing, there is now some cygpath command used in mozilla/browser/installer/Makefile, but under msys, absolute (i.e. ala "cygwin") paths are like "/c/path/to/foo" and not "/cygdrive/c/path/to/foo". However, msys accept and understand "c:/path/to/foo". Will attach a patch.
Attached patch patch (deleted) — Splinter Review
Add "-m" option to the cygpath commands when we are under msys.
Assignee: nobody → regis.caspar+bz
Status: NEW → ASSIGNED
Attachment #223171 - Flags: review?(robert.bugzilla)
Can this just be "shell cygpath -t mixed -a..." or am I missing something? Looks like xpinstall does this though I am unsure of what -ai does in addition to. http://lxr.mozilla.org/seamonkey/source/xpinstall/packager/windows/Makefile.in#47
(In reply to comment #2) > Can this just be "shell cygpath -t mixed -a..." or am I missing something? -t mixed and -m are similar. From cygpath --help: -i, --ignore ignore missing argument I proposed msys detection in the patch because original code don't use -m and I don't know if it's ok under cygwin to use -m.
Are there any other cygpath call sites in the tree (e.g. search on cygpath in lxr.mozilla.org) broken with msys as well?
(In reply to comment #4) > Are there any other cygpath call sites in the tree (e.g. search on cygpath in > lxr.mozilla.org) broken with msys as well? For Firefox modules, no (I don't know for everything else because I only make Firefox builds). For this issue, if you are sure -m isn't a problem under cygwin, the patch could become trivial (just s/cygpath -a/cygpath -ma/)
I'll download msys and verify if -m works properly with both. The introduction of the cygpath command actually happened quite some time ago well before the landing of building using NSIS for the installer so I am removing the blocks. http://bonsai.mozilla.org/cvsblame.cgi?file=mozilla/browser/installer/windows/Makefile.in&rev=1.29&root=/cvsroot
No longer blocks: 326580, 339061
Using -ma works fine for me with cygwin. Could you also provide a patch for mail to the same file for consistency's sake? If so, request review from mscott for that patch.
Attachment #223171 - Flags: review?(robert.bugzilla) → review-
Attached patch patch v2 (deleted) — Splinter Review
Patch changing "cygpath -a" to "cygpath -ma" for mail/installer/windows/Makefile.in and browser/installer/windows/Makefile.in For the record, http://lxr.mozilla.org/seamonkey/search?string=cygpath shows some more "-m potential" like: /directory/c-sdk/configure.in, line 894 -- WIN_TOP_SRC=`cygpath -w $srcdir | sed -e 's|\\\\|/|g'` => WIN_TOP_SRC=`cygpath -m $srcdir" ...
Attachment #223663 - Flags: review?
Attachment #223663 - Flags: review? → review?(mscott)
I found some more glitch preventing to build the installer on win32 under msys. Patch v2 works here because I have some more file patched: there is a lot of perl script dieing if ($O =~ /msys/). Additionally, I made a builds today and after a successful build I tried making installers. I ended in a cmd shell under msys !? I then made a lxr search on "cmd /C" (http://lxr.mozilla.org/seamonkey/search?string=cmd+%2FC) and found some results. I don't think the building process should require cmd to work, why not rather use cygwin or msys shell (i.e. sh)? So there is two possibilities, either msys support is added or it is discarded and this bug would becomes INVALID. I will cancel review on patch 2 for now, update summary and attach a diff.
Attachment #223663 - Flags: review?(mscott)
Attached patch diff (cf. comment #9) (deleted) — Splinter Review
Summary: build_static.pl fails under msys because it dislikes /cygdrive → Building installer should be possible under msys
I think this would be a good thing to do but I'd like to audit the places that use cygpath, cmd.exe (if there are any others) throughout the tree and make sure they all work with msys vs. fixing them in multiple patches. If someone took that on I'd be glad to review, help get the module owners to review, etc. Also, I have read that accessibility doesn't build without msvc and I'd like to know how / if the NSIS installer handles that properly or should it also be handled differently.
(In reply to comment #11) > I think this would be a good thing to do but I'd like to audit the places that > use cygpath, cmd.exe (if there are any others) throughout the tree and make > sure they all work with msys vs. fixing them in multiple patches. There's only a few cmd in the tree. Most are "handled" in the diff I added. All build fine here (msys / VC8) the only tests I didn't made is the locales repackaging (browser|mail)/locales/Makefile.in, a TB installer build and cat -B file-1 file-2 > file under cygwin (I don't have it). Here I don't want to point at cygpath usage, it's a requirement and that's OK, but about cmd!? we have a "powerful" shell (sh) but we use another "simple" one (cmd). In a more critical point of view why a perl script require the use of a windows batch?? > If someone took that on I'd be glad to review, help get the module owners to > review, etc. OK, most of the msys support is already in the diff I attached. I'd be glad to help/take it but I will probably need some direction. -> reassign to default
Assignee: regis.caspar+bz → nobody
Status: ASSIGNED → NEW
(In reply to comment #12) > the only tests I didn't made is the locales repackaging > (browser|mail)/locales/Makefile.in, a TB installer build and cat -B > file-1 file-2 > file under cygwin (I don't have it). And I didn't test anything under OS2 (I don't have it either)
What would you replace cmd /C copy /b with? Why doesn't "cmd" work with MSYS?
(In reply to comment #14) > What would you replace cmd /C copy /b with? cat -B > Why doesn't "cmd" work with MSYS? That's a good question. The exact problem is that cmd doesn't exit back to sh, so you end into a cmd session Note: $ sh --version GNU bash, version 2.04.0(1)-release (i686-pc-msys) Copyright 1999 Free Software Foundation, Inc.
Regis, the build config for the installer has changed quite a lot since this bug was reported. Is this still a problem?
(In reply to comment #16) > Regis, the build config for the installer has changed quite a lot since this > bug was reported. Is this still a problem? I will check and report. If the diff (third attachment) is correct, the problem still exists ($^O = 'msys' under MSYS and cmd.exe cannot be "cast"). BTW: (In reply to comment #15) > > What would you replace cmd /C copy /b with? > cat -B the -B is a win32 only flag which is useless here
(In reply to comment #17) > I will check and report. Robert, I can build dist/install/sea/firefox-3.0a1.en-US.win32.installer.exe with make -C browser/installer installer without any problem. The content of the built one match the content (file names / directories) of the official nightly installer. So this seems fixed. -> WORKSFORME
Status: NEW → RESOLVED
Closed: 18 years ago
Resolution: --- → WORKSFORME
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: