Closed
Bug 339096
Opened 18 years ago
Closed 18 years ago
Building installer should be possible under msys
Categories
(Firefox :: Installer, defect)
Tracking
()
RESOLVED
WORKSFORME
People
(Reporter: regis.caspar+bz, Unassigned)
Details
Attachments
(3 files)
(deleted),
patch
|
robert.strong.bugs
:
review-
|
Details | Diff | Splinter Review |
(deleted),
patch
|
Details | Diff | Splinter Review | |
(deleted),
patch
|
Details | Diff | Splinter Review |
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.
Reporter | ||
Comment 1•18 years ago
|
||
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)
Comment 2•18 years ago
|
||
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
Reporter | ||
Comment 3•18 years ago
|
||
(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.
Comment 4•18 years ago
|
||
Are there any other cygpath call sites in the tree (e.g. search on cygpath in lxr.mozilla.org) broken with msys as well?
Reporter | ||
Comment 5•18 years ago
|
||
(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/)
Comment 6•18 years ago
|
||
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
Comment 7•18 years ago
|
||
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.
Updated•18 years ago
|
Attachment #223171 -
Flags: review?(robert.bugzilla) → review-
Reporter | ||
Comment 8•18 years ago
|
||
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?
Reporter | ||
Updated•18 years ago
|
Attachment #223663 -
Flags: review? → review?(mscott)
Reporter | ||
Comment 9•18 years ago
|
||
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.
Reporter | ||
Updated•18 years ago
|
Attachment #223663 -
Flags: review?(mscott)
Reporter | ||
Comment 10•18 years ago
|
||
Reporter | ||
Updated•18 years ago
|
Summary: build_static.pl fails under msys because it dislikes /cygdrive → Building installer should be possible under msys
Comment 11•18 years ago
|
||
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.
Reporter | ||
Comment 12•18 years ago
|
||
(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
Reporter | ||
Comment 13•18 years ago
|
||
(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)
Comment 14•18 years ago
|
||
What would you replace cmd /C copy /b with? Why doesn't "cmd" work with MSYS?
Reporter | ||
Comment 15•18 years ago
|
||
(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.
Comment 16•18 years ago
|
||
Regis, the build config for the installer has changed quite a lot since this bug was reported. Is this still a problem?
Reporter | ||
Comment 17•18 years ago
|
||
(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
Reporter | ||
Comment 18•18 years ago
|
||
(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.
Description
•