Closed Bug 294122 (msys) Opened 19 years ago Closed 16 years ago

Changes needed for building with MSYS

Categories

(Firefox Build System :: General, enhancement)

x86
Windows XP
enhancement
Not set
normal

Tracking

(Not tracked)

VERIFIED FIXED

People

(Reporter: hyc, Assigned: hyc)

References

Details

Attachments

(5 files, 28 obsolete files)

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
hyc
: review+
julien.pierre
: review+
Details | Diff | Splinter Review
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
Attached patch Fix inconsistent option switches (obsolete) — Splinter Review
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.
Status: UNCONFIRMED → NEW
Ever confirmed: true
Product: Mozilla Application Suite → Core
Version: unspecified → Trunk
Attached patch More comprehensive patch (obsolete) — Splinter Review
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.
Attachment #183563 - Attachment is obsolete: true
Attached patch Correct attachment (obsolete) — Splinter Review
Sorry, the previous attachment was the wrong file, had some typos in it. This
one is right.
Attachment #183578 - Attachment is obsolete: true
Attachment #183582 - Flags: review?(cls)
Alias: msys
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)
(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.
Attached patch Option switch cleanup (obsolete) — Splinter Review
OK, this patch only deals with the option switches, slash to dash.
Attachment #183582 - Attachment is obsolete: true
Attachment #183614 - Flags: review?(cls)
Attached patch MSYS uname support (obsolete) — Splinter Review
This is just the diffs to uname parsing, to handle MSYS.
Attachment #183615 - Flags: review?(cls)
Attached patch MSYS perl dependencies (obsolete) — Splinter Review
Changes for the perl scripts to recognize MSYS perl, and to remove cygpath
dependencies.
Attached patch Fix cygpath dependencies (obsolete) — Splinter Review
This patch eliminates usage of cygpath/cygwin_wrapper on MSYS and fixes use of
cygpath in other places.
Attachment #183616 - Flags: review?(cls)
Attachment #183617 - Flags: review?(cls)
Attached file Notes on building with MSYS (obsolete) —
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
(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.
Attachment #183616 - Flags: review?(cls)
(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 :->
(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?
Attached patch MSYS perl dependencies, again (obsolete) — Splinter Review
Same as before, but with comment #12 (if conditions reordered).
Attachment #183616 - Attachment is obsolete: true
Attachment #183644 - Flags: review?(cls)
Attached file MSYS build notes, again (obsolete) —
Since I could not reproduce the JAR problem I mentioned before, I've deleted
that bit from the build notes.
Attachment #183618 - Attachment is obsolete: true
Attachment #183614 - Flags: review?(cls) → review+
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-
Attachment #183644 - Flags: review?(cls) → review+
(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?
(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?
(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.
> 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.
(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.
Attached patch Cygpath fixes, again (obsolete) — Splinter Review
Leaves original "cygpath -w | sed" behavior intact.
Attachment #183617 - Attachment is obsolete: true
Attached patch Cygpath fixes... (obsolete) — Splinter Review
Still had cygpath -m in a couple places, oops.
Attachment #183682 - Attachment is obsolete: true
Attachment #183683 - Flags: review?(cls)
Comment on attachment 183683 [details] [diff] [review]
Cygpath fixes...

missing an endif in xpinstall/packager/windows/Makefile.in
Attachment #183683 - Flags: review?(cls) → review-
Comment on attachment 183683 [details] [diff] [review]
Cygpath fixes...

missing an endif in xpinstall/packager/windows/Makefile.in
Attachment #183683 - Flags: review-
Add missing endif
Attachment #183683 - Attachment is obsolete: true
Attachment #183685 - Flags: review?(cls)
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.
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.
(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.
Attachment #183615 - Flags: superreview?(wtchang)
Attachment #183615 - Flags: review?(cls)
Attachment #183615 - Flags: review+
Per comment #34, use CYGWIN_ME* in the case statements. Already approved by
cls.
Attachment #183615 - Attachment is obsolete: true
Attachment #183699 - Flags: superreview?(wtchang)
Attachment #183699 - Flags: review+
Attachment #183615 - Flags: superreview?(wtchang)
(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.
Comment on attachment 183685 [details] [diff] [review]
Cygpath fixes - once more, with feeling

Have one more slight improvement...
Attachment #183685 - Flags: review?(cls)
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
Attached patch Cygpath fixes, with gmake again (obsolete) — Splinter Review
Eliminate a couple calls to pwd
Attachment #183710 - Attachment is obsolete: true
Sorry for the typos, this one works.
Attachment #183711 - Attachment is obsolete: true
Attachment #183714 - Flags: review?(cls)
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.

(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 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.
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
(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?
(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
(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.
Attached file MSYS build notes, updated (obsolete) —
Mount point requirements...
Attachment #183646 - Attachment is obsolete: true
Comment on attachment 183714 [details] [diff] [review]
Cygpath fixes, with typos corrected

This patch is corrupt. Re-uploading.
Attachment #183714 - Flags: review?(cls)
Fix corruption in previous diff
Attachment #183714 - Attachment is obsolete: true
Attachment #183788 - Flags: review?(cls)
Attached patch More path fixes (obsolete) — Splinter Review
Addresses the points in comment #33 and comment #35. Allows source and object
directories to reside on separate drives.
Attached patch More path fixes (obsolete) — Splinter Review
Fix patch context
Attachment #183790 - Attachment is obsolete: true
Attached file MSYS build notes, more updates (obsolete) —
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.
Attachment #183749 - Attachment is obsolete: true
Attachment #183791 - Flags: review?(cls)
Attachment #183792 - Flags: review?(benjamin)
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+
Attached patch make-jar patch to exclude attrs (obsolete) — Splinter Review
As per comment #23, add the -X option to the zip command to exclude any
additional file attributes from the archive.
(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 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
Attachment #183614 - Flags: approval1.8b2?
Attachment #183644 - Flags: approval1.8b2?
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+
(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?
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.
Attachment #183855 - Flags: approval1.8b2?
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.
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)
Attached patch More path fixes, excluding NSPR (obsolete) — Splinter Review
Attachment #183791 - Attachment is obsolete: true
Attachment #183959 - Flags: review?(cls)
Attachment #183788 - Flags: review?(cls) → review+
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-
Attached patch remove mount requirement (obsolete) — Splinter Review
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.
Attachment #183788 - Flags: superreview?(wtchang)
(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.
(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).
(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...
(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 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 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 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 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-
(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.
Attached patch Better path/mount fixes (obsolete) — Splinter Review
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.
Attachment #183959 - Attachment is obsolete: true
Updated to remove the notes about special mount points etc. as they are no
longer needed.
Attachment #183792 - Attachment is obsolete: true
Attachment #184389 - Attachment description: No more path dependencies. → Build notes, revised again
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 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?
(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.
(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 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
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
Comment on attachment 183615 [details] [diff] [review]
MSYS uname support

cls review transferred to attachment 183699 [details] [diff] [review].
Attachment #183615 - Flags: review+
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-
OK, fixed the backslash issues, tested Cygwin/MSVC and MSYS/MSVC.
Attachment #184388 - Attachment is obsolete: true
Attachment #184809 - Flags: review?(cls)
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 :-)
Attachment #184809 - Flags: review?(cls) → review+
Attachment #183614 - Flags: approval1.8b3?
Attachment #183644 - Flags: approval1.8b3?
Attachment #183699 - Flags: approval1.8b3?
Attachment #183855 - Flags: approval1.8b3?
Attachment #183788 - Flags: approval1.8b3?
Attachment #184809 - Flags: approval1.8b3?
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+
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.
Attached patch NSS changes (obsolete) — Splinter Review
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 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 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 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 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 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
I can help check in the NSPR changes if someone will post a NSPR-only patch.
Attachment #183788 - Attachment description: Cygpath fixes, patch fixed → Cygpath fixes, patch fixed (checked in, except NSS changes)
Attachment #184809 - Attachment description: Path/mount fixes again → Path/mount fixes again (checked in)
(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.
Attached patch cygpath tweakSplinter Review
This fixes my cygwin build that uses a relative --enable-srcdir
Attachment #185665 - Flags: review?(cls)
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
(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.
Attachment #185665 - Flags: review?(cls) → review+
Attachment #185665 - Flags: approval1.8b3?
Attachment #185665 - Flags: approval1.8b3? → approval1.8b3+
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 :-)
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 :-(
(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 ?
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
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 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-
Attached patch NSS changes, fixed (obsolete) — Splinter Review
OK, this patch defines USE_CYGWIN in arch.mk and references it in rules.mk.
Attachment #185022 - Attachment is obsolete: true
Attachment #188382 - Flags: review?(wtchang)
(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.
Attached patch NSS changesSplinter Review
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)
Attachment #188382 - Flags: review?(wtchang)
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
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.
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 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)
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 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+
(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.
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.
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
Blocks: 302993
Depends on: 302994
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?
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+
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
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%.
(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)
(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).
http://forums.mozillazine.org/viewtopic.php?t=364654 has the answer. The path is the issue. cygwin/bin needs to be before %windir%\system32 

Depends on: 328499
No longer depends on: 328499
Assignee: hyc → nobody
Status: ASSIGNED → NEW
argh, managed to unassign this while cc'ing myself.
Assignee: nobody → hyc
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?
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
Closed: 16 years ago
Resolution: --- → FIXED
V.Fixed (I'm using it)

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

Attachment

General

Creator:
Created:
Updated:
Size: