Closed Bug 1100925 Opened 10 years ago Closed 6 years ago

Switch mozilla-build to MSYS2

Categories

(Firefox Build System :: MozillaBuild, task)

Product:
Firefox Build System
Component:
MozillaBuild
Type:
task
Priority:
Not set
Severity:
normal

Tracking

(Not tracked)

Status:
RESOLVED WONTFIX

People

(Reporter: emorley, Unassigned)

References

(Depends on 1 open bug)

Details

Attachments

(14 files, 5 obsolete files)

vs 2013 64-bit env script
2.33 KB, text/plain
Details
vs 2015 64-bit env script
3.02 KB, text/plain
Details
working msys2 build patch
20.21 KB, patch
Details | Diff | Splinter Review
Bug 1100925 - Mach now detects msys2.
58 bytes, text/x-review-board-request
gps
: review+
Details
Bug 1100925 - Added mingw32-make to the list of possible makes.
58 bytes, text/x-review-board-request
gps
: review+
Details
Bug 1100925 - bin_path method of virtualenv.py in mozbuild now accurately detects if virtualenv is in a windows system.
58 bytes, text/x-review-board-request
gps
: review+
Details
Bug 1100925 - Added flags in virtualenv to differenciate between a native Windows version of Python and a MinGW64 version of Python running in msys2.
58 bytes, text/x-review-board-request
gps
: review+
Details
Bug 1100925 - For whatever reason MinGW Python cannot import functools which is needed in the build process.
58 bytes, text/x-review-board-request
gps
: review+
Details
Bug 1100925 - Virtualenv no longer attempts to use symlinks when the os module does not have it.
58 bytes, text/x-review-board-request
gps
: review+
Details
Bug 1100925 - Virtualenv now copies a site.py file in its folder (if it exists) instead of using the hex string for site.py that is in its source code.
58 bytes, text/x-review-board-request
gps
: review+
Details
Bug 1100925 - Changed old-configure.in to detect mingw32-make as a priority.
58 bytes, text/x-review-board-request
gps
: review+
Details
Bug 1100925 - Added site.py from virtualenv upstream.
58 bytes, text/x-review-board-request
gps
: review+
Details
Bug 1100925 - Added modification for MinGW64 version of Python so it will have system modules.
58 bytes, text/x-review-board-request
gps
: review+
Details
Bug 1100925 - Vendored pylru 1.0.9 into mozilla-central.
58 bytes, text/x-review-board-request
gps
: review+
Details 
Filing this to track any potential switch to the MSYS2 project.
It uses a much newer fork of Cygwin and comes with support for pacman, so should be much more maintainable going forwards - and easier for devs to update/add packages themselves, taking pressure off of the mozilla-build release process.

http://msys2.sourceforge.net/
https://sourceforge.net/projects/msys2/

To experiment with MSYS2, see the installation instructions at:
https://sourceforge.net/p/msys2/wiki/MSYS2%20installation/

See also bug 791511, which is for in the shorter term updating to a newer version of the MSYS1 project.
Issues I've had/filed and are now resolved:
https://sourceforge.net/p/msys2/tickets/98/
https://sourceforge.net/p/msys2/tickets/103/
https://sourceforge.net/p/msys2/tickets/97/
https://sourceforge.net/p/msys2/tickets/102/

Outstanding issues (with tilde expansion when setting HOME to be the Windows user profile directory):
https://sourceforge.net/p/msys2/tickets/111/
To save anyone else some time:
* To start the console in a custom directory, CHERE_INVOKING=1 is your friend:
https://code.google.com/p/conemu-maximus5/wiki/CygwinStartDir
* If trying to use with conemu, add "export TERM=cygwin" to your .profile, to avoid presses of the home/end keys ending up with a tilde ('~') being inserted.
* Conemu settings:
Command: `set MSYSTEM=MSYS & "C:\msys64\usr\bin\bash.exe --login -i"`
Alternatively: `set MSYSTEM=MSYS & set CHERE_INVOKING=1 & "C:\msys64\usr\bin\bash.exe --login -i"`
...and with task params: `/dir "C:\my-startup-directory"`
Why does this depend on bug 791511?
No longer depends on: 791511
Hello Ryan, we talked about this a few days ago, sorry for the delay. It seems that MozillaBuild was created to supplement MSYS with missing features, so I wonder if MozillaBuild would even need to exist in a possible switch to MSYS2.

Maybe someone could list the requirements currently fulfilled by MozillaBuild, to be answered whether MSYS2 provides them out of the box or not. This could be done in a wiki page or even here. Maybe a pacman package could provide the required customizations, or even some documented instructions could be enough.
So starting from a fresh msys2 install, here's a minimal set of things needed to get a buildable gecko:

- First update runtime:
$ pacman --needed -Sy bash pacman pacman-mirrors msys2-runtime

- Restart msys2

- Install things needed for sanity
$ pacman -S vim diffutils patchutils

- Install things needed for mozilla
$ pacman -S yasm p7zip git python2 mercurial upx zip unzip make tar

- We need NSIS in the path, unless we --disable-installer:
export PATH="/c/path-to/nsis-3.0a2:$PATH"

- $PYTHON must point to native windows python, not msys2 python (can we fix?)

- we need (some of) these env vars for a native build:
OSTYPE=msys
MSYSTEM=MINGW32
MACHTYPE=i686-pc-msys

- finally, we need the MSVC env vars.

For the last three issues.. $PYTHON needing to be native windows python has something to do with virtualenv.  The env vars are purely needed for configure to properly detect the build env; I'm not sure which one is actually needed or all three.

The MSVC env vars is the tricky part; we have two options here:
- one, keep using our current start-* scripts and just launch a msys bash shell at the end
- two, rewrite/convert into minimal native shell scripts

I've attached a minimal .sh script that sets things up for 64-bit VS2013 (on a 64-bit) machine.  With a bit of work this could be made more general, especially since we now only support VS2013 and above for builds.  The paths could be queried from the registry too.
I may do more work on this, but if someone else wants to take this on please do!  I'd like feedback if this is a direction that people want to actually go, though.  What I'd suggest is:

- Create a mozilla-build-tools package that includes mozmake.exe and the scripts needed to set up visual studio build environments

- Figure out how to get rid of mozmake dependency -- msys2 has make 4.1, but it is fully using MSYS paths (e.g. abspath returns /c/foo instead of c:/foo).  We may not care about this if we package mozmake.

- Create a bootstrap install script/.bat/.ps that pulls in msys2 and then uses pacman to install the other packages along with downloading nsis

The advantages here are pretty big, in that we would no longer need to maintain any of the things that get bundled with mozilla-build, and updates would be much easier to deal with.
Another advantage.. under msys2, git-cinnabar *just worked* (well, once I added "$*" to the git-remote-hg script that calls git-remote-hg.py).
Some more notes -- msys2's path conversion stuff is documented at http://sourceforge.net/p/msys2/wiki/Porting/ .  There are some cases where this breaks down, specifically cl command lines that include full path names, like:

  cl -FdC:/foo/bar

this incorrectly gets expanded out to

  cl -FdC:C:/migw64/foo/bar

because the path logic is thinking the part after : starts a value for an argument, and initial / gets expanded out.  Luckily, there's an env var that can selectively disable this on an arg basis.  It's a semicolon-separated list of argument prefixes that are to be ignored in this logic.  So far I'm using this for a successful build:

  export MSYS2_ARG_CONV_EXCL=-F

which catches all the cl -Fo -Fd -Fi etc. flags. (side note.. apparently we can also use "-Fo: C:/..." [note the : and the space].  Not sure if this is new with vc 12.0, but it would get around this issue as well and would more closely match normal cc syntax)
Updated env script; WINDOWSSDKDIR needs to be all caps (not WindowsSdkDir), because configure.in looks for $WINDOWSSDKDIR.
Weird bug.  If you run configure in a completely fresh objdir, then nsprpub/config.status has DEFINES like this (for a debug build):

S["DEFINES"]=" -UNDEBUG -DDEBUG_vladimir -UWINNT"

if I rerun configure (with the same mozconfig) in this dir though, I end up with:

S["DEFINES"]=" -UDEBUG -U_DEBUG -UWINNT"

Which then causes things to break when I try to build.  During the second configure run, I see:

Removing C:/proj/moz/t2/nsprpub/config.cache because of host_alias value change from:
  x86_64-pc-mingw32
to:
  undefined
Removing C:/proj/moz/t2/memory/jemalloc/src/config.cache because of CFLAGS value change from:

to:
  undefined
... and that seems to be caused by the msys python being used instead of the win32 one, when "mach environment" is rerun.  The "mach" runner has some smarts to pick a python at the start, but it doesn't take $PYTHON into account.  This fixes it:

-'''which' python2.7 > /dev/null && exec python2.7 "$0" "$@" || exec python "$0" "$@"
+'''[' -e "$PYTHON" ] && exec "$PYTHON" "$0" "$@" || 'which' python2.7 > /dev/null && exec python2.7 "$0" "$@" || exec python "$0" "$@"
 '''
The new git (2.4.1) for windows is based on msys2 and mintty with bash 4.3. It already has very useful builds (including portable) at https://github.com/git-for-windows/git/releases , and it seems quite stable to me, with mintty working fine with colors.

I was thinking that instead of basing mozillabuild directly on msys2, we could instead base it on the new git for windows (GFW). This is because GFW gets extensive exposure and is likely to be stable and with many more users than just Mozilla developers.

This way, if mozillabuild would consist of a diff (zip to extract) over git for windows, I think it would be easier to maintain, more stable, and will allow mozilla devs to use the same environment for both hg and git.
(In reply to Avi Halachmi (:avih) from comment #13)

Hi,

I'm a core MSYS2 developer. Please understand that I am not being negative towards GFW, we're really happy that they've finally ditched MSYS and based their work on MSYS2 and we aim to continue to work together to get all of their fixes merged and working well within MSYS2 itself, but I think - for now at least - that this is the wrong decision for Mozilla to make. I am not belittling the good work that the guys at GFW have done and are continuing to do.

> The new git (2.4.1) for windows is based on msys2 and mintty with bash 4.3.
> It already has very useful builds (including portable) at
> https://github.com/git-for-windows/git/releases , and it seems quite stable
> to me, with mintty working fine with colors.

All of these features are directly inherited from MSYS2, and AFAIK they use many of our packages directly. MSYS2 was always portable in nature. From the start it's be engineered that way. For each 'release' we provide an installer and also an archive. You only need to unpack the archive to a folder to use it. The installer writes a few registry keys to support un-installation and adds some start menu entries.

> 
> I was thinking that instead of basing mozillabuild directly on msys2, we
> could instead base it on the new git for windows (GFW). This is because GFW
> gets extensive exposure and is likely to be stable and with many more users
> than just Mozilla developers.

MSYS2 is used by a large number of individuals, projects and businesses outside of Mozilla (104k package downloads this week from Sourceforge and 2000 downloads of latest installers and archives). GFW's central focus is not on a fully functioning POSIX like build environment for compiling native Windows software. MSYS2's focus is exactly that. For a concrete example of this and why it matters, GFW changed our path conversion routines for all calls to native software from msys2 to suit native git.exe:

https://github.com/git-for-windows/msys2-runtime/commit/9c6b3063d13ec002fe5d106cfec03472f0deede3

(our git.exe is an msys2 application and therefore doesn't call the path conversion routines and so does not need the changes in 9c6b30)

9c6b30 is a mix of:

1. Wrong: ( https://github.com/git-for-windows/msys2-runtime/commit/9c6b3063d13ec002fe5d106cfec03472f0deede3#diff-47ffd31e8cd7e0c954ad3ff6c18d1f29L731 breaks building a native GCC on MSYS2 and many other applications)

2. Not wanted for anything but git - basically the rest of that commit (it needs to be special-cased for when the program being launched is git.exe or perl.exe-by-git.exe).

> 
> This way, if mozillabuild would consist of a diff (zip to extract) over git
> for windows, I think it would be easier to maintain, more stable, and will
> allow mozilla devs to use the same environment for both hg and git.

IMHO, until these issues are resolved (but arguably in general given the difference in focus of each project), you'd be better off making mozillabuild a diff over:

http://sourceforge.net/projects/msys2/files/Base/x86_64/msys2-base-x86_64-20150512.tar.xz/download
and:
http://sourceforge.net/projects/msys2/files/Base/i686/msys2-base-i686-20150512.tar.xz/download
.. instead.
I should also make it clear that MSYS2 provides both hg and git.
(In reply to Ray Donnelly from comment #14)

Thank you for the informative reply.

> All of these features are directly inherited from MSYS2 ...

_IIRC_ Ryan's earlier attempts had issues with mintty, colors and use of arrow keys at the same time, but this GFW build didn't - for the same use case. FWIW, I don't recall having the same issue when using MSYS2 directly (unrelated to mozilla-build environment). At any case, such issues are highly likely to be nothing more than configuration issues, but nevertheless, default configuration does matter.

> ... GFW's central focus is
> not on a fully functioning POSIX like build environment for compiling native
> Windows software. MSYS2's focus is exactly that...

> (our git.exe is an msys2 application and therefore doesn't call the path
> conversion routines and so does not need the changes in 9c6b30)
> 
> 9c6b30 is a mix of:
> 
> 1. ... breaks building a native GCC on MSYS2 and many other applications
> 
> 2. Not wanted for anything but git ...

TBH I'm not entirely sure which path convention would fit the Mozilla build environment better. Mozilla applications are built on Windows using the Visual Studio compiler, but with a mix of normal binutils and custom tools and python scripts (I'm definitely not an expert on this though).

I believe :vlad and :glandium could conduct a more productive discussion on this matter than myself.

Considering that vlad appears to have made some decent progress with MSYS2 based setup, it might not be an issue at all.
(In reply to Avi Halachmi (:avih) from comment #16)
> (In reply to Ray Donnelly from comment #14)
> 
> Thank you for the informative reply.
> 
> > All of these features are directly inherited from MSYS2 ...
> 
> _IIRC_ Ryan's earlier attempts had issues with mintty, colors and use of
> arrow keys at the same time, but this GFW build didn't - for the same use
> case. FWIW, I don't recall having the same issue when using MSYS2 directly
> (unrelated to mozilla-build environment). At any case, such issues are
> highly likely to be nothing more than configuration issues, but
> nevertheless, default configuration does matter.

I believe GFW plan to upstream such fixes, but these things sound like minor annoyances.

> 
> > ... GFW's central focus is
> > not on a fully functioning POSIX like build environment for compiling native
> > Windows software. MSYS2's focus is exactly that...
> 
> > (our git.exe is an msys2 application and therefore doesn't call the path
> > conversion routines and so does not need the changes in 9c6b30)
> > 
> > 9c6b30 is a mix of:
> > 
> > 1. ... breaks building a native GCC on MSYS2 and many other applications
> > 
> > 2. Not wanted for anything but git ...
> 
> TBH I'm not entirely sure which path convention would fit the Mozilla build
> environment better. Mozilla applications are built on Windows using the
> Visual Studio compiler, but with a mix of normal binutils and custom tools
> and python scripts (I'm definitely not an expert on this though).

The issue here is not to do with preferences of path conventions.  Any time an msys2 executable (e.g. bash.exe, make.exe) calls a Windows executable (python.exe, cl.exe, link.exe) it converts each argument and environment variable from (what looks like) msys2 paths (or path lists) to what it thinks is the correct Windows-native value. GFW changed these path conversions routines to keep git.exe happy with the side effect that a lot of other things are unhappy (i.e. broken due to being incorrectly converted).

> 
> I believe :vlad and :glandium could conduct a more productive discussion on
> this matter than myself.
> 
> Considering that vlad appears to have made some decent progress with MSYS2
> based setup, it might not be an issue at all.
(In reply to Ray Donnelly from comment #17)
> The issue here is not to do with preferences of path conventions.  Any time
> an msys2 executable (e.g. bash.exe, make.exe) calls a Windows executable
> (python.exe, cl.exe, link.exe) it converts each argument and environment
> variable from (what looks like) msys2 paths (or path lists) to what it
> thinks is the correct Windows-native value. GFW changed these path
> conversions routines to keep git.exe happy with the side effect that a lot
> of other things are unhappy (i.e. broken due to being incorrectly converted).

FWIW, those path conversions have been so painful that we've solved all issues by simply not using msys make. At this point I don't think we should even try to make the build system work with msys2 make. Using msys2 instead of msys for what we currently use msys for is totally fine, however.
(In reply to Mike Hommey [:glandium] from comment #18)
> (In reply to Ray Donnelly from comment #17)
> > The issue here is not to do with preferences of path conventions.  Any time
> > an msys2 executable (e.g. bash.exe, make.exe) calls a Windows executable
> > (python.exe, cl.exe, link.exe) it converts each argument and environment
> > variable from (what looks like) msys2 paths (or path lists) to what it
> > thinks is the correct Windows-native value. GFW changed these path
> > conversions routines to keep git.exe happy with the side effect that a lot
> > of other things are unhappy (i.e. broken due to being incorrectly converted).
> 
> FWIW, those path conversions have been so painful that we've solved all
> issues by simply not using msys make. At this point I don't think we should
> even try to make the build system work with msys2 make. Using msys2 instead
> of msys for what we currently use msys for is totally fine, however.

I added an environment variable to MSYS2 that helps with these issues: MSYS2_ARG_CONV_EXCL. It's a hack, but a very useful one. You can read about it at:

http://sourceforge.net/p/msys2/wiki/Porting/
(In reply to Ray Donnelly from comment #19)
> I added an environment variable to MSYS2 that helps with these issues:
> MSYS2_ARG_CONV_EXCL. It's a hack, but a very useful one. You can read about
> it at:
> 
> http://sourceforge.net/p/msys2/wiki/Porting/

Yep, MSYS2_ARG_CONV_EXCL was crucial in getting a build working (I discovered it in comment #9 -- thanks for adding that, it's a very helpful escape hatch).

I would suggest that the right way to go is to start with MSYS2, and then have a few mozilla-specific packages (such as something to set up the VC paths etc., which might be a useful package in general).  In my tests, there was nothing in mozilla-build that wasn't already provided by MSYS2 packages (except for NSIS, which is arguably a separate thing anyway).  I don't think we want to do any diffs/unzip-overlays or anything... that's what got us into the current m-b situation.  A .bat/powershell script that will download and install msys2 + moz packages would be the "installer", just like we do on other platforms.  MSYS2 has a package management system, let's use it.

(In reply to Mike Hommey [:glandium] from comment #18)
> FWIW, those path conversions have been so painful that we've solved all
> issues by simply not using msys make. At this point I don't think we should
> even try to make the build system work with msys2 make. Using msys2 instead
> of msys for what we currently use msys for is totally fine, however.

There is no try -- it already works!  Only thing that was needed was adding a few of VS's -F* options to MSYS2_ARG_CONV_EXCL, and the make from MSYS2 builds fine.
We'd like an NSIS package. AFAIR, one of our contributors was working on a PKGBUILD for it, and I think GFW uses NSIS too so they'd perhaps they've also done some work for that.
Vladimir, are these environment variables for MSVC really the only customization required to the system besides installing the necessary dependencies? In that case a pacman package could be enough: people would simply install MSYS2 and the MozillaBuild package. For what it's worth, I am working on the NSIS package, anyone is invited to help: https://sourceforge.net/p/nsis/feature-requests/528.
(In reply to Vladimir Vukicevic [:vlad] [:vladv] from comment #20)
> (In reply to Mike Hommey [:glandium] from comment #18)
> > FWIW, those path conversions have been so painful that we've solved all
> > issues by simply not using msys make. At this point I don't think we should
> > even try to make the build system work with msys2 make. Using msys2 instead
> > of msys for what we currently use msys for is totally fine, however.
> 
> There is no try -- it already works!  Only thing that was needed was adding
> a few of VS's -F* options to MSYS2_ARG_CONV_EXCL, and the make from MSYS2
> builds fine.

I'm surprised you got past the ifeq (a,$(firstword a$(subst /, ,$(abspath .)))) check in config/baseconfig.mk. Anyways, it might work for a basic build, but the build system is also plenty more than that. The worst offenders are l10n repacks and gaia.
I'm not convinced it would make sense for us to use an hypothetical msys2 nsis, if the resulting installers are msys2 based when we'd prefer creating a native win32 installer.
(In reply to Mike Hommey [:glandium] from comment #24)
> I'm not convinced it would make sense for us to use an hypothetical msys2
> nsis, if the resulting installers are msys2 based when we'd prefer creating
> a native win32 installer.

AFAIK the installers produced by the NSIS package Renato mentioned aren't any different from the ones produced by other NSIS builds.
The NSIS package is native Windows software and works the same way as the official binary. Resulting installers have no relation with MSYS2. Providing native software is the general focus of MSYS2, the POSIX layer is more of auxiliary.
I followed these instructions to setup git on windows so I could use git-cinnabar. Git is really slow under MSYS2 compared to git-for-windows and git on OS X. Is there anyway to speed up git under MSYS2?
:djg, other than general NTFS-related tweaking (such as [1]), it's possible you could get some speed by modifying /etc/nsswitch.conf [2] or /etc/fstab [3]. I haven't really tried any of these myself, but there have been some reports of success with them. Other than that, the Cygwin mailing list and IRC channel are probably the best places to find help.

[1] https://stackoverflow.com/questions/4485059/git-bash-is-extremely-slow-in-windows-7-x64
[2] https://cygwin.com/cygwin-ug-net/ntsec.html#ntsec-mapping-nsswitch
[3] https://cygwin.com/cygwin-ug-net/using.html#mount-table
Further to the links David provides .. while Git-for-Windows is now based on MSYS2, the git.exe in MSYS2 is an msys2 executable while the git.exe in Git-for-Windows is a native Windows executable. I intend to spend time over the upcoming holiday season merging and reworking parts of the pull request from Git-for-Windows' lead Johannes Schindelin that addresses this:

https://github.com/Alexpux/Cygwin/pull/8
(In reply to Dan Glastonbury :djg :kamidphish from comment #27)
> I followed these instructions to setup git on windows so I could use
> git-cinnabar. Git is really slow under MSYS2 compared to git-for-windows and
> git on OS X. Is there anyway to speed up git under MSYS2?

If you have Windows Defender (or any other antivirus, really) installed, add your source/objdir (ideally add a common root), mozilla-build, msys2 root, etc. to the exclusion list.  Do this by opening up Windows Defender (in windows < 10), or in settings -> security/update -> windows defender (win 10), and add folder exclusions for the proper paths.

Also, if your source/objdirs are on the system drive, disable 8dot3 filename generation (you should do this anyway for performance, regardless of where your source dir is -- 8dot3 is only enabled by default for the system drive).  Run "fsutil 8dot3 query" to see the current state, "fsutil 8dot3 set 1" to disable on all drives.  This still leaves existing 8dot3 names around; to nuke them, "fsutil 8dot3 strip /s c:\proj" (or wherver your source/objdir/etc. common root is).

However, with those changes, performance of git under msys always felt similar to that of the git-for-windows package for me (and both reasonably fast), so there might be something else going on.
Run `mach doctor` to perform most of the performance tweaks vlad suggested.
I think the info in comment 30 or `mach doctor` should be mentioned in "Windows build prerequisites". I see the exclusion tweak there, but doesn't see anything about 8to3.

FWIW, it seems in Windows 10, 8to3 command is renamed to 8to3name.
Attachment #8698498 - Attachment mime type: application/x-shellscript → text/plain
Updated vs2015 env vars script. My build actually succeeds with this, with one wrinkle -- it failed in an icu dir, but when I ran make manually in that dir, it was fine.  Going to try to reproduce that and see what the issue is.
Attachment #8698498 - Attachment is obsolete: true
(In reply to Vladimir Vukicevic [:vlad] [:vladv] from comment #30)
> However, with those changes, performance of git under msys always felt
> similar to that of the git-for-windows package for me (and both reasonably
> fast), so there might be something else going on.

My source code and mozilla-build are on an SSD that is mapped as m:\, ie. it's a completely separate drive to my OS drive. A git status takes about 16 seconds which makes me :-(
Attached patch Build with MSYS2 patch (obsolete) — — Splinter Review 
With this patch and one of the previous attached envs, I can do "./mach build" in a MSYS2 MINGW64 shell and have the build succeed (with visual studio 2015). "./mach run" equally works.

I'll post a more complete list of packages that need to be installed, but I wanted to get this up and get some feedback.
Attachment #8739163 - Flags: feedback?(ryanvm)
Attachment #8739163 - Flags: feedback?(gps)
Comment on attachment 8739163 [details] [diff] [review]
Build with MSYS2 patch

Glandium should look at this too.
Attachment #8739163 - Flags: feedback?(mh+mozilla)
Comment on attachment 8739163 [details] [diff] [review]
Build with MSYS2 patch

Review of attachment 8739163 [details] [diff] [review]:
-----------------------------------------------------------------

Your tests to detect msys2 *very* likely match in a msys1 environment (one with msys make and msys python)

::: python/virtualenv/virtualenv.py
@@ +1194,5 @@
>  
> +    if is_msys2:
> +        site_customize = os.path.join(lib_dir, 'site-packages', 'sitecustomize.py')
> +        content = ("import sys\n"
> +                   "sys.path.append(\"%s\")" % os.path.dirname(os.__file__))

It's weird that you'd need this. In any case, this should be reviewed upstream.
Attachment #8739163 - Flags: feedback?(mh+mozilla)
(In reply to Mike Hommey [:glandium] from comment #38)
> Comment on attachment 8739163 [details] [diff] [review]
> Build with MSYS2 patch
> 
> Review of attachment 8739163 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> Your tests to detect msys2 *very* likely match in a msys1 environment (one
> with msys make and msys python)

Yep, that's true. Do we care?  But I can use any tests that we think are appropriate -- looks like uname -r returns 2.x.x for msys2 and 1.x for msys1, so I can use that.

> ::: python/virtualenv/virtualenv.py
> @@ +1194,5 @@
> >  
> > +    if is_msys2:
> > +        site_customize = os.path.join(lib_dir, 'site-packages', 'sitecustomize.py')
> > +        content = ("import sys\n"
> > +                   "sys.path.append(\"%s\")" % os.path.dirname(os.__file__))
> 
> It's weird that you'd need this. In any case, this should be reviewed
> upstream.

It's needed due to a quirk of how the mingw64 python is built, apparently.  Upstream virtualenv has seen some of these patches but hasn't been interested in taking them since according to them it's not an "official" python build.  I can try to upstream, but I'd rather patch our local copy first to unblock msys2 working.
Working patch -- didn't change detection and stuff yet, wanted to get a working thing up.  I don't understand why the nsis in msys2 doesn't catch the NSIS_PACKEDVER define -- it's version 3.0b3.  Also the patch would be smaller if we could get hg.exe to work (py2exe weirdness).  Here's my full notes for how to do this, starting from a completely clean windows install:

- Install VS 2015 Community (or Pro etc)
- msys2.github.io - x86_64 version
- update-core, restart msys2 window
- at this point, $HOME is C:/msys64/home/$USER (/home/$USER in msys2).  If you want it to share your Windows user dir: cd /home && mv $USER $USER-old && cmd //c mklink //d $USER C:\\Users\\$USER
- pacman -Sy
- pacman -S mingw-w64-x86_64-make mingw-w64-x86_64-yasm mingw-w64-x86_64-python2 mingw-w64-x86_64-python2-pip
- pacman -S mingw-w64-i686-nsis   -- we specifically need the 32-bit/i686 nsis
- pacman -S wget patch patchutils diffutils autoconf2.13 tar zip unzip
- exit shell, launch mingw64 shell
{ install cmder, setup tasks -- running stuff from mintty doesn't work well? }
-- task for msys2/mingw64: set CHERE_INVOKING=1 & set MSYSTEM=MINGW64 & C:\msys64\usr\bin\bash.exe --login -i -new_console:C:"W:\msys64\msys2.ico"

- launch mingw64 shell - NOTE - a bunch of stuff (e.g. mercurial-setup) doesn't work if just the normal mintty mingw64_shell.bat is run; works fine from cmder/conemu.  Need to investigate.

- pacman -S mingw-w64-x86_64-toolchain mingw-w64-i686-toolchain (just for building mercurial)
- pip install mercurial
- rm /c/msys64/mingw64/bin/hg.exe (we want to use the python script, not a py2exe'd .exe that expects the native win32 python27.dll)
- mkdir c:/proj && cd /c/proj
- hg clone http://hg.mozilla.org/mozilla-central
- cd mozilla-central
- [not yet] ./mach bootstrap (just kidding, not yet!)
- ./mach mercurial-setup
- . ../vs2015-x64-10-env.sh -- can we do this setup in a mozconfig?
- run 'cl //?', make sure we can find the compiler
- MAKE=mingw32-make MAKENSISU=c:/msys64/mingw32/bin/makensis.exe ./mach -l BUILD.LOG build
  -- even if we detect the right make in mozbuild/base.py, some things down the line don't use it properly; so we have to specify it explicitly
  -- the x86 makensis is in the x86 mingw32 path, which isn't visible here (in a x86_64 env shell)
Attachment #8739163 - Attachment is obsolete: true
Attachment #8739163 - Flags: feedback?(ryanvm)
Attachment #8739163 - Flags: feedback?(gps)
Attachment #8739703 - Flags: feedback?(ryanvm)
Attachment #8739703 - Flags: feedback?(mh+mozilla)
Attachment #8739703 - Flags: feedback?(gps)
vlad: is there a pressing need to land this? I ask because the intern I start mentoring in ~6 weeks is tentatively going to work on overhauling the Windows dev environment. You are making their life easier by doing this work and freeing them up to work on other things. It's just that I prefer for interns to work on meaningful, end-to-end projects and your (awesome) work here is starting to encroach on that and diminish their learning experience. So unless there is an urgent need to do this work now, I'm wondering if you wouldn't mind dropping it and waiting a little bit? (For the record, this might be the first time I've asked someone not to contribute to the build system.)
Flags: needinfo?(vladimir)
(In reply to Gregory Szorc [:gps] from comment #41)
> vlad: is there a pressing need to land this? I ask because the intern I
> start mentoring in ~6 weeks is tentatively going to work on overhauling the
> Windows dev environment. You are making their life easier by doing this work
> and freeing them up to work on other things. It's just that I prefer for
> interns to work on meaningful, end-to-end projects and your (awesome) work
> here is starting to encroach on that and diminish their learning experience.
> So unless there is an urgent need to do this work now, I'm wondering if you
> wouldn't mind dropping it and waiting a little bit? (For the record, this
> might be the first time I've asked someone not to contribute to the build
> system.)

There's plenty of work they'll still be able to do -- I'd rather not put this off just so that someone else can pick it up.  There's reasons such as people wanting a more up to date openssh, a nicer console environment, etc. that can all be solved.  Waiting 6 weeks for an intern to start means that having them land this will be on the order of 8-10 weeks.  That's 2-3 months, I don't see any reason to wait.
Flags: needinfo?(vladimir)
Also -- installing git from https://bintray.com/git-for-windows/pacman/mingw-w64-x86_64-git/2.8.1.1.5b8a668-1/view#files/x86_64 gives a much faster git experience, until all that stuff is merged in to msys2 proper.
Comment on attachment 8739703 [details] [diff] [review]
working msys2 build patch

Review of attachment 8739703 [details] [diff] [review]:
-----------------------------------------------------------------

::: browser/installer/windows/nsis/uninstaller.nsi
@@ +20,5 @@
>  RequestExecutionLevel user
>  
>  ; The commands inside this ifdef require NSIS 3.0a2 or greater so the ifdef can
>  ; be removed after we require NSIS 3.0a2 or greater.
> +;;!ifdef NSIS_PACKEDVERSION

There are similar ifdefs in other nsi files, why is only this one a problem? Also, I won't pretend to know NSIS, but this variable looks very much misused in the first place. http://nsis.sourceforge.net/Docs/Chapter5.html#prensispackedversion

::: config/baseconfig.mk
@@ +24,5 @@
>  ifdef .PYMAKE
>  $(error Pymake is no longer supported. Please upgrade to MozillaBuild 1.9 or newer and build with 'mach' or 'mozmake')
>  endif
>  
> +ifeq ($(MSYSTEM),MINGW32)

Still not a desirable check. In fact, doesn't mingw32 make use windows paths?

::: python/mach/mach/mixin/process.py
@@ +29,5 @@
>      raise Exception('Could not detect environment shell!')
>  
>  _in_msys = False
>  
> +if os.environ.get('MSYSTEM', None) != None:

is not None

::: python/mozbuild/mozbuild/virtualenv.py
@@ +71,5 @@
>          # virtualenv.py provides a similar API via path_locations(). However,
>          # we have a bit of a chicken-and-egg problem and can't reliably
>          # import virtualenv. The functionality is trivial, so just implement
>          # it here.
> +        if sys.platform == 'cygwin' or (sys.platform == 'win32' and os.sep != '/'):

This test is weird, even in its existing form, because virtualenv doesn't care about the sys.platform == 'cygwin' case.

::: python/virtualenv/virtualenv.py
@@ +1191,5 @@
>              copyfile(src, join(home_dir, 'registry'), symlink=False)
>          copyfile(join(prefix, 'cachedir'), join(home_dir, 'cachedir'),
>                   symlink=False)
>  
> +    if is_msys2:

You're not explaining why you need this (and it's weird that you'd need it).
Attachment #8739703 - Flags: feedback?(mh+mozilla)
FWIW, the patches for this are at the top of https://github.com/vvuk/mc-hg/commits/msys2-build

I would love to get rid of the topmost "no hg.exe" patch, or at least find a cleaner way to do it.

(In reply to Mike Hommey [:glandium] from comment #45)
ser/installer/windows/nsis/uninstaller.nsi
> @@ +20,5 @@
> >  RequestExecutionLevel user
> >  
> >  ; The commands inside this ifdef require NSIS 3.0a2 or greater so the ifdef can
> >  ; be removed after we require NSIS 3.0a2 or greater.
> > +;;!ifdef NSIS_PACKEDVERSION
> 
> There are similar ifdefs in other nsi files, why is only this one a problem?
> Also, I won't pretend to know NSIS, but this variable looks very much
> misused in the first place.
> http://nsis.sourceforge.net/Docs/Chapter5.html#prensispackedversion

Yeah; given that we require nsis 3.0a2 or greater now in configure, we can just remove this check.

> ::: config/baseconfig.mk
> @@ +24,5 @@
> >  ifdef .PYMAKE
> >  $(error Pymake is no longer supported. Please upgrade to MozillaBuild 1.9 or newer and build with 'mach' or 'mozmake')
> >  endif
> >  
> > +ifeq ($(MSYSTEM),MINGW32)
> 
> Still not a desirable check. In fact, doesn't mingw32 make use windows paths?

We don't actually need to change this check.  If the proper make is used (mingw32-make [even in the mingw64 env; the binary is called mingw32-make.exe]) we're fine.

> ::: python/virtualenv/virtualenv.py
> @@ +1191,5 @@
> >              copyfile(src, join(home_dir, 'registry'), symlink=False)
> >          copyfile(join(prefix, 'cachedir'), join(home_dir, 'cachedir'),
> >                   symlink=False)
> >  
> > +    if is_msys2:
> 
> You're not explaining why you need this (and it's weird that you'd need it).

I'm not 100% sure why; essentially, when copied to a virtualenv, without it python doesn't know how to find its standard libraries.  It seems like a bug in the way the mingw64 python is built, and this is an easy workaround.
Comment on attachment 8739703 [details] [diff] [review]
working msys2 build patch

I don't really have a strong opinion on this from the build system end of things. That's beyond my area of understanding. As long as the other build peers are happy with it, so am I :)
Attachment #8739703 - Flags: feedback?(ryanvm)
Comment on attachment 8739703 [details] [diff] [review]
working msys2 build patch

Review of attachment 8739703 [details] [diff] [review]:
-----------------------------------------------------------------

I fixed hg.exe on MinGW upstream in Mercurial 3.8. So those hacks can be removed.

I'm not a fan of the virtualenv.py hacks. I'd rather we create a new copy of virtualenv.py and apply the msys2 hacks there. The build system can invoke the proper virtualenv.py depending on the environment. Of course, I'd prefer to get these hacks upstream. I can perhaps lean on some Python people to convince them the patches are warranted upstream.
Attachment #8739703 - Flags: feedback?(gps)
I'm not sure how useful it'll be but here's my version of virtualenv changes for mingw-w64-python from Firefox 42 days:

https://github.com/Alexpux/MINGW-packages/blob/master/mingw-w64-firefox/0005-Allow-mingw-w64-python.patch

Here I got Firefox to build OK with GCC on Windows, but there are some issues that I didn't get a chance to fix yet. I keep meaning to get back to fixing it someday as MSYS2 needs a webbrowser.
Depends on: 1275437
Before, mach would only detect MINGW32 (used with msys). Now it also detects MINGW64 (used with msys2).

Review commit: https://reviewboard.mozilla.org/r/57280/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/57280/
Attachment #8759288 - Flags: review?(gps)
Attachment #8759289 - Flags: review?(gps)
Attachment #8759290 - Flags: review?(gps)
Technically, this includes the mingw64 version of make, but it is still called mingw32-make when installed from pacman with msys2.

This is a necessary step to make Firefox build on a pure msys2 environment.

Currently, windows environments use mozmake, which is a patched version of GNU make 4.0, however this needs to be maintained by Mozilla, where it was last updated 3 years ago...

Review commit: https://reviewboard.mozilla.org/r/57282/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/57282/
Before bin_path would detect msys2 as if it were a windows system which would cause a virtualenv problem since the binaries should go in a /bin folder not a /Scripts folder for msys2.

Based on a patch by :vlad https://github.com/vvuk/mc-hg/commit/89b21efe938af375f8f7971e7e3bb2aa0ecbde2a

Review commit: https://reviewboard.mozilla.org/r/57284/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/57284/
This is going to be necessary in the following commits to ensure that virtualenv has access to native libraries.

Review commit: https://reviewboard.mozilla.org/r/57308/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/57308/
Attachment #8759322 - Flags: review?(gps)
By placing the functools module in the REQUIRED_MODULES list, the environment will not throw an ImportError when the build systemimports functools.

Review commit: https://reviewboard.mozilla.org/r/57332/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/57332/
Attachment #8759342 - Flags: review?(gps)
Before, virtualenv would try to use symlinks based entirely on the value given by the options parser.

Unfortunately, some versions of Python cannot symlink (specifically the MinGW version of Python).

Now, virtualenv ensures the os module has the symlink funciton before attempting to use the symlink function.

Review commit: https://reviewboard.mozilla.org/r/57348/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/57348/
Attachment #8759362 - Flags: review?(gps)
(In reply to Nathan Hakkakzadeh [:Nat] from comment #55)
> Now, virtualenv ensures the os module has the symlink funciton before
> attempting to use the symlink function.

Ah great! Could you upstream this fix? (I've hit it many a time elsewhere)
https://github.com/pypa/virtualenv/issues
(In reply to Nathan Hakkakzadeh [:Nat] from comment #55)
> Now, virtualenv ensures the os module has the symlink funciton before
> attempting to use the symlink function.

Note that the symlink function being there doesn't actually indicate that symlinks are possible. (iirc, it's possible to have os.symlink on Windows, and os.symlink can fail on e.g. Linux depending on the file system)
(In reply to Mike Hommey [:glandium] from comment #57)
> (In reply to Nathan Hakkakzadeh [:Nat] from comment #55)
> > Now, virtualenv ensures the os module has the symlink funciton before
> > attempting to use the symlink function.
> 
> Note that the symlink function being there doesn't actually indicate that
> symlinks are possible. (iirc, it's possible to have os.symlink on Windows,
> and os.symlink can fail on e.g. Linux depending on the file system)

Good to know. I'll add a more thorough check.
Attachment #8759288 - Flags: review?(gps) → review+
Comment on attachment 8759288 [details]
Bug 1100925 - Mach now detects msys2.

https://reviewboard.mozilla.org/r/57280/#review54164

::: python/mach/mach/mixin/process.py:33
(Diff revision 1)
>  else:
>      raise Exception('Could not detect environment shell!')
>  
>  _in_msys = False
>  
> -if os.environ.get('MSYSTEM', None) == 'MINGW32':
> +if os.environ.get('MSYSTEM', None) in ['MINGW32', 'MINGW64']:

Nit: use tuples for things like this (tuples are immutable and can be used in some places where lists cannot (such as with `str.startswith`).
Comment on attachment 8759289 [details]
Bug 1100925 - Added mingw32-make to the list of possible makes.

https://reviewboard.mozilla.org/r/57282/#review54166

The commit message about mozmake is a bit harsh. mozmake is literally GNU Make with certain compile time settings. See bug 927213.
Attachment #8759289 - Flags: review?(gps) → review+
Attachment #8759290 - Flags: review?(gps) → review+
Comment on attachment 8759290 [details]
Bug 1100925 - bin_path method of virtualenv.py in mozbuild now accurately detects if virtualenv is in a windows system.

https://reviewboard.mozilla.org/r/57284/#review54172

::: python/mozbuild/mozbuild/virtualenv.py:19
(Diff revision 1)
> +is_win = (sys.platform == 'win32' and os.sep == '\\')
> +is_msys2 = (sys.platform == 'win32' and os.sep == '/')
> +is_cygwin = (sys.platform == 'cygwin')

Nit: these should be UPPERCASE since they are variables in the module scope.
https://reviewboard.mozilla.org/r/57284/#review54178

::: python/mozbuild/mozbuild/virtualenv.py:19
(Diff revision 1)
> +is_win = (sys.platform == 'win32' and os.sep == '\\')
> +is_msys2 = (sys.platform == 'win32' and os.sep == '/')
> +is_cygwin = (sys.platform == 'cygwin')

I could also bikeshed this name. "is_native_win" feels slightly better since technically speaking "is_win" should match all 3 conditions here :)
https://reviewboard.mozilla.org/r/57332/#review54180

wat. What's the fail importing functools?
https://reviewboard.mozilla.org/r/57348/#review54184

What glandium said.

And this fix should definitely go upstream. I can't imagine they'd have trouble taking this one.
https://reviewboard.mozilla.org/r/57332/#review54180

Here's the traceback, it looks like :vlad came up with the same solution [here](https://github.com/vvuk/mc-hg/commit/89b21efe938af375f8f7971e7e3bb2aa0ecbde2a).

``` 0:08.72 cd C:/msys64/home/mozilla-central/obj-x86_64-pc-mingw64
 0:08.77 C:/msys64/home/mozilla-central/configure
 0:09.06 Creating Python environment
 0:09.23 Traceback (most recent call last):
 0:09.23   File "C:/msys64/home/mozilla-central/obj-x86_64-pc-mingw64/_virtualenv/lib/python2.7/site.py", line 703, in <module>
 0:09.23     main()
 0:09.23   File "C:/msys64/home/mozilla-central/obj-x86_64-pc-mingw64/_virtualenv/lib/python2.7/site.py", line 692, in main
 0:09.23     aliasmbcs()
 0:09.23   File "C:/msys64/home/mozilla-central/obj-x86_64-pc-mingw64/_virtualenv/lib/python2.7/site.py", line 515, in aliasmbcs
 0:09.23     import locale, codecs
 0:09.23   File "C:/msys64/home/mozilla-central/obj-x86_64-pc-mingw64/_virtualenv/lib/python2.7/locale.py", line 19, in <module>
 0:09.23     import functools
 0:09.23 ImportError: No module named functools
 ```
https://reviewboard.mozilla.org/r/57332/#review54180

```
 0:08.77 C:/msys64/home/mozilla-central/configure
 0:09.06 Creating Python environment
 0:09.23 Traceback (most recent call last):
 0:09.23   File "C:/msys64/home/mozilla-central/obj-x86_64-pc-mingw64/_virtualenv/lib/python2.7/site.py", line 703, in <module>
 0:09.23     main()
 0:09.23   File "C:/msys64/home/mozilla-central/obj-x86_64-pc-mingw64/_virtualenv/lib/python2.7/site.py", line 692, in main
 0:09.23     aliasmbcs()
 0:09.23   File "C:/msys64/home/mozilla-central/obj-x86_64-pc-mingw64/_virtualenv/lib/python2.7/site.py", line 515, in aliasmbcs
 0:09.23     import locale, codecs
 0:09.23   File "C:/msys64/home/mozilla-central/obj-x86_64-pc-mingw64/_virtualenv/lib/python2.7/locale.py", line 19, in <module>
 0:09.23     import functools
```

Sorry I bungled the Markdown in the previous comment and I can't edit it.
Comment on attachment 8759288 [details]
Bug 1100925 - Mach now detects msys2.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/57280/diff/1-2/
Comment on attachment 8759289 [details]
Bug 1100925 - Added mingw32-make to the list of possible makes.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/57282/diff/1-2/
Comment on attachment 8759290 [details]
Bug 1100925 - bin_path method of virtualenv.py in mozbuild now accurately detects if virtualenv is in a windows system.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/57284/diff/1-2/
Comment on attachment 8759322 [details]
Bug 1100925 - Added flags in virtualenv to differenciate between a native Windows version of Python and a MinGW64 version of Python running in msys2.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/57308/diff/1-2/
Comment on attachment 8759342 [details]
Bug 1100925 - For whatever reason MinGW Python cannot import functools which is needed in the build process.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/57332/diff/1-2/
Comment on attachment 8759362 [details]
Bug 1100925 - Virtualenv no longer attempts to use symlinks when the os module does not have it.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/57348/diff/1-2/
Comment on attachment 8759289 [details]
Bug 1100925 - Added mingw32-make to the list of possible makes.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/57282/diff/2-3/
Comment on attachment 8759290 [details]
Bug 1100925 - bin_path method of virtualenv.py in mozbuild now accurately detects if virtualenv is in a windows system.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/57284/diff/2-3/
Comment on attachment 8759322 [details]
Bug 1100925 - Added flags in virtualenv to differenciate between a native Windows version of Python and a MinGW64 version of Python running in msys2.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/57308/diff/2-3/
Comment on attachment 8759342 [details]
Bug 1100925 - For whatever reason MinGW Python cannot import functools which is needed in the build process.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/57332/diff/2-3/
Comment on attachment 8759362 [details]
Bug 1100925 - Virtualenv no longer attempts to use symlinks when the os module does not have it.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/57348/diff/2-3/
Comment on attachment 8759290 [details]
Bug 1100925 - bin_path method of virtualenv.py in mozbuild now accurately detects if virtualenv is in a windows system.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/57284/diff/3-4/
Comment on attachment 8759322 [details]
Bug 1100925 - Added flags in virtualenv to differenciate between a native Windows version of Python and a MinGW64 version of Python running in msys2.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/57308/diff/3-4/
Comment on attachment 8759342 [details]
Bug 1100925 - For whatever reason MinGW Python cannot import functools which is needed in the build process.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/57332/diff/3-4/
Comment on attachment 8759362 [details]
Bug 1100925 - Virtualenv no longer attempts to use symlinks when the os module does not have it.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/57348/diff/3-4/
Comment on attachment 8760375 [details]
Bug 1100925 - Virtualenv now copies a site.py file in its folder (if it exists) instead of using the hex string for site.py that is in its source code.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/57964/diff/1-2/
Comment on attachment 8760375 [details]
Bug 1100925 - Virtualenv now copies a site.py file in its folder (if it exists) instead of using the hex string for site.py that is in its source code.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/57964/diff/2-3/
Comment on attachment 8760375 [details]
Bug 1100925 - Virtualenv now copies a site.py file in its folder (if it exists) instead of using the hex string for site.py that is in its source code.

https://reviewboard.mozilla.org/r/57964/#review55126

This is a tentative r+ (only because I'm not thrilled at all the virtualenv hacks). But per IRL conversations, these are necessary. I'd like to see the final patch series before any of this lands, however.
Attachment #8760375 - Flags: review?(gps) → review+
Comment on attachment 8759362 [details]
Bug 1100925 - Virtualenv no longer attempts to use symlinks when the os module does not have it.

https://reviewboard.mozilla.org/r/57348/#review55128

This isn't a full fix to avoid symlinks when symlinks aren't supported. But it is better than what it was doing before. Progress is progress.
Attachment #8759362 - Flags: review?(gps) → review+
Comment on attachment 8759322 [details]
Bug 1100925 - Added flags in virtualenv to differenciate between a native Windows version of Python and a MinGW64 version of Python running in msys2.

https://reviewboard.mozilla.org/r/57308/#review55130

::: python/virtualenv/virtualenv.py:56
(Diff revision 4)
>  
>  py_version = 'python%s.%s' % (sys.version_info[0], sys.version_info[1])
>  
>  is_jython = sys.platform.startswith('java')
>  is_pypy = hasattr(sys, 'pypy_version_info')
> -is_win = (sys.platform == 'win32')
> +is_win = (sys.platform == 'win32' and os.sep == '\\')

Is it necessary to change is_win here? Can we get by with just adding is_msys2? I worry about unintended consequences...
Attachment #8759322 - Flags: review?(gps)
Comment on attachment 8759342 [details]
Bug 1100925 - For whatever reason MinGW Python cannot import functools which is needed in the build process.

https://reviewboard.mozilla.org/r/57332/#review55132
Attachment #8759342 - Flags: review?(gps) → review+
https://reviewboard.mozilla.org/r/57308/#review55130

> Is it necessary to change is_win here? Can we get by with just adding is_msys2? I worry about unintended consequences...

The reason I change `is_win` is because there are a lot of Windows specific hacks that do not apply to the MinGW version of Python. I'll try both ways.
This is what we want on msys2 installations, since it is fast and compliant.

Review commit: https://reviewboard.mozilla.org/r/60876/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/60876/
Because if $COMPILE_ENVIRONMENT is not set, configure never checks if the OS is 64-bit or not.

Review commit: https://reviewboard.mozilla.org/r/61116/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/61116/
Attachment #8766072 - Flags: review?(gps)
Attachment #8765594 - Flags: review?(gps)
Attachment #8765595 - Flags: review?(gps)
Comment on attachment 8765594 [details]
Bug 1100925 - Changed old-configure.in to detect mingw32-make as a priority.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/60876/diff/1-2/
Comment on attachment 8765595 [details]
Bug 1100925 - Have JS subconfigure recognize mingw32-make as a valid GMAKE.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/60878/diff/1-2/
Comment on attachment 8765594 [details]
Bug 1100925 - Changed old-configure.in to detect mingw32-make as a priority.

https://reviewboard.mozilla.org/r/60876/#review58288

<p>If it's what we want, shouldn't it be first in the list?</p>
Attachment #8765594 - Flags: review?(gps)
Attachment #8765595 - Flags: review?(gps)
Comment on attachment 8765595 [details]
Bug 1100925 - Have JS subconfigure recognize mingw32-make as a valid GMAKE.

https://reviewboard.mozilla.org/r/60878/#review58290

This should get rolled into the previous patch. Same comment as that one. Except without the <p> which Review Board's web interface added somehow.
Comment on attachment 8766072 [details]
Bug 1100925 - 64-bit build checks only happen on msys2 if $COMPILE_ENVIRONMENT is set.

https://reviewboard.mozilla.org/r/61116/#review58292
Attachment #8766072 - Flags: review?(gps) → review+
https://reviewboard.mozilla.org/r/60876/#review58288

Not entirely sure yet. I've tried it with all sorts of make, there doesn't seem to be a noticable difference in functionality, mingw32-make might be slower though.
Comment on attachment 8759288 [details]
Bug 1100925 - Mach now detects msys2.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/57280/diff/2-3/
Attachment #8759322 - Flags: review?(gps)
Attachment #8765594 - Flags: review?(gps)
Comment on attachment 8759289 [details]
Bug 1100925 - Added mingw32-make to the list of possible makes.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/57282/diff/3-4/
Comment on attachment 8759290 [details]
Bug 1100925 - bin_path method of virtualenv.py in mozbuild now accurately detects if virtualenv is in a windows system.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/57284/diff/4-5/
Comment on attachment 8759322 [details]
Bug 1100925 - Added flags in virtualenv to differenciate between a native Windows version of Python and a MinGW64 version of Python running in msys2.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/57308/diff/4-5/
Comment on attachment 8759342 [details]
Bug 1100925 - For whatever reason MinGW Python cannot import functools which is needed in the build process.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/57332/diff/4-5/
Comment on attachment 8759362 [details]
Bug 1100925 - Virtualenv no longer attempts to use symlinks when the os module does not have it.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/57348/diff/4-5/
Comment on attachment 8760375 [details]
Bug 1100925 - Virtualenv now copies a site.py file in its folder (if it exists) instead of using the hex string for site.py that is in its source code.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/57964/diff/3-4/
Comment on attachment 8765594 [details]
Bug 1100925 - Changed old-configure.in to detect mingw32-make as a priority.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/60876/diff/2-3/
Attachment #8765595 - Attachment is obsolete: true
Attachment #8766072 - Attachment is obsolete: true
https://reviewboard.mozilla.org/r/57308/#review55130

> The reason I change `is_win` is because there are a lot of Windows specific hacks that do not apply to the MinGW version of Python. I'll try both ways.

So the alternative seems incorrect since most of the times `is_win` is specific to the actual windows version of python.
It is just the site.py from the virtualenv upstream but with an added sys.path
for MinGW64.

Review commit: https://reviewboard.mozilla.org/r/62592/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/62592/
Attachment #8768281 - Flags: review?(gps)
Comment on attachment 8759322 [details]
Bug 1100925 - Added flags in virtualenv to differenciate between a native Windows version of Python and a MinGW64 version of Python running in msys2.

https://reviewboard.mozilla.org/r/57308/#review59454
Attachment #8759322 - Flags: review?(gps) → review+
Comment on attachment 8765594 [details]
Bug 1100925 - Changed old-configure.in to detect mingw32-make as a priority.

https://reviewboard.mozilla.org/r/60876/#review59456

This is good. But the commit message needs cleaned up.
Attachment #8765594 - Flags: review?(gps) → review+
Comment on attachment 8768281 [details]
Bug 1100925 - Added site.py from virtualenv upstream.

https://reviewboard.mozilla.org/r/62592/#review59458

You should check in the original site.py as one commit then modify it in another.

Also, where are the changes to virtualenv.py to load this new file?
Attachment #8768281 - Flags: review?(gps) → review-
Added a quick for MinGW64 then adds the default location of system modules to
sys.path.

Review commit: https://reviewboard.mozilla.org/r/62728/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/62728/
Attachment #8768281 - Attachment description: Bug 1100925 - Added custom site.py in virtualenv folder. → Bug 1100925 - Added site.py from virtualenv upstream.
Attachment #8768548 - Flags: review?(gps)
Attachment #8768281 - Flags: review- → review?(gps)
Comment on attachment 8768281 [details]
Bug 1100925 - Added site.py from virtualenv upstream.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/62592/diff/1-2/
Comment on attachment 8768281 [details]
Bug 1100925 - Added site.py from virtualenv upstream.

https://reviewboard.mozilla.org/r/62592/#review59498
Attachment #8768281 - Flags: review?(gps) → review+
Comment on attachment 8768548 [details]
Bug 1100925 - Added modification for MinGW64 version of Python so it will have system modules.

https://reviewboard.mozilla.org/r/62728/#review59502

I can't give r+ because of the TODO and hardcoded path. But otherwise this looks good.

::: python/virtualenv/site.py:69
(Diff revision 1)
>  """
>  
>  import sys
>  import os
>  
> +if 'MSYSTEM' in os.environ and os.environ['MSYSTEM'] == 'MINGW64':

This should be written as `if os.environ.get('MSYSTEM') == 'MINGW64'`
Attachment #8768548 - Flags: review?(gps) → review-
Comment on attachment 8768548 [details]
Bug 1100925 - Added modification for MinGW64 version of Python so it will have system modules.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/62728/diff/1-2/
Attachment #8768548 - Flags: review- → review?(gps)
Comment on attachment 8768548 [details]
Bug 1100925 - Added modification for MinGW64 version of Python so it will have system modules.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/62728/diff/2-3/
Comment on attachment 8768548 [details]
Bug 1100925 - Added modification for MinGW64 version of Python so it will have system modules.

https://reviewboard.mozilla.org/r/62728/#review59520

::: python/virtualenv/site.py:71
(Diff revisions 2 - 3)
>  import sys
>  import os
>  
> -if os.environ.get('MSYSTEM'):
> +if os.environ.get('MSYSTEM') == 'MINGW64':
>      # Hack to add the core python runtime directory for MinGW64 version of Python
>      sys.path.append('C:/msys64/mingw64/lib/python2.7') # TODO: Use which to not have path hardcoded

This TODO still needs addressed before r+ can be granted.
Attachment #8768548 - Flags: review?(gps) → review-
Comment on attachment 8759288 [details]
Bug 1100925 - Mach now detects msys2.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/57280/diff/3-4/
Attachment #8768548 - Flags: review- → review?(gps)
Comment on attachment 8759289 [details]
Bug 1100925 - Added mingw32-make to the list of possible makes.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/57282/diff/4-5/
Comment on attachment 8759290 [details]
Bug 1100925 - bin_path method of virtualenv.py in mozbuild now accurately detects if virtualenv is in a windows system.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/57284/diff/5-6/
Comment on attachment 8759322 [details]
Bug 1100925 - Added flags in virtualenv to differenciate between a native Windows version of Python and a MinGW64 version of Python running in msys2.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/57308/diff/5-6/
Comment on attachment 8759342 [details]
Bug 1100925 - For whatever reason MinGW Python cannot import functools which is needed in the build process.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/57332/diff/5-6/
Comment on attachment 8759362 [details]
Bug 1100925 - Virtualenv no longer attempts to use symlinks when the os module does not have it.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/57348/diff/5-6/
Comment on attachment 8760375 [details]
Bug 1100925 - Virtualenv now copies a site.py file in its folder (if it exists) instead of using the hex string for site.py that is in its source code.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/57964/diff/4-5/
Comment on attachment 8765594 [details]
Bug 1100925 - Changed old-configure.in to detect mingw32-make as a priority.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/60876/diff/3-4/
Comment on attachment 8768281 [details]
Bug 1100925 - Added site.py from virtualenv upstream.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/62592/diff/2-3/
Comment on attachment 8768548 [details]
Bug 1100925 - Added modification for MinGW64 version of Python so it will have system modules.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/62728/diff/3-4/
Comment on attachment 8768548 [details]
Bug 1100925 - Added modification for MinGW64 version of Python so it will have system modules.

https://reviewboard.mozilla.org/r/62728/#review60692

Please add an inline comment on why this is done.
Attachment #8768548 - Flags: review?(gps)
Comment on attachment 8759288 [details]
Bug 1100925 - Mach now detects msys2.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/57280/diff/4-5/
Attachment #8768548 - Flags: review?(gps)
Comment on attachment 8759289 [details]
Bug 1100925 - Added mingw32-make to the list of possible makes.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/57282/diff/5-6/
Comment on attachment 8759290 [details]
Bug 1100925 - bin_path method of virtualenv.py in mozbuild now accurately detects if virtualenv is in a windows system.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/57284/diff/6-7/
Comment on attachment 8759322 [details]
Bug 1100925 - Added flags in virtualenv to differenciate between a native Windows version of Python and a MinGW64 version of Python running in msys2.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/57308/diff/6-7/
Comment on attachment 8759342 [details]
Bug 1100925 - For whatever reason MinGW Python cannot import functools which is needed in the build process.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/57332/diff/6-7/
Comment on attachment 8759362 [details]
Bug 1100925 - Virtualenv no longer attempts to use symlinks when the os module does not have it.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/57348/diff/6-7/
Comment on attachment 8760375 [details]
Bug 1100925 - Virtualenv now copies a site.py file in its folder (if it exists) instead of using the hex string for site.py that is in its source code.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/57964/diff/5-6/
Comment on attachment 8765594 [details]
Bug 1100925 - Changed old-configure.in to detect mingw32-make as a priority.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/60876/diff/4-5/
Comment on attachment 8768281 [details]
Bug 1100925 - Added site.py from virtualenv upstream.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/62592/diff/3-4/
Comment on attachment 8768548 [details]
Bug 1100925 - Added modification for MinGW64 version of Python so it will have system modules.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/62728/diff/4-5/
Comment on attachment 8760375 [details]
Bug 1100925 - Virtualenv now copies a site.py file in its folder (if it exists) instead of using the hex string for site.py that is in its source code.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/57964/diff/6-7/
Comment on attachment 8765594 [details]
Bug 1100925 - Changed old-configure.in to detect mingw32-make as a priority.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/60876/diff/5-6/
Comment on attachment 8768281 [details]
Bug 1100925 - Added site.py from virtualenv upstream.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/62592/diff/4-5/
Comment on attachment 8768548 [details]
Bug 1100925 - Added modification for MinGW64 version of Python so it will have system modules.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/62728/diff/5-6/
This makes building on msys2 easier since its pip is broken.

Review commit: https://reviewboard.mozilla.org/r/64102/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/64102/
Attachment #8770722 - Flags: review?(gps)
Comment on attachment 8759288 [details]
Bug 1100925 - Mach now detects msys2.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/57280/diff/5-6/
Comment on attachment 8759289 [details]
Bug 1100925 - Added mingw32-make to the list of possible makes.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/57282/diff/6-7/
Comment on attachment 8759290 [details]
Bug 1100925 - bin_path method of virtualenv.py in mozbuild now accurately detects if virtualenv is in a windows system.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/57284/diff/7-8/
Comment on attachment 8759322 [details]
Bug 1100925 - Added flags in virtualenv to differenciate between a native Windows version of Python and a MinGW64 version of Python running in msys2.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/57308/diff/7-8/
Comment on attachment 8759342 [details]
Bug 1100925 - For whatever reason MinGW Python cannot import functools which is needed in the build process.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/57332/diff/7-8/
Comment on attachment 8759362 [details]
Bug 1100925 - Virtualenv no longer attempts to use symlinks when the os module does not have it.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/57348/diff/7-8/
Comment on attachment 8760375 [details]
Bug 1100925 - Virtualenv now copies a site.py file in its folder (if it exists) instead of using the hex string for site.py that is in its source code.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/57964/diff/7-8/
Comment on attachment 8765594 [details]
Bug 1100925 - Changed old-configure.in to detect mingw32-make as a priority.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/60876/diff/6-7/
Comment on attachment 8768281 [details]
Bug 1100925 - Added site.py from virtualenv upstream.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/62592/diff/5-6/
Comment on attachment 8768548 [details]
Bug 1100925 - Added modification for MinGW64 version of Python so it will have system modules.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/62728/diff/6-7/
Comment on attachment 8768548 [details]
Bug 1100925 - Added modification for MinGW64 version of Python so it will have system modules.

https://reviewboard.mozilla.org/r/62728/#review61104
Attachment #8768548 - Flags: review?(gps) → review+
Comment on attachment 8770722 [details]
Bug 1100925 - Vendored pylru 1.0.9 into mozilla-central.

https://reviewboard.mozilla.org/r/64102/#review61106

Please update the commit message to note which version of pylru is vendored. That way people can easily audit what you did and figure out the update history for the vendored package.
Attachment #8770722 - Flags: review?(gps) → review+
https://reviewboard.mozilla.org/r/62728/#review61108

Before we land these commits, can you please add in-line comments to vendored code of the form `# MOZ: <explanation>` so it is obvious a change is made by Mozilla. This will help us not accidentally lose these changes when upstream virtualenv updates are applied.
Comment on attachment 8770722 [details]
Bug 1100925 - Vendored pylru 1.0.9 into mozilla-central.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/64102/diff/1-2/
Attachment #8770722 - Attachment description: Bug 1100925 - Vendored pylru into mozilla-central. → Bug 1100925 - Vendored pylru 1.0.9 into mozilla-central.
Comment on attachment 8759362 [details]
Bug 1100925 - Virtualenv no longer attempts to use symlinks when the os module does not have it.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/57348/diff/8-9/
Comment on attachment 8760375 [details]
Bug 1100925 - Virtualenv now copies a site.py file in its folder (if it exists) instead of using the hex string for site.py that is in its source code.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/57964/diff/8-9/
Comment on attachment 8765594 [details]
Bug 1100925 - Changed old-configure.in to detect mingw32-make as a priority.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/60876/diff/7-8/
Comment on attachment 8768281 [details]
Bug 1100925 - Added site.py from virtualenv upstream.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/62592/diff/6-7/
Comment on attachment 8768548 [details]
Bug 1100925 - Added modification for MinGW64 version of Python so it will have system modules.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/62728/diff/7-8/
Comment on attachment 8770722 [details]
Bug 1100925 - Vendored pylru 1.0.9 into mozilla-central.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/64102/diff/2-3/
Pushed by gszorc@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/7ab265d60751
Mach now detects msys2. r=gps
https://hg.mozilla.org/integration/autoland/rev/2dfcc10b2e94
Added mingw32-make to the list of possible makes. r=gps
https://hg.mozilla.org/integration/autoland/rev/e23500a00972
bin_path method of virtualenv.py in mozbuild now accurately detects if virtualenv is in a windows system. r=gps
https://hg.mozilla.org/integration/autoland/rev/bbd6316e775c
Added flags in virtualenv to differenciate between a native Windows version of Python and a MinGW64 version of Python running in msys2. r=gps
https://hg.mozilla.org/integration/autoland/rev/b88241fa95ab
For whatever reason MinGW Python cannot import functools which is needed in the build process. r=gps
https://hg.mozilla.org/integration/autoland/rev/ca387c1ae6a3
Virtualenv no longer attempts to use symlinks when the os module does not have it. r=gps
https://hg.mozilla.org/integration/autoland/rev/b2d61577bd01
Virtualenv now copies a site.py file in its folder (if it exists) instead of using the hex string for site.py that is in its source code. r=gps
https://hg.mozilla.org/integration/autoland/rev/fcfae9412c0d
Changed old-configure.in to detect mingw32-make as a priority. r=gps
https://hg.mozilla.org/integration/autoland/rev/dafdf2ce1247
Added site.py from virtualenv upstream. r=gps
https://hg.mozilla.org/integration/autoland/rev/2a590793fd0e
Added modification for MinGW64 version of Python so it will have system modules. r=gps
https://hg.mozilla.org/integration/autoland/rev/53ee209a0557
Vendored pylru 1.0.9 into mozilla-central. r=gps
What just landed is incomplete. There needs to be some follow-ups before it is usable. I figured it was best to land something before bit rot occurred.

This bug will likely transition into a tracker.
Keywords: leave-open
Depends on: 1287319
Depends on: 1287633
Depends on: 1287943
Depends on: 1289988
Depends on: 1290040
Depends on: 1290044
Depends on: 1290045
Depends on: 1290616
Depends on: 1291944
Depends on: 1292715
Depends on: 1293830
Blocks: mozdev
We've decided we don't want to go in this direction. bug 1311729 covers getting a build working in WSL.
Status: NEW → RESOLVED
Closed: 6 years ago
Resolution: --- → WONTFIX

Since bug 1567506 concluded that WSL won't provide better local user experience, could this be reopened so that at least a volunteer contribution can happen?

Product: mozilla.org → 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: