The default bug view has changed. See this FAQ.
Bug 294122 (msys)

Changes needed for building with MSYS

VERIFIED FIXED

Status

()

Core
Build Config
--
enhancement
VERIFIED FIXED
12 years ago
9 years ago

People

(Reporter: Howard Chu, Assigned: Howard Chu)

Tracking

Trunk
x86
Windows XP
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(5 attachments, 28 obsolete attachments)

10.77 KB, patch
cls
: review+
Details | Diff | Splinter Review
964 bytes, text/plain
Details
18.50 KB, patch
cls
: review+
Details | Diff | Splinter Review
1.57 KB, patch
cls
: review+
Details | Diff | Splinter Review
1.74 KB, patch
Howard Chu
: review+
Julien Pierre
: review+
Details | Diff | Splinter Review
(Assignee)

Description

12 years ago
User-Agent:       Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.8b2) Gecko/20050513
Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.8b2) Gecko/20050513

There are a number of hurdles to cross before you can build successfully with
MSYS, but I believe it is worth the effort. A full compile of the Suite+Calendar
that takes 37 minutes on my system using Cygwin takes only 26 minutes with MSYS.
Just running "make clean" on Cygwin takes almost 6 minutes, vs only 1 minute
with MSYS.

I'll be posting patches for this in stages as I try to streamline the process.

Reproducible: Always
(Assignee)

Comment 1

12 years ago
Created attachment 183563 [details] [diff] [review]
Fix inconsistent option switches

Because MSYS mangles any commandline argument that contains a slash, all
commandline options need to use dash instead. All the Windows utilities already
recognize both, and the existing build has been using slashes and dashes mixed
indiscriminately. Changing everything to dashes does not break the Cygwin
build.

Updated

12 years ago
Status: UNCONFIRMED → NEW
Component: Build Config → Build Config
Ever confirmed: true
Product: Mozilla Application Suite → Core
Version: unspecified → Trunk
(Assignee)

Comment 2

12 years ago
Created attachment 183578 [details] [diff] [review]
More comprehensive patch

This patch includes all the changes of the previous patch plus the remaining
configurey needed to get a working build. I'll be attaching detailed build
instructions separately. In brief, you need MSYS, MSYS_DTK, and a zip/unzip
utility. Currently I use the Cygwin zip tools, but I've just downloaded 7zip to
try that out, I expect it will work fine. You also need a whoami command; I'm
using a simple shell script "echo $USERNAME" which seems to be sufficient.

The changes in this patch are mainly of three types:
  option switch cleanup - using dashes everywhere
  uname tweaks - parsing MSYS's uname output correctly
  cygpath tweaks - removing any Cygwin-style path-munging, since MSYS does it
automatically.

There's also related perl script changes to make sure that MSYS perl is
recognized, otherwise the jar files for chrome aren't correct.
(Assignee)

Updated

12 years ago
Attachment #183563 - Attachment is obsolete: true
(Assignee)

Comment 3

12 years ago
Created attachment 183582 [details] [diff] [review]
Correct attachment

Sorry, the previous attachment was the wrong file, had some typos in it. This
one is right.
Attachment #183578 - Attachment is obsolete: true
(Assignee)

Updated

12 years ago
Attachment #183582 - Flags: review?(cls)

Updated

12 years ago
Alias: msys

Comment 4

12 years ago
Comment on attachment 183582 [details] [diff] [review]
Correct attachment

Thanks for the patch.  At first glance, it seems that there were side-effects
to adding msys to the uname checks.  In particular, OS_TARGET isn't being set
to WINNT in some cases.  It looks like you're changing the library naming
scheme that we decided to use in bug 134113#c301 .

You should break up the patch so that changes can be reviewed independently.  I
would suggest separate patches for:
- the commandline args change
- adding msys uname support
- adding support for msys perl
- the NSS changes

. And you don't need to include the configure changes in the patches.
Attachment #183582 - Flags: review?(cls)
(Assignee)

Comment 5

12 years ago
(In reply to comment #4)
> (From update of attachment 183582 [details] [diff] [review] [edit])
> Thanks for the patch.  At first glance, it seems that there were side-effects
> to adding msys to the uname checks.  In particular, OS_TARGET isn't being set
> to WINNT in some cases.  It looks like you're changing the library naming
> scheme that we decided to use in bug 134113#c301 .
> 
> You should break up the patch so that changes can be reviewed independently.

OK, willdo. As for changing the naming scheme, that was unintentional. I was
getting inconsistent results of whether the "lib" prefix was used or not in
various places, and it seemed the correct thing to do was to make sure the knobs
(e.g. $(LIB_PREFIX)) were being used everywhere, rather than the hardcoded usage.

It turns out that the proposed patch was still incomplete; after I removed
/c/cygwin/bin from my path I ran into several uses of cygpath that I hadn't
caught before. Still trying to nail that down.
(Assignee)

Comment 6

12 years ago
Created attachment 183614 [details] [diff] [review]
Option switch cleanup

OK, this patch only deals with the option switches, slash to dash.
Attachment #183582 - Attachment is obsolete: true
(Assignee)

Updated

12 years ago
Attachment #183614 - Flags: review?(cls)
(Assignee)

Comment 7

12 years ago
Created attachment 183615 [details] [diff] [review]
MSYS uname support

This is just the diffs to uname parsing, to handle MSYS.
(Assignee)

Updated

12 years ago
Attachment #183615 - Flags: review?(cls)
(Assignee)

Comment 8

12 years ago
Created attachment 183616 [details] [diff] [review]
MSYS perl dependencies

Changes for the perl scripts to recognize MSYS perl, and to remove cygpath
dependencies.
(Assignee)

Comment 9

12 years ago
Created attachment 183617 [details] [diff] [review]
Fix cygpath dependencies

This patch eliminates usage of cygpath/cygwin_wrapper on MSYS and fixes use of
cygpath in other places.
(Assignee)

Updated

12 years ago
Attachment #183616 - Flags: review?(cls)
(Assignee)

Updated

12 years ago
Attachment #183617 - Flags: review?(cls)
(Assignee)

Comment 10

12 years ago
Created attachment 183618 [details]
Notes on building with MSYS

Some build notes for the Win32 build page
http://www.mozilla.org/build/win32.html
Comment on attachment 183618 [details]
Notes on building with MSYS

>Software Requirements
>Common Problems and Hints
>  building with an objdir outside of the source tree will produce invalid JAR files for the chrome. Using an objdir under the top level of the source tree works.


This is unnacceptable for most users (at the very least myself).

All my object directories live outside my source tree, so that for example when
I modify stuff, or corrupt my pull it is sometimes easier for me just to nuke
my source-tree entirely and re-pull than it is otherwise...also I have so many
object directories that it would clutter my sources themselves, nevermind the
desire to keep my objectdir's common which include some stable/other code
branches as-built.

I would highly suggest investigating why, and fixing this.

[Disclaimer: I am not a reviewer or driver of any sort]
Comment on attachment 183616 [details] [diff] [review]
MSYS perl dependencies

>Index: config/preprocessor.pl
>===================================================================
>-        s/[\x0D\x0A]+$/\n/os if ($^O eq 'cygwin' || "$^O" eq "MSWin32");
>+        s/[\x0D\x0A]+$/\n/os if ($^O eq 'cygwin' || "$^O" eq "MSWin32" || "$^O" eq 'msys');

If MSys is meant to be the preferred (and faster) build method after these
patches, why not make the "$^O" eq 'msys' the first expression tested here
(amoung all or's), for microsecond optimization per preprocessor run?
Comment on attachment 183617 [details] [diff] [review]
Fix cygpath dependencies

>Index: configure.in
>===================================================================
>@@ -884,6 +887,11 @@
>     HOST_NSPR_MDCPUCFG='\"md/_winnt.cfg\"'
>     HOST_OPTIMIZE_FLAGS="${HOST_OPTIMIZE_FLAGS=-O2}"
>     HOST_BIN_SUFFIX=.exe
>+    case "$host" in
>+    *mingw*)
>+dnl MinGW/MSYS does not need CYGWIN_WRAPPER
>+        ;;
>+    *)
>     CYGWIN_WRAPPER="${srcdir}/build/cygwin-wrapper"
>     if test "`echo ${srcdir} | grep -c ^/ 2>/dev/null`" = 0; then
>         _pwd=`pwd`
>@@ -894,6 +902,8 @@
>         PERL="${CYGWIN_WRAPPER} $PERL"
>     fi
>     ;;
>+    esac
>+    ;;

Personal Nit: Fix indenting around here, with your additions this section of
configure.in is harder to comprehend.
Assignee: nobody → hyc
(Assignee)

Comment 14

12 years ago
(In reply to comment #13)
> (From update of attachment 183617 [details] [diff] [review] [edit])
> >Index: configure.in
> >===================================================================
> >@@ -884,6 +887,11 @@
> >     HOST_NSPR_MDCPUCFG='\"md/_winnt.cfg\"'
> >     HOST_OPTIMIZE_FLAGS="${HOST_OPTIMIZE_FLAGS=-O2}"
> >     HOST_BIN_SUFFIX=.exe
> >+    case "$host" in
> >+    *mingw*)
> >+dnl MinGW/MSYS does not need CYGWIN_WRAPPER
> >+        ;;
> >+    *)
> >     CYGWIN_WRAPPER="${srcdir}/build/cygwin-wrapper"
> >     if test "`echo ${srcdir} | grep -c ^/ 2>/dev/null`" = 0; then
> >         _pwd=`pwd`
> >@@ -894,6 +902,8 @@
> >         PERL="${CYGWIN_WRAPPER} $PERL"
> >     fi
> >     ;;
> >+    esac
> >+    ;;
> 
> Personal Nit: Fix indenting around here, with your additions this section of
> configure.in is harder to comprehend.
> 

Hm, I actually did that already, but I used "diff -wu" to keep the diffs
smaller. I can regenerate it without "-w" if it matters.

re: comment #12 sounds good to me.

re: comment #11 yes, you're right, it needs to be fixed. At the moment it
appears to be something flaky with the zip utility, but I don't have any
detailed info yet.
(Assignee)

Updated

12 years ago
Attachment #183616 - Flags: review?(cls)
(Assignee)

Comment 15

12 years ago
(In reply to comment #11)
> (From update of attachment 183618 [details] [edit])
> >Software Requirements
> >Common Problems and Hints
> >  building with an objdir outside of the source tree will produce invalid JAR
files for the chrome. Using an objdir under the top level of the source tree works.
> 
> 
> This is unnacceptable for most users (at the very least myself).
> 
> All my object directories live outside my source tree, so that for example when
> I modify stuff, or corrupt my pull it is sometimes easier for me just to nuke
> my source-tree entirely and re-pull than it is otherwise...also I have so many
> object directories that it would clutter my sources themselves, nevermind the
> desire to keep my objectdir's common which include some stable/other code
> branches as-built.
> 
> I would highly suggest investigating why, and fixing this.
> 
> [Disclaimer: I am not a reviewer or driver of any sort]
> 

In case anyone gets any inspiration before I do, here is the diff of a
successful build using an objdir inside the source tree vs the objdir outside
the source tree. I edited the objdir paths in the successful build log to be the
same as the other so they would be omitted from the diff. As you can see, the
only difference is 176 vs 208 bytes in the Add operations on each archive.

http://www.highlandsun.com/hyc/Edif2.txt
(In reply to comment #15)
> As you can see, the
> only difference is 176 vs 208 bytes in the Add operations on each archive.
> 
> http://www.highlandsun.com/hyc/Edif2.txt

Could you show us the unmodified content of one of the couple of files, too ?
(In reply to comment #16)
> Could you show us the unmodified content of one of the couple of files, too ?

Well, a file diff would do instead of course :->
(Assignee)

Comment 18

12 years ago
(In reply to comment #17)
> (In reply to comment #16)
> > Could you show us the unmodified content of one of the couple of files, too ?
> 
> Well, a file diff would do instead of course :->

Strangely enough, when I rebuilt again, there was no problem with the resulting
files. I'm repeating the build one more time to verify. Has anyone else tried
building with the patches yet?
(Assignee)

Comment 19

12 years ago
Created attachment 183644 [details] [diff] [review]
MSYS perl dependencies, again

Same as before, but with comment #12 (if conditions reordered).
Attachment #183616 - Attachment is obsolete: true
(Assignee)

Updated

12 years ago
Attachment #183644 - Flags: review?(cls)
(Assignee)

Comment 20

12 years ago
Created attachment 183646 [details]
MSYS build notes, again

Since I could not reproduce the JAR problem I mentioned before, I've deleted
that bit from the build notes.
(Assignee)

Updated

12 years ago
Attachment #183618 - Attachment is obsolete: true

Updated

12 years ago
Attachment #183614 - Flags: review?(cls) → review+

Comment 21

12 years ago
Comment on attachment 183617 [details] [diff] [review]
Fix cygpath dependencies

Older versions of cygpath (from cygwin < 1.3.13 iirc) do not support the -m
option.  I'm not sure how old the cygwin install is on the tinderboxes now but
we probably shouldn't break that.  Or add a check if we do decide to require
the new version.
Attachment #183617 - Flags: review?(cls) → review-

Updated

12 years ago
Attachment #183644 - Flags: review?(cls) → review+
(Assignee)

Comment 22

12 years ago
(In reply to comment #21)
> (From update of attachment 183617 [details] [diff] [review] [edit])
> Older versions of cygpath (from cygwin < 1.3.13 iirc) do not support the -m
> option.  I'm not sure how old the cygwin install is on the tinderboxes now but
> we probably shouldn't break that.  Or add a check if we do decide to require
> the new version.
> 
The xpinstall/packager/windows Makefile already uses the "-t mixed" option which
is the same thing. Why isn't that already a problem then?

Comment 23

12 years ago
(In reply to comment #15)
>In case anyone gets any inspiration before I do, here is the diff of a
>successful build using an objdir inside the source tree vs the objdir outside
>the source tree. I edited the objdir paths in the successful build log to be the
>same as the other so they would be omitted from the diff. As you can see, the
>only difference is 176 vs 208 bytes in the Add operations on each archive.
We really ought to add -X to all zip commands; that would fix this too.
just to be sure: The patch does not break the cygwin/msvc combination, right?

Does it allow building with msys/msvc?
(Assignee)

Comment 25

12 years ago
(In reply to comment #24)
> just to be sure: The patch does not break the cygwin/msvc combination, right?
> 
> Does it allow building with msys/msvc?

Right. I am using MSVC, VC6 in fact. I get a successful build with both Cygwin
and MSYS. I haven't looked at building with gcc, but it should be unaffected.

Comment 26

12 years ago
> The xpinstall/packager/windows Makefile already uses the "-t mixed" option which
> is the same thing. Why isn't that already a problem then?

That makefile only gets used if you're making packages for the installer whereas
the changes in the patch will affect all builds.

Btw, my build died when building the jar files.  When I try to invoke zip
(infozip or cygwin) from the msys shell, it appears to be trying to create a zip
file on stdout rather than printing the 'Usage:' message.  I installed msys
1.0.10, msysDTK 1.0.1, infozip zip 2.3.1 & unzip 5.5.2.

When I moved the infozip files out of c:\msys\1.0\bin, then make-jars.pl
completely successfully.  I still get the garbage when just invoking 'zip' on
the commandline though.
(Assignee)

Comment 27

12 years ago
(In reply to comment #26)
> > The xpinstall/packager/windows Makefile already uses the "-t mixed" option which
> > is the same thing. Why isn't that already a problem then?
> 
> That makefile only gets used if you're making packages for the installer whereas
> the changes in the patch will affect all builds.

OK, I'll rework this patch, I still have the previous version handy.

> Btw, my build died when building the jar files.  When I try to invoke zip
> (infozip or cygwin) from the msys shell, it appears to be trying to create a zip
> file on stdout rather than printing the 'Usage:' message.  I installed msys
> 1.0.10, msysDTK 1.0.1, infozip zip 2.3.1 & unzip 5.5.2.
> 
> When I moved the infozip files out of c:\msys\1.0\bin, then make-jars.pl
> completely successfully.  I still get the garbage when just invoking 'zip' on
> the commandline though.

Right, the MSYS /bin directory is reserved for actual MSYS code, i.e. programs
that are built with the MSYS DLL. I created a /usr/local/bin on my system for
things like zip and whoami. The MSYS docs warn you not to put other binaries in
/bin but I guess I should add that to the build notes.

I see a similar behavior from running zip without arguments on the command line. 
It doesn't do that when run using the Windows shell, so there's definitely
something funny there. Fortunately it has no impact on the build since it's
never invoked without arguments.
(Assignee)

Comment 28

12 years ago
Created attachment 183682 [details] [diff] [review]
Cygpath fixes, again

Leaves original "cygpath -w | sed" behavior intact.
Attachment #183617 - Attachment is obsolete: true
(Assignee)

Comment 29

12 years ago
Created attachment 183683 [details] [diff] [review]
Cygpath fixes...

Still had cygpath -m in a couple places, oops.
Attachment #183682 - Attachment is obsolete: true
(Assignee)

Updated

12 years ago
Attachment #183683 - Flags: review?(cls)
(Assignee)

Comment 30

12 years ago
Comment on attachment 183683 [details] [diff] [review]
Cygpath fixes...

missing an endif in xpinstall/packager/windows/Makefile.in
Attachment #183683 - Flags: review?(cls) → review-
(Assignee)

Comment 31

12 years ago
Comment on attachment 183683 [details] [diff] [review]
Cygpath fixes...

missing an endif in xpinstall/packager/windows/Makefile.in
Attachment #183683 - Flags: review-
(Assignee)

Comment 32

12 years ago
Created attachment 183685 [details] [diff] [review]
Cygpath fixes - once more, with feeling

Add missing endif
Attachment #183683 - Attachment is obsolete: true
(Assignee)

Updated

12 years ago
Attachment #183685 - Flags: review?(cls)

Comment 33

12 years ago
Btw, the changes do break the msys/mingw gcc build.  When NSS is creating its
shared libraries, it's is passing the full path of IMPORT_LIB to gcc as
-Wl,--out-implib,$(IMPORT_LIBRARY).  gcc gets confused by the path that's given
since the path isn't being translated by the msys shell.

When calling configure with an absolute path, the build breaks in xpcom with a
similar issue.  -Wl,--version-script,/abs/path/to/script is given and mingw ld
is choking on the path.
(Assignee)

Comment 34

12 years ago
Comment on attachment 183615 [details] [diff] [review]
MSYS uname support

Looking this over, I think the CYGWIN_ME cases should be CYGWIN_ME* but I'm not
sure. Does anyone out there have a WinME setup that can verify? Or we can just
change it to that, to be safe.
(Assignee)

Comment 35

12 years ago
(In reply to comment #33)
> Btw, the changes do break the msys/mingw gcc build.

While setting up a gcc build, I saw an error message from "test" go by.

/home/hyc/mozilla/nsprpub/configure: test: : integer expression expected

There is a clause in the common Win32 path that should only be there for the
MSVC build. Should I file this separately, or can we take care of it here?

Index: configure.in
===================================================================
RCS file: /cvsroot/mozilla/nsprpub/configure.in,v
retrieving revision 1.83.2.99
diff -u -r1.83.2.99 configure.in
--- configure.in        11 May 2005 18:00:17 -0000      1.83.2.99
+++ configure.in        16 May 2005 04:37:52 -0000
@@ -1351,6 +1360,11 @@
             DLLFLAGS="$DLLFLAGS -DEBUG"
             LDFLAGS="$LDFLAGS -DEBUG"
         fi
+
+       OS_DLLFLAGS="-nologo -DLL -SUBSYSTEM:WINDOWS"
+       if test "$MSC_VER" -le "1200" -a -z "$MOZ_DEBUG_SYMBOLS"; then
+           OS_DLLFLAGS="$OS_DLLFLAGS -PDB:NONE"
+       fi
         
         if test "$OS_TARGET" = "WINNT"; then
             CFLAGS="$CFLAGS -GT"
@@ -1388,11 +1402,6 @@
         OBJDIR_SUFFIX=OBJD
     fi
 
-    OS_DLLFLAGS="-nologo -DLL -SUBSYSTEM:WINDOWS"
-    if test "$MSC_VER" -le "1200" -a -z "$MOZ_DEBUG_SYMBOLS"; then
-        OS_DLLFLAGS="$OS_DLLFLAGS -PDB:NONE"
-    fi
-
     case "$OS_TARGET" in
     WINNT)
            MDCPUCFG_H=_winnt.cfg


Obviously MSC_VER is not defined when building with gcc. I traced the
OS_DLLFLAGS macro and it is only used by LINK_DLL, which is not used for gcc builds.

Updated

12 years ago
Attachment #183615 - Flags: superreview?(wtchang)
Attachment #183615 - Flags: review?(cls)
Attachment #183615 - Flags: review+
(Assignee)

Comment 36

12 years ago
Created attachment 183699 [details] [diff] [review]
MSYS uname, with CYGWIN_ME correction

Per comment #34, use CYGWIN_ME* in the case statements. Already approved by
cls.
Attachment #183615 - Attachment is obsolete: true
(Assignee)

Updated

12 years ago
Attachment #183699 - Flags: superreview?(wtchang)
Attachment #183699 - Flags: review+
(Assignee)

Updated

12 years ago
Attachment #183615 - Flags: superreview?(wtchang)
(Assignee)

Comment 37

12 years ago
(In reply to comment #33)
> Btw, the changes do break the msys/mingw gcc build.  When NSS is creating its
> shared libraries, it's is passing the full path of IMPORT_LIB to gcc as
> -Wl,--out-implib,$(IMPORT_LIBRARY).  gcc gets confused by the path that's given
> since the path isn't being translated by the msys shell.
> 
> When calling configure with an absolute path, the build breaks in xpcom with a
> similar issue.  -Wl,--version-script,/abs/path/to/script is given and mingw ld
> is choking on the path.

I didn't have this problem. It built cleanly and I'm using the result to enter
this message now. I'm guessing you used the Cygwin gcc with MinGW support, and
not the actual MinGW gcc. I used a pure MinGW/MSYS toolchain.
(Assignee)

Comment 38

12 years ago
Comment on attachment 183685 [details] [diff] [review]
Cygpath fixes - once more, with feeling

Have one more slight improvement...
Attachment #183685 - Flags: review?(cls)
(Assignee)

Comment 39

12 years ago
Created attachment 183710 [details] [diff] [review]
Cygpath fixes, better use of gmake

Skip another sed invocation. We could use bash's $PWD too, really, instead of
`pwd` e.g. $(shell cygpath -w $$PWD) but it's probably not critical.
Attachment #183685 - Attachment is obsolete: true
(Assignee)

Comment 40

12 years ago
Created attachment 183711 [details] [diff] [review]
Cygpath fixes, with gmake again

Eliminate a couple calls to pwd
(Assignee)

Updated

12 years ago
Attachment #183710 - Attachment is obsolete: true
(Assignee)

Comment 41

12 years ago
Created attachment 183714 [details] [diff] [review]
Cygpath fixes, with typos corrected

Sorry for the typos, this one works.
Attachment #183711 - Attachment is obsolete: true
(Assignee)

Updated

12 years ago
Attachment #183714 - Flags: review?(cls)

Comment 42

12 years ago
Actually, I was using the pure mingw/msys toolchain as well.  I guess I forgot
to mention that I was building (as always) in an objdir outside of the source
tree.  I believe that you won't run into the NSS problem when building in the
source tree.  And you won't run into the version-script issue if you don't call
configure with an absolute path.

(Assignee)

Comment 43

12 years ago
(In reply to comment #42)
> Actually, I was using the pure mingw/msys toolchain as well.  I guess I forgot
> to mention that I was building (as always) in an objdir outside of the source
> tree.  I believe that you won't run into the NSS problem when building in the
> source tree.  And you won't run into the version-script issue if you don't call
> configure with an absolute path.

OK, I'll try it again. I was using an objdir outside the source tree, and I
didn't run configure by hand. Perhaps seeing your mozconfig options might help
make things clearer. Mine had:

mk_add_options MOZ_CO_PROJECT=suite,calendar
#mk_add_options MOZ_OBJDIR=/cygdrive/d/home/hyc/mozobc
mk_add_options MOZ_OBJDIR=/home/hyc/mozob2
#ac_add_options --enable-crypto --enable-application=suite --enable-calendar
--enable-win32-target=WINNT
ac_add_options --enable-crypto --enable-application=suite --enable-calendar
ac_add_options --disable-accessibility --disable-activex

(In MSYS I mount D:\home as /home. The source tree is /home/hyc/mozilla.)
With these options, I cd to mozilla and run make -f client.mk build and
everything works.

Comment 44

12 years ago
Comment on attachment 183646 [details]
MSYS build notes, again

We should eliminate the "whoami" requirement by using
$USERNAME in our makefiles and configure scripts on
Windows.

Comment 45

12 years ago
I think it's the extra mount point that's making the difference.  In my case,
I'm building under c:/root/obj-msys and c:/root isn't mounted under msys.  gcc
is given & chokes on /c/root/obj-msys/nss/softokn/softokn3.a . In your case, I
think gcc sees d:/home and /home as the same thing.  

export MOZCONFIG=/c/cygwin/home/cls/.mozconfig
### START
# mingw
CC=gcc
CXX=g++
CPP=cpp
AS=as
LD=ld
ac_add_options --disable-activex
ac_add_options --disable-activex-scripting
ac_add_options --disable-accessibility

mk_add_options MOZ_CO_PROJECT=suite,browser,mail
mk_add_options MOZ_OBJDIR=c:/root/obj-test

#MOZ_INTERNAL_LIBART_LGPL=1
#mk_add_options MOZ_INTERNAL_LIBART_LGPL=1
#mk_add_options MOZ_PHOENIX=1
#mk_add_options MOZ_THUNDERBIRD=1
#mk_add_options MOZ_CO_MODULE="SeaMonkeyAll mozilla/tools/trace-malloc"
ac_add_options --disable-debug
ac_add_options --enable-optimize

ac_add_options --enable-application=suite
ac_add_options --disable-ldap
ac_add_options --disable-oji
ac_add_options --disable-jsd
ac_add_options --disable-mailnews
ac_add_options --enable-extensions='cookie wallet irc'
ac_add_options --enable-crypto
### END
(Assignee)

Comment 46

12 years ago
(In reply to comment #44)
> (From update of attachment 183646 [details] [edit])
> We should eliminate the "whoami" requirement by using
> $USERNAME in our makefiles and configure scripts on
> Windows.

As it turns out, whoami / $WHOAMI is only referenced in the configure scripts to
generate
  DEFINES="$DEFINES -DDEBUG_`$WHOAMI`"
and does not get used directly in any Makefiles, so eliminating this is easy.

re: comment #45 if adding the mountpoint does make the difference I'll add that
to the build notes and consider the problem solved. OK?
(Assignee)

Comment 47

12 years ago
(In reply to comment #46)
> (In reply to comment #44)
> > (From update of attachment 183646 [details] [edit] [edit])
> > We should eliminate the "whoami" requirement by using
> > $USERNAME in our makefiles and configure scripts on
> > Windows.

My mistake. There is already a special case for Windows, whoami is not needed.
Status: NEW → ASSIGNED
(Assignee)

Comment 48

12 years ago
(In reply to comment #45)
> I think it's the extra mount point that's making the difference.  In my case,
> I'm building under c:/root/obj-msys and c:/root isn't mounted under msys.  gcc
> is given & chokes on /c/root/obj-msys/nss/softokn/softokn3.a . In your case, I
> think gcc sees d:/home and /home as the same thing.  

Yes, that's it. I set MOZ_OBJDIR=/d/home/hyc/mozobj and did a "cd
/d/home/hyc/mozilla" to start the build and it failed as you describe. I'll add
a note that you cannot allow drive letters to appear in any paths. That means
you must create a mount point for the top level directory. I also tried an MSVC
build with two mountpoints on separate drives. That failed to link because it
couldn't find a .def file. This unfortunately means that the source and object
must reside on the same drive. I'll keep looking around for a solution but that
seems to be the way it goes for now.
(Assignee)

Comment 49

12 years ago
Created attachment 183749 [details]
MSYS build notes, updated

Mount point requirements...
Attachment #183646 - Attachment is obsolete: true
(Assignee)

Comment 50

12 years ago
Comment on attachment 183714 [details] [diff] [review]
Cygpath fixes, with typos corrected

This patch is corrupt. Re-uploading.
Attachment #183714 - Flags: review?(cls)
(Assignee)

Comment 51

12 years ago
Created attachment 183788 [details] [diff] [review]
Cygpath fixes, patch fixed (checked in, except NSS changes)

Fix corruption in previous diff
Attachment #183714 - Attachment is obsolete: true
(Assignee)

Updated

12 years ago
Attachment #183788 - Flags: review?(cls)
(Assignee)

Comment 52

12 years ago
Created attachment 183790 [details] [diff] [review]
More path fixes

Addresses the points in comment #33 and comment #35. Allows source and object
directories to reside on separate drives.
(Assignee)

Comment 53

12 years ago
Created attachment 183791 [details] [diff] [review]
More path fixes

Fix patch context
Attachment #183790 - Attachment is obsolete: true
(Assignee)

Comment 54

12 years ago
Created attachment 183792 [details]
MSYS build notes, more updates

Updated to correspond to Attachment #183791 [details] [diff]. By the way, all four of the
previous patches must be applied before applying the patch in #183791.
(Assignee)

Updated

12 years ago
Attachment #183749 - Attachment is obsolete: true
(Assignee)

Updated

12 years ago
Attachment #183791 - Flags: review?(cls)
(Assignee)

Updated

12 years ago
Attachment #183792 - Flags: review?(benjamin)

Comment 55

12 years ago
Comment on attachment 183699 [details] [diff] [review]
MSYS uname, with CYGWIN_ME correction

Howard, thanks a lot for the patch.

I'd like to suggest some changes.

1. You didn't list yourself as a contributor in
mozilla/configure.in.  Is this an oversight or did
you do that in another patch?

2. I suggest that we take the opportunity to remove
the code that handles Cygwin prior to the Beta 20
release.  One reason is that your code that defines
OS_RELEASE won't work if $OS_ARCH is "CYGWIN32_NT".

3. I don't want the WINCE code in mozilla/nsprpub/configure.in.
OS_ARCH has a slightly different meaning in NSPR, so
we may not be able to copy that code from mozilla/configure.in
without changes.  I will add the necessary WINCE code to
NSPR when I check in the WINCE porting patch.

4. In mozilla/security/coreconf/arch.mk, I'd be interested
in finding out what Cygwin's and MSYS's uname -m return on
your machine.  (Pentium Pro is too old...)
Attachment #183699 - Flags: superreview?(wtchang) → superreview+
(Assignee)

Comment 56

12 years ago
Created attachment 183855 [details] [diff] [review]
make-jar patch to exclude attrs

As per comment #23, add the -X option to the zip command to exclude any
additional file attributes from the archive.
(Assignee)

Comment 57

12 years ago
(In reply to comment #55)
> (From update of attachment 183699 [details] [diff] [review] [edit])
> Howard, thanks a lot for the patch.
> 
> I'd like to suggest some changes.

> 1. You didn't list yourself as a contributor in
> mozilla/configure.in.  Is this an oversight or did
> you do that in another patch?

I did that in a prior patch, attachment #183614 [details] [diff] [review].
 
> 2. I suggest that we take the opportunity to remove
> the code that handles Cygwin prior to the Beta 20
> release.  One reason is that your code that defines
> OS_RELEASE won't work if $OS_ARCH is "CYGWIN32_NT".

Sounds fine to me. Actually, my patch would leave OS_RELEASE empty for
CYGWIN32_NT, which is the same behavior as the original code. The original code
didn't set OS_RELEASE for CYGWIN32_NT.
 
> 3. I don't want the WINCE code in mozilla/nsprpub/configure.in.
> OS_ARCH has a slightly different meaning in NSPR, so
> we may not be able to copy that code from mozilla/configure.in
> without changes.  I will add the necessary WINCE code to
> NSPR when I check in the WINCE porting patch.

OK. Right, I assumed since the general structure was the same here I could just
copy the new stuff in completely, but I guess that was a wrong assumption.
 
> 4. In mozilla/security/coreconf/arch.mk, I'd be interested
> in finding out what Cygwin's and MSYS's uname -m return on
> your machine.  (Pentium Pro is too old...)

Both return i686 on my machine, which is a Pentium M.

So should I create a new patch to address #2 and #3?

Comment 58

12 years ago
Comment on attachment 183791 [details] [diff] [review]
More path fixes

-BUILD_TOOLS	= $(topsrcdir)/build/unix
+BUILD_TOOLS	= $(WIN_TOP_DRV)$(topsrcdir)/build/unix

is causing the script
(http://lxr.mozilla.org/seamonkey/source/config/rules.mk#68) that is used when
silent builds (mk_add_options MOZ_MAKE_FLAGS=-s) fail to have an invalid path:
c:/c/Mozilla/mozilla/build/unix/print-failed-commands.sh:
c:/c/Mozilla/mozilla/build/unix/print-failed-commands.sh: No such file or
directory
(Assignee)

Updated

12 years ago
Attachment #183614 - Flags: approval1.8b2?
(Assignee)

Updated

12 years ago
Attachment #183644 - Flags: approval1.8b2?
(Assignee)

Updated

12 years ago
Attachment #183699 - Flags: approval1.8b2?
Attachment #183855 - Flags: review+
Comment on attachment 183792 [details]
MSYS build notes, more updates

Please add this document to http://developer-test.mozilla.org/ somewhere (this
will become developer.mozilla.org whenever the new machines are configured): I
am planning on moving the build docs from http://www.mozilla.org/build/ to
developer.mozilla.org very shortly.
Attachment #183792 - Flags: review?(benjamin) → review+
(Assignee)

Comment 60

12 years ago
(In reply to comment #58)
> (From update of attachment 183791 [details] [diff] [review] [edit])
> -BUILD_TOOLS	= $(topsrcdir)/build/unix
> +BUILD_TOOLS	= $(WIN_TOP_DRV)$(topsrcdir)/build/unix
> 
> is causing the script
> (http://lxr.mozilla.org/seamonkey/source/config/rules.mk#68) that is used when
> silent builds (mk_add_options MOZ_MAKE_FLAGS=-s) fail to have an invalid path:
> c:/c/Mozilla/mozilla/build/unix/print-failed-commands.sh:
> c:/c/Mozilla/mozilla/build/unix/print-failed-commands.sh: No such file or
> directory

That seems odd; if you created a mount point for /Mozilla this shouldn't happen.
Can you tell me the steps to repreoduce this problem?

Comment 61

12 years ago
Ok, not making a mountpoint was my fault.  Without a mountpoint, I ran into
problems building NSS:

quickder.c
c:\Mozilla\mozilla\security\nss\lib\util\quickder.c : fatal error C1083: Cannot
open compiler generated file: '/c/Mozilla/obj-firefox/nss/secutil/quic
kder.obj': No such file or directory
make[5]: *** [/c/Mozilla/obj-firefox/nss/secutil/quickder.obj] Error 1

With a mountpoint, NSS builds and the make -s failure script works.
(Assignee)

Updated

12 years ago
Attachment #183855 - Flags: approval1.8b2?

Comment 62

12 years ago
I checked in the NSPR configure.in changes that Howard
proposed in comment 35 on the NSPR trunk for NSPR 4.6.
These changes will eventually appear in Mozilla client's
NSPR branch.
(Assignee)

Comment 63

12 years ago
Comment on attachment 183791 [details] [diff] [review]
More path fixes

Will re-upload, omitting the nsprpub/configure.in patch that has already been
committed.
Attachment #183791 - Flags: review?(cls)
(Assignee)

Comment 64

12 years ago
Created attachment 183959 [details] [diff] [review]
More path fixes, excluding NSPR
Attachment #183791 - Attachment is obsolete: true
(Assignee)

Updated

12 years ago
Attachment #183959 - Flags: review?(cls)

Updated

12 years ago
Attachment #183788 - Flags: review?(cls) → review+

Comment 65

12 years ago
Comment on attachment 183959 [details] [diff] [review]
More path fixes, excluding NSPR

Can we use something a bit more generic than DEFDIR?  Like dos_srcdir?

Also, DEFDIR is emtpy for non-win32 builds which gives an incomplete path for
DEFFILE.  This shouldn't break anything but for completeness, it should use a
full path.

The extra mount requirement still doesn't sit well with me.  I guess I haven't
completely shaken the "it works out of the box" mentality.  If we're going to
have to go through this hassle to support builds across drives, then adding
support for builds w/o mount points shouldn't be much harder.  I'm testing a
patch now.
Attachment #183959 - Flags: review?(cls) → review-

Comment 66

12 years ago
Created attachment 184273 [details] [diff] [review]
remove mount requirement

With this patch, I can do a msys/gcc build without creating a special mount
point for my build tree.

One minor problem is that this patch checks for CYGDRIVE_MOUNT during the NSS
build.	CYGDRIVE_MOUNT is not normally set during the standalone NSS build. 
The last cygpath patch also has this problem.  We might need a better way to
distinguish between a cygwin shell & msys shell build.

Updated

12 years ago
Attachment #183788 - Flags: superreview?(wtchang)
(Assignee)

Comment 67

12 years ago
(In reply to comment #65)
> (From update of attachment 183959 [details] [diff] [review] [edit])
> Can we use something a bit more generic than DEFDIR?  Like dos_srcdir?

I guess dos_srcdir is a good choice, especially since there's already some
places using that.
 
> Also, DEFDIR is emtpy for non-win32 builds which gives an incomplete path for
> DEFFILE.  This shouldn't break anything but for completeness, it should use a
> full path.

I don't understand why this is an issue, since only Win32 builds use DEFFILE.

> The extra mount requirement still doesn't sit well with me.  I guess I haven't
> completely shaken the "it works out of the box" mentality.  If we're going to
> have to go through this hassle to support builds across drives, then adding
> support for builds w/o mount points shouldn't be much harder.  I'm testing a
> patch now.

My only concern regarding your patch is that adding more $(shell) invocations to
the .mk files will still be slow (though not *as* slow as cygwin-wrapper, of
course). That's why in my patch I tried to do all the decision-making in the
configure script so that only a simple variable reference is needed in the
Makefiles.

Comment 68

12 years ago
(In reply to comment #67)
>
> I don't understand why this is an issue, since only Win32 builds use DEFFILE.

I thought that DEFFILE was set unconditionally in some makefiles that were
cross-platform but I could be mistaken.

> My only concern regarding your patch is that adding more $(shell) invocations to
> the .mk files will still be slow (though not *as* slow as cygwin-wrapper, of
> course). That's why in my patch I tried to do all the decision-making in the
> configure script so that only a simple variable reference is needed in the
> Makefiles.

I thought about the extra shell penalty as well.  Unfortunately, $(dos_srcdir)
cannot be set once at configure time unless we add it to each of the generated
Makefiles.  I don't think you're going to see much of a slowdown because that
shell command was always being invoked anyway if you built with auto-generating
non-compiler created dependencies, which is the default for win32.  People who
disable dependency generation so that they can do a fast clobber build from
scratch might notice a difference.

With the nomount patch, my build was approximately the same as without (within a
minute).  My box may not be a good test subject though because the difference
between a full suite cygwin & msys build is only 3 minutes (54m vs 51m).
(Assignee)

Comment 69

12 years ago
(In reply to comment #68)
> I thought about the extra shell penalty as well.  Unfortunately, $(dos_srcdir)
> cannot be set once at configure time unless we add it to each of the generated
> Makefiles.

We could break $(srcdir) into two parts - prefix and path. Then it's only a
matter of switching the prefix to go from srcdir to dos_srcdir.

  I don't think you're going to see much of a slowdown because that
> shell command was always being invoked anyway if you built with auto-generating
> non-compiler created dependencies, which is the default for win32.  People who
> disable dependency generation so that they can do a fast clobber build from
> scratch might notice a difference.

OK. I'm pretty sure I'm using the default setup; I'm not going to worry about
this if you're not.

> With the nomount patch, my build was approximately the same as without (within a
> minute).  My box may not be a good test subject though because the difference
> between a full suite cygwin & msys build is only 3 minutes (54m vs 51m).

Completely irrelevant but I do wonder why our systems respond so differently.
I'll note that Windows' filesystem cache performance is abysmal, and reclaims
recently freed cache pages much too aggressively. So a slow disk on Windows hits
you a lot harder than it does on say, Linux. I've been thinking about writing a
"cache heater" that tracks what files are most frequently used on a system, and
keeps handles open on all of them to prevent Windows from purging them from the
cache. (The point being, Windows releases cache pages as soon as a file has zero
references. It does keep them around, in case the file is used again "soon" but
it will also reuse those pages to satisfy some other file request. It appears
that a compile cycle is pathologically bad here. E.g., the compiler cl.exe is
used over and over, but it's always getting paged in fresh from disk on each
invocation because its previous cache pages were already used for something
else. The cache heater's job would be to prevent the reference count from
reaching zero on frequently used files, forcing Windows to allocate new cache
pages instead of reusing existing ones.) And sorry for digressing so far off
topic...
(Assignee)

Comment 70

12 years ago
(In reply to comment #69)
> (In reply to comment #68)
> > I thought about the extra shell penalty as well.  Unfortunately, $(dos_srcdir)
> > cannot be set once at configure time unless we add it to each of the generated
> > Makefiles.
> 
> We could break $(srcdir) into two parts - prefix and path. Then it's only a
> matter of switching the prefix to go from srcdir to dos_srcdir.

Something like this - configure sets the prefix WIN_TOP_PREFIX, similar to my
previous patch.

If the top_srcdir is /cygdrive/d/foo (i.e., Cygwin) then we set
  WIN_TOP_DRV=d
  WIN_TOP_PREFIX=D:/

If the top_srcdir is /d/foo (i.e., MSYS) then we set
  WIN_TOP_DRV=d
  WIN_TOP_PREFIX=D:/

Then in the config.mk we just do

ifeq (,$(CROSS_COMPILE)$(filter-out WINCE WINNT,$(OS_ARCH)))
ifdef CYGDRIVE_MOUNT
WIN32_PATH = $(subst $(CYGDRIVE_MOUNT)/$(WIN_TOP_DRV)/,$(WIN_TOP_PREFIX),$(1))
else
WIN32_PATH = $(subst /$(WIN_TOP_DRV)/,$(WIN_TOP_PREFIX),$(1))
endif
else
WIN32_PATH = $(1)
endif

dos_srcdir := $(call WIN32_PATH, $(srcdir))

Except this then breaks if they *are* using a mountpoint.

Comment 71

12 years ago
Comment on attachment 183614 [details] [diff] [review]
Option switch cleanup

Not required for for 1.8b2. Please try again in b3. Thanks.
Attachment #183614 - Flags: approval1.8b2? → approval1.8b2-

Comment 72

12 years ago
Comment on attachment 183644 [details] [diff] [review]
MSYS perl dependencies, again

Not required for for 1.8b2. Please try again in b3. Thanks.
Attachment #183644 - Flags: approval1.8b2? → approval1.8b2-

Comment 73

12 years ago
Comment on attachment 183699 [details] [diff] [review]
MSYS uname, with CYGWIN_ME correction

Not required for for 1.8b2. Please try again in b3. Thanks.
Attachment #183699 - Flags: approval1.8b2? → approval1.8b2-

Comment 74

12 years ago
Comment on attachment 183855 [details] [diff] [review]
make-jar patch to exclude attrs

Not required for for 1.8b2. Please try again in b3. Thanks.
Attachment #183855 - Flags: approval1.8b2? → approval1.8b2-
(Assignee)

Comment 75

12 years ago
(In reply to comment #70)
> (In reply to comment #69)
> > (In reply to comment #68)
> > > I thought about the extra shell penalty as well.  Unfortunately, $(dos_srcdir)
> > > cannot be set once at configure time unless we add it to each of the generated
> > > Makefiles.
> > 
> > We could break $(srcdir) into two parts - prefix and path. Then it's only a
> > matter of switching the prefix to go from srcdir to dos_srcdir.
> 
It's even easier than that. configure can set WIN_TOP_SRC in autoconf.mk, so we
have these definitions, for example:

top_srcdir=/cygdrive/d/home/hyc/mozilla
srcdir=/cygdrive/d/home/hyc/mozilla/mailnews
WIN_TOP_SRC=d:/home/hyc/mozilla

Then config.mk just needs to do this

WIN32_PATH = $(subst $(top_srcdir),$(WIN_TOP_SRC),$(1))
dos_srcdir = $(call WIN32_PATH,$(srcdir))

It's trivial for the configure script to set WIN_TOP_SRC correctly, and no
conditional or $(shell) is needed in the Makefiles at all.
(Assignee)

Comment 76

12 years ago
Created attachment 184388 [details] [diff] [review]
Better path/mount fixes

This patch works for me on MSYS with both MSVC and gcc, and doesn't require
mounts or anything else. It of course works if you use mounts too.
(Assignee)

Updated

12 years ago
Attachment #183959 - Attachment is obsolete: true
(Assignee)

Comment 77

12 years ago
Created attachment 184389 [details]
Build notes, revised again

Updated to remove the notes about special mount points etc. as they are no
longer needed.
Attachment #183792 - Attachment is obsolete: true
(Assignee)

Updated

12 years ago
Attachment #184389 - Attachment description: No more path dependencies. → Build notes, revised again
(Assignee)

Comment 78

12 years ago
Comment on attachment 184388 [details] [diff] [review]
Better path/mount fixes

There's an extraneous diff to directory/c-sdk/configure in here, but that
should be harmless.
Attachment #184388 - Flags: review?(cls)
Not extraneous, actually, because like in the main tree, we check in the
configure script to the directory/c-sdk, but unlike in the main tree, it's not
done automatically.  Note that any changes to directory/c-sdk need to land both
on the client branch and the trunk, so it might make sense to split them out
into a separate patch.

Comment 80

12 years ago
Comment on attachment 184388 [details] [diff] [review]
Better path/mount fixes

Any chance you can use cvs.mozilla.org revision numbers in these patches, so we
know what code you're patching against?

Comment 81

12 years ago
Oh, I think I get it now - I have to apply attachment 183614 [details] [diff] [review], attachment 183644 [details] [diff] [review]
and attachment 183788 [details] [diff] [review] before I can apply attachment 184388 [details] [diff] [review].
(Assignee)

Comment 82

12 years ago
(In reply to comment #80)
> (From update of attachment 184388 [details] [diff] [review] [edit])
> Any chance you can use cvs.mozilla.org revision numbers in these patches, so 
we
> know what code you're patching against?

Sorry about that. I've got a bunch of different bits I'm working on, so I 
imported it into my own CVS tree so I could track my own revisions a little 
easier. For this last patch, I imported the trunk on May 23 and then applied 
the previous 5 patches, then generated this last diff.

Mid-air collision. Yeah, you got it.
(Assignee)

Comment 83

12 years ago
(In reply to comment #79)
> Not extraneous, actually, because like in the main tree, we check in the
> configure script to the directory/c-sdk, but unlike in the main tree, it's 
not
> done automatically.

Well, all of the patches are cumulative, but none of the previous ones 
included the diff to configure (per Chris's request). So the configure diff in 
here can't apply cleanly anyway, you'll just have to regenerate it with 
autoconf.

> Note that any changes to directory/c-sdk need to land both
> on the client branch and the trunk, so it might make sense to split them out
> into a separate patch.

Oh. Since Chris approved the previous patches mixed as they are, I hesitate to 
juggle this any more right now. It was a pain to get the incremental diffs 
right before I set up my own CVS tree. But I guess it should be easy enough to 
cvs diff the directory/c-sdk subtree against the initial revision and attach 
that here, if needed.

Comment 84

12 years ago
Comment on attachment 183615 [details] [diff] [review]
MSYS uname support

> # If uname -s returns "CYGWIN_NT-4.0", we assume that we are using

>+# If uname -s returns MINGW32_NT-5.1, we assume that we are using
Shouldn't this read "MINGW32_NT-4.0" for consistency?

Also, clearing the obsolete flag, as this patch appears to be one of the five
necessary patches mentioned.
Attachment #183615 - Attachment is obsolete: false
(Assignee)

Comment 85

12 years ago
Comment on attachment 183615 [details] [diff] [review]
MSYS uname support

(In reply to comment #84)
> (From update of attachment 183615 [details] [diff] [review] [edit])
> > # If uname -s returns "CYGWIN_NT-4.0", we assume that we are using
> 
> >+# If uname -s returns MINGW32_NT-5.1, we assume that we are using
> Shouldn't this read "MINGW32_NT-4.0" for consistency?

On my machine it returns -5.1. Frankly I don't see that the specifics matter in
the comment, since the code does the right thing, so you're welcome to change
it as you see fit.

> Also, clearing the obsolete flag, as this patch appears to be one of the five
> necessary patches mentioned.

No. Attachment 183615 [details] [diff] was obsoleted by attachment 183699 [details] [diff] [review]. I am re-applying the
obsolete flag on 183615.

The valid, approved patches are attachment 183614 [details] [diff] [review], attachment 183644 [details] [diff] [review],
attachment 183699 [details] [diff] [review], attachment 183788 [details] [diff] [review], and attachment 183855 [details] [diff] [review]. I generated
attachment 184388 [details] [diff] [review] after applying the above 5 patches.
Attachment #183615 - Attachment is obsolete: true
(Assignee)

Comment 86

12 years ago
Comment on attachment 183615 [details] [diff] [review]
MSYS uname support

cls review transferred to attachment 183699 [details] [diff] [review].
Attachment #183615 - Flags: review+

Comment 87

12 years ago
Comment on attachment 184388 [details] [diff] [review]
Better path/mount fixes

Nice work getting rid of that per-make shell expression.  However, in the
cygwin build, WIN_TOP_SRC isn't being set correctly.  I had to add extra \\s to
the sed expressions to get them to work.
Attachment #184388 - Flags: review?(cls) → review-
(Assignee)

Comment 88

12 years ago
Created attachment 184809 [details] [diff] [review]
Path/mount fixes again (checked in)

OK, fixed the backslash issues, tested Cygwin/MSVC and MSYS/MSVC.
Attachment #184388 - Attachment is obsolete: true
(Assignee)

Updated

12 years ago
Attachment #184809 - Flags: review?(cls)

Comment 89

12 years ago
OK, so I can't make a direct comparison because my 666MHz Cygwin/MinGW "depend"
build was interrupted by a shlibsign issue but I guess it takes over 10 hours.
Meanwhile a new 866MHz MSYS/MSVC build took a little over three hours. Neat :-)

Updated

12 years ago
Attachment #184809 - Flags: review?(cls) → review+
(Assignee)

Updated

12 years ago
Attachment #183614 - Flags: approval1.8b3?
(Assignee)

Updated

12 years ago
Attachment #183644 - Flags: approval1.8b3?
(Assignee)

Updated

12 years ago
Attachment #183699 - Flags: approval1.8b3?
(Assignee)

Updated

12 years ago
Attachment #183855 - Flags: approval1.8b3?
(Assignee)

Updated

12 years ago
Attachment #183788 - Flags: approval1.8b3?
(Assignee)

Updated

12 years ago
Attachment #184809 - Flags: approval1.8b3?
(Assignee)

Comment 90

12 years ago
Comment on attachment 184273 [details] [diff] [review]
remove mount requirement

I guess this is now obsoleted by attachment 184809 [details] [diff] [review]
Attachment #184273 - Attachment is obsolete: true
Comment on attachment 183614 [details] [diff] [review]
Option switch cleanup

a=shaver
Attachment #183614 - Flags: approval1.8b3? → approval1.8b3+
Comment on attachment 183644 [details] [diff] [review]
MSYS perl dependencies, again

a=shaver
Attachment #183644 - Flags: approval1.8b3? → approval1.8b3+
Comment on attachment 183699 [details] [diff] [review]
MSYS uname, with CYGWIN_ME correction

a=shaver
Attachment #183699 - Flags: approval1.8b3? → approval1.8b3+
Comment on attachment 183855 [details] [diff] [review]
make-jar patch to exclude attrs

a=shaver
Attachment #183855 - Flags: approval1.8b3? → approval1.8b3+
Comment on attachment 183788 [details] [diff] [review]
Cygpath fixes, patch fixed (checked in, except NSS changes)

Please re-request approval when sr has been granted.
Attachment #183788 - Flags: approval1.8b3?
Comment on attachment 184809 [details] [diff] [review]
Path/mount fixes again (checked in)

a=shaver
Attachment #184809 - Flags: approval1.8b3? → approval1.8b3+
(Assignee)

Comment 97

12 years ago
Comment on attachment 184809 [details] [diff] [review]
Path/mount fixes again (checked in)

This patch depends on all the previous, so it cannot be committed until
attachment 183788 [details] [diff] [review] is approved.

Comment 98

12 years ago
Created attachment 185022 [details] [diff] [review]
NSS changes

I'm moving the NSS changes into a separate patch since I can't check them in
and wtc's on vacation for another 2 weeks.

Comment 99

12 years ago
Comment on attachment 183788 [details] [diff] [review]
Cygpath fixes, patch fixed (checked in, except NSS changes)

Requesting approval for the non-NSS portions of the patch.
Attachment #183788 - Flags: superreview?(wtchang) → approval1.8b3?
Comment on attachment 183788 [details] [diff] [review]
Cygpath fixes, patch fixed (checked in, except NSS changes)

a=shaver.
Attachment #183788 - Flags: approval1.8b3? → approval1.8b3+

Comment 101

12 years ago
Comment on attachment 183614 [details] [diff] [review]
Option switch cleanup

2005-06-01 07:28
mozilla/nsprpub/pr/tests/dll/Makefile.in	1.16
mozilla/nsprpub/pr/src/Makefile.in	1.40
mozilla/nsprpub/lib/prstreams/Makefile.in	1.21
mozilla/nsprpub/lib/msgc/src/Makefile.in	1.13
mozilla/nsprpub/lib/libc/src/Makefile.in	1.31
mozilla/nsprpub/lib/ds/Makefile.in	1.35
mozilla/nsprpub/config/Makefile.in	1.20
mozilla/directory/c-sdk/config/Makefile.in	5.7
mozilla/directory/c-sdk/build.mk	5.19
mozilla/js/src/Makefile.in	3.95
mozilla/embedding/tests/mfcembed/components/Makefile.in 	1.14
mozilla/embedding/tests/mfcembed/Makefile.in	1.36
mozilla/embedding/browser/activex/src/control_kicker/Makefile.in	1.7
mozilla/config/rules.mk 	3.473
mozilla/config/config.mk	3.316
mozilla/embedding/browser/activex/src/control/Makefile.in	1.31
mozilla/accessible/public/msaa/Makefile.in	1.11
mozilla/configure.in	1.1454
mozilla/configure	1.1439
Attachment #183614 - Attachment is obsolete: true

Comment 102

12 years ago
Comment on attachment 183644 [details] [diff] [review]
MSYS perl dependencies, again

2005-06-01 07:20
mozilla/toolkit/mozapps/installer/makecfgini.pl 	1.6
mozilla/config/preprocessor.pl	3.30
mozilla/config/make-jars.pl	3.80
mozilla/config/make-chromelist.pl	3.9
mozilla/config/add-chrome.pl	1.12
Attachment #183644 - Attachment is obsolete: true

Comment 103

12 years ago
Comment on attachment 183699 [details] [diff] [review]
MSYS uname, with CYGWIN_ME correction

2005-06-01 08:30
mozilla/nsprpub/configure.in	1.188
mozilla/nsprpub/configure	1.186
mozilla/directory/c-sdk/configure.in	5.29
mozilla/directory/c-sdk/configure	5.28
Attachment #183699 - Attachment is obsolete: true

Comment 104

12 years ago
Comment on attachment 183855 [details] [diff] [review]
make-jar patch to exclude attrs

2005-06-01 08:34
mozilla/config/make-jars.pl	3.81
Attachment #183855 - Attachment is obsolete: true

Comment 105

12 years ago
I can help check in the NSPR changes if someone will post a NSPR-only patch.

Updated

12 years ago
Attachment #183788 - Attachment description: Cygpath fixes, patch fixed → Cygpath fixes, patch fixed (checked in, except NSS changes)

Updated

12 years ago
Attachment #184809 - Attachment description: Path/mount fixes again → Path/mount fixes again (checked in)
(Assignee)

Comment 106

12 years ago
(In reply to comment #66)
> One minor problem is that this patch checks for CYGDRIVE_MOUNT during the NSS
> build. CYGDRIVE_MOUNT is not normally set during the standalone NSS build. 
> The last cygpath patch also has this problem.  We might need a better way to
> distinguish between a cygwin shell & msys shell build.

I see what you mean. Since we explicitly detect Cygwin vs MSYS in arch.mk, we
could set a new flag there for the purpose. So far the patch hasn't caused any
trouble for my Cygwin builds, but that must be because I'm not building NSS by
itself. I guess when WTC gets back we can hash it out then.
Created attachment 185665 [details] [diff] [review]
cygpath tweak

This fixes my cygwin build that uses a relative --enable-srcdir
Attachment #185665 - Flags: review?(cls)

Comment 108

12 years ago
Comment on attachment 185665 [details] [diff] [review]
cygpath tweak

What's broken that this patch fixes?  I'm fairly certain that my relative path
build worked fine.

Any idea when the -a option was added to cygpath?  I'd like to not break
regular builds without a configure check if we can help it.
My build was failing to run $(PERL) $(BUILD_TOOLS)/mddepend $@ $(MDDEPEND_FILES)
to build .deps/.all.dp in mozilla/db/src because $(BUILD_TOOLS) was equal to my
$srcdir and thus does not work from inside a subdirectory. Using cygpath v1.29
(Assignee)

Comment 110

12 years ago
(In reply to comment #108)
> (From update of attachment 185665 [details] [diff] [review] [edit])
> What's broken that this patch fixes?  I'm fairly certain that my relative path
> build worked fine.
> 
> Any idea when the -a option was added to cygpath?  I'd like to not break
> regular builds without a configure check if we can help it.
> 
http://cygwin.com/cgi-bin/cvsweb.cgi/src/winsup/utils/cygpath.cc?cvsroot=src

The -a option was added in rev 1.3 of cygpath, so that's safe.

But on my build (cygpath v1.37) "cygpath -w" always returns an absolute path, so
I'm surprised that it's even necessary. And note that the original configure
scripts only used "cygpath -w" in a variety of places so if this is a problem,
it's been a problem for a long time - why is it only showing up now?
(In reply to comment #110)
>But on my build (cygpath v1.37) "cygpath -w" always returns an absolute path,
>so I'm surprised that it's even necessary. And note that the original configure
>scripts only used "cygpath -w" in a variety of places so if this is a problem,
>it's been a problem for a long time - why is it only showing up now?
Previous versions of configure (well, at least the version that I looked at)
only passed the result of `pwd` which of course is always absolute, thus when
passed to cygpath -w will always return an absolute path. However I was building
with --enable-srcdir and my issue is that the new code passes the srcdir to
cygpath -w while expecting an absolute result.

Updated

12 years ago
Attachment #185665 - Flags: review?(cls) → review+

Updated

12 years ago
Attachment #185665 - Flags: approval1.8b3?

Updated

12 years ago
Attachment #185665 - Flags: approval1.8b3? → approval1.8b3+

Comment 112

12 years ago
FYI, bug 292021 just added a build requirement upon iconv.  This doesn't come
with the standard msysDTK.  You will need to download the prebuilt binary from
mingw.org or compile libiconv for mingw yourself (which needs a small patch, see
http://sourceforge.net/mailarchive/forum.php?forum=mingw-users ).
(In reply to comment #112)
>FYI, bug 292021 just added a build requirement upon iconv.
But, fortunately for me, not for suite :-)

Comment 114

12 years ago
Hi I'm trying to build mingw32 SeaMonkey builds on Linux (Debian unstable, daily
updated) and there seems a problem with following checkin:

>case "$host_os" in
>+mingw*)
>+    WIN_TOP_SRC=`cd $srcdir; pwd -W`
>+    ;;

pwd -W
bash: pwd: -W: invalid option
pwd: usage: pwd [-PL]

So my WIN_TOP_SRC is empty and so building fails :-(
(Assignee)

Comment 115

12 years ago
(In reply to comment #114)
> Hi I'm trying to build mingw32 SeaMonkey builds on Linux (Debian unstable, daily
> updated) and there seems a problem with following checkin:
> 
> >case "$host_os" in
> >+mingw*)
> >+    WIN_TOP_SRC=`cd $srcdir; pwd -W`
> >+    ;;
> 
> pwd -W
> bash: pwd: -W: invalid option
> pwd: usage: pwd [-PL]
> 
> So my WIN_TOP_SRC is empty and so building fails :-(

Yuck. Looks like we didn't take cross-compiling into account. It appears that
many of these checks should use $build_os instead of $host_os.

Of course now I'm confused - what is the difference between host_os and target_os ?
(Assignee)

Comment 116

12 years ago
Comment on attachment 185022 [details] [diff] [review]
NSS changes

Also see comment #66 and comment #106
Attachment #185022 - Flags: review?(wtchang)
afaik host and target are identical when not building a compiler

Comment 118

12 years ago
That particular snippet should not affect cross-compiled builds since $host_os
!= $target_os when cross compiling Mozilla.  See bug 299557 for recent mingw
cross-compile fixes.

Comment 119

12 years ago
Comment on attachment 185022 [details] [diff] [review]
NSS changes

Howard: the security/coreconf/arch.mk change is fine.
In security/coreconf/rules.mk, NSS doesn't define the
CYGDRIVE_MOUNT variable.  I suggest that you replace
CYGDRIVE_MOUNT by something based on "uname -s".  For
example, we can set USE_CYGWIN to 1 in arch.mk if
the original value of $(OS_ARCH) contains CYGWIN.
Attachment #185022 - Flags: review?(wtchang) → review-
(Assignee)

Comment 120

12 years ago
Created attachment 188382 [details] [diff] [review]
NSS changes, fixed

OK, this patch defines USE_CYGWIN in arch.mk and references it in rules.mk.
(Assignee)

Updated

12 years ago
Attachment #185022 - Attachment is obsolete: true
(Assignee)

Updated

12 years ago
Attachment #188382 - Flags: review?(wtchang)
(Assignee)

Comment 121

12 years ago
(In reply to comment #59)
> (From update of attachment 183792 [details] [edit])
> Please add this document to http://developer-test.mozilla.org/ somewhere (this
> will become developer.mozilla.org whenever the new machines are configured): I
> am planning on moving the build docs from http://www.mozilla.org/build/ to
> developer.mozilla.org very shortly.

OK, I've created a stub for the build docs and put a copy of the MSYS notes
there as well. From the main page, Mozilla: Developing Mozilla -> References &
Guides: Building Mozilla.

Comment 122

12 years ago
Created attachment 188475 [details] [diff] [review]
NSS changes

This is Howard's patch except that I use USE_MSYS instead
of USE_CYGWIN.	The reason is not obvious, so let me explain.

The Mozilla build instructions for Win32 require the so-called
Netscape wintools, which need to be installed in the %MOZ_TOOLS%
directory.  The instructions tell us to add
%MOZ_TOOLS%\bin;c:\cygwin\bin to our PATH variable.  Note the
ordering of %MOZ_TOOLS%\bin and c:\cygwin\bin.

Netscape wintools contain some tools that were no longer needed
today.	One of them is a uname.exe program, which returns WINNT
on my system.  (In contrast, Cygwin's uname.exe returns
CYGWIN_NT-5.1.)

So some Mozilla developers may have the Netscape wintools uname.exe
before Cygwin's uname.exe on their PATH.  Therefore it is not
reliable to use the output of uname to determine if Cygwin is used.

Since MSYS is a new toolkit, we can fix this problem when we update
the build instructions to mention MSYS.  Therefore it is reliable
to use uname to determine if MSYS is used.
Attachment #188382 - Attachment is obsolete: true
Attachment #188475 - Flags: superreview?(cls)
Attachment #188475 - Flags: review?(hyc)

Updated

12 years ago
Attachment #188382 - Flags: review?(wtchang)

Comment 123

12 years ago
Comment on attachment 188475 [details] [diff] [review]
NSS changes

Just to elaborate: this is also why I had to use the
ifeq (,$(findstring ;,$(PATH))) test to determine
if Cygwin is used (because MKS uses ; as the
separator in PATH).  So the new code really works
by eliminating other possibilities:

 ifeq (,$(findstring ;,$(PATH)))  # if not MKS
 ifndef USE_MSYS		  # if not MSYS
 PWD := $(subst \,/,$(shell cygpath -w $(PWD))) # must be Cygwin
 endif
 endif

Updated

12 years ago
Attachment #188475 - Flags: superreview?(cls) → superreview+
(In reply to comment #123)
>  ifeq (,$(findstring ;,$(PATH)))  # if not MKS
>  ifndef USE_MSYS		  # if not MSYS
>  PWD := $(subst \,/,$(shell cygpath -w $(PWD))) # must be Cygwin
>  endif
>  endif
Strange that you should mention this. I managed to get a default build of
seamonkey to compile using SFU and MSVC, but my biggest issue was the mixing of
windows and native paths. For instance this PWD seems to be always passed to
native apps via cygwin-wrapper which will do the windows path conversion anyway.
(Assignee)

Comment 125

12 years ago
Comment on attachment 188475 [details] [diff] [review]
NSS changes

(just for completeness' sake) That makes sense.

I forgot about the uname issue, but I know that got me confused before; I have
moztools/bin last on my path now.
Attachment #188475 - Flags: review?(hyc) → review+

Comment 126

12 years ago
Comment on attachment 188475 [details] [diff] [review]
NSS changes

Julien, please see my comment 122 and comment 123 for
additional info on this patch.	This patch may need to
be applied to NSS_3_10_BRANCH for timely inclusion in
NSS_CLIENT_TAG.
Attachment #188475 - Flags: review?(julien.pierre.bugs)

Comment 127

12 years ago
I'm reasonably sure that my Win32/MinGW/cygwin build problem was caused by the
last checkin from this bug. The error I get is :-

/cygdrive/e/mozilla/source/mozilla/configure: line 6026: cd: e:\moztools: No suc
h file or directory
configure: error: cd $MOZ_TOOLS failed. MOZ_TOOLS ==? e:\moztools
*** Fix above errors and then restart with "make -f client.mk build"
make: *** [/cygdrive/e/mozilla/source/mozilla/obj/mozilla/Makefile] Error 1

Comment 128

12 years ago
Comment on attachment 188475 [details] [diff] [review]
NSS changes

The changes are fine.

Re: comment #122, wintools and cygwin, the wintools also include a non-cygwin
gmake.exe . A lot of people are using that gmake.exe accidentally.

The approach currently used in coreconf doesn't work with this gmake.exe -
coreconf thinks it is MKS, and uses semicolons, which breaks. I'm not sure if
there is an efficient way to better detect cygwin. If it can't be done, then
perhaps gmake.exe should be removed from wintools, and packaged separately, for
those who don't use cygwin ? Or maybe it could be stored in a "notcygwin"
subdirectory in the wintools zipfile, which should only be added to the PATH if
you aren't using cygwin.

Anyway, this is a pre-existing problem and in no way caused by the patch, but
it would be nice to solve it while this part of the build system is being
worked on.

Note that NSPR builds fine with either the wintools gmake or the cygwin make,
but that might be because autoconf detects cygwin by a difference means.
Attachment #188475 - Flags: review?(julien.pierre.bugs) → review+
(Assignee)

Comment 129

12 years ago
(In reply to comment #127)
> I'm reasonably sure that my Win32/MinGW/cygwin build problem was caused by the
> last checkin from this bug. The error I get is :-
> 
> /cygdrive/e/mozilla/source/mozilla/configure: line 6026: cd: e:\moztools: No suc
> h file or directory
> configure: error: cd $MOZ_TOOLS failed. MOZ_TOOLS ==? e:\moztools
> *** Fix above errors and then restart with "make -f client.mk build"
> make: *** [/cygdrive/e/mozilla/source/mozilla/obj/mozilla/Makefile] Error 1

Which checkin, exactly?

I don't get this error, using either MSYS or Cygwin.

You could probably workaround it by using a Cygwin-style path for MOZ_TOOLS, or
using a forward slash instead of a backslash.

Comment 130

12 years ago
Sorry, my mistake, sort of.
I knew what caused it, but it slipped my mind. A very recent cygwin upgrade
chnaged the default shell from ash to bash, for which I had to do the advised
change to MOZ_TOOLS.

Thanks for the help, and my build is now chugging along.

Comment 131

12 years ago
the cygwin ash package 20040127-3 gives error 

/cygdrive/e/mozilla/source/mozilla/configure: line 6026: cd: C:\moztools: No suc
 h file or directory
 configure: error: cd $MOZ_TOOLS failed. MOZ_TOOLS ==? C:\moztools
 *** Fix above errors and then restart with "make -f client.mk build"
 make: *** [/cygdrive/C/mozilla/source/mozilla/obj/mozilla/Makefile] Error 1
----------------
ash 20040127-1 works fine

Updated

12 years ago
Blocks: 302993

Updated

12 years ago
Depends on: 302994

Updated

12 years ago
No longer blocks: 302993
Comment on attachment 188475 [details] [diff] [review]
NSS changes

So when can we expect this to land on the trunk and (hopefully) the 1.8 branch?
Created attachment 196240 [details] [diff] [review]
steps to making installers under msys (checked in ds32.exe changes only)

MSYS doesn't like /-style options; fortunately ds32.exe accepts - but for xcopy
I admitted defeat and removed the /F (full path display) entirely.

More issues as and when I resolve them.
Attachment #196240 - Flags: superreview?(dveditz)
Attachment #196240 - Flags: review?(dveditz)
Comment on attachment 196240 [details] [diff] [review]
steps to making installers under msys (checked in ds32.exe changes only)

r/sr=dveditz

I'm slightly concerned about zapping the xcopy /F option. Please ask chase if
that produces important logging we need, or if it's OK w/out. It's probably OK,
but if not you'll have to switch to cp -v -R, or put in conditional code here
that can tell when it's an MSYS build.
Attachment #196240 - Flags: superreview?(dveditz)
Attachment #196240 - Flags: superreview+
Attachment #196240 - Flags: review?(dveditz)
Attachment #196240 - Flags: review+

Updated

12 years ago
Attachment #196240 - Attachment description: steps to making installers under msys → steps to making installers under msys (checked in ds32.exe changes only)
Attachment #196240 - Attachment is obsolete: true

Comment 135

11 years ago
I've read http://developer.mozilla.org/en/docs/Building_on_Windows_with_MSYS and built Firefox with MSYS, but I can't build the installer:

cd /c/moz/mozilla/toolkit/mozapps/installer && \
  perl build_static.pl -config "/cygdrive/c/moz/ff-objdir/browser/installer/windows/instgen" -srcDir /cygdrive/c/moz/mozilla" \
  -objDir "/cygdrive/c/moz/ff-objdir/"
Warning: Remote XPI URL not set. Using ftp://not.supplied.invalid instead!

could not open /cygdrive/c/moz/ff-objdir/browser/installer/windows/instgen/installer.cfg: No such file or directory
make[1]: *** [installer] Error 2
make[1]: Leaving directory `/c/moz/ff-objdir/browser/installer/windows'
make: *** [installer] Error 2

I checked out arch.mk (1.19) and rules.mk (1.66). Cygwin is installed and at the end of %PATH%.

Comment 136

11 years ago
(In reply to comment #135)
I've forgotten to mention that I'm using the branch (MOZILLA_1_8_BRANCH).
That's usually an issue with the version of perl being used (some versions had issues building the installer, I can't find the bug atm)

Comment 138

11 years ago
(In reply to comment #137)
> That's usually an issue with the version of perl being used (some versions had
> issues building the installer, I can't find the bug atm)
> 
I've tried perl from MSYS as well as ActivePerl (5.6.1.638 and 5.8.7.815) without success. To not spam this bug I've opened a thread at MozillaZine (http://forums.mozillazine.org/viewtopic.php?t=360836).

Comment 139

11 years ago
http://forums.mozillazine.org/viewtopic.php?t=364654 has the answer. The path is the issue. cygwin/bin needs to be before %windir%\system32 

Updated

11 years ago
Depends on: 328499

Updated

11 years ago
No longer depends on: 328499
Depends on: 352206

Updated

11 years ago
Assignee: hyc → nobody
Status: ASSIGNED → NEW

Comment 140

11 years ago
argh, managed to unassign this while cc'ing myself.
Assignee: nobody → hyc

Updated

10 years ago
Depends on: 363195
No longer depends on: 363195
Building with MSYS clearly works at this point as the official reference setup (MozillaBuild) uses it. Is there anything else that needs to be done in this bug or can it be marked FIXED?
(Assignee)

Comment 142

9 years ago
Agreed, it was made the official setup back in December 2006

http://benjamin.smedbergs.us/blog/2006-12-07/unified-windows-build-prerequisites/

I'm closing it.
Status: NEW → RESOLVED
Last Resolved: 9 years ago
Resolution: --- → FIXED
V.Fixed (I'm using it)

If there are remaining issues, file them as separate(/dependent) bugs.
Status: RESOLVED → VERIFIED
You need to log in before you can comment on or make changes to this bug.