944649 - Refresh mozmake for mozillabuild 1.9

Status: RESOLVED FIXED

(Firefox Build System :: MozillaBuild, task)

Description

[Mike Hommey [:glandium]]

So, bug 927213 added mozmake, and it's great. It's however patched, because this was required... until I made the build work with a pristine GNU make, albeit a git snapshot, like mozmake, but unpatched. That's tracked by the dependencies of bug 944569.

However, as I'd like to keep possible fallbacks possibilities open, as well as not forcing people to use pymake if they need to build a tree that works with mozmake but doesn't have the patches for pristine make, I'd like that pristine make not to replace mozmake, and make to still be around because i suspect some people are using mozilla-build as a msys environment, and there is some value in having a pristine msys make around, even if it can't be used to build the mozilla tree anymore.

So I'm proposing that mozillabuild would actually contain 3 different makes:
- make.exe, which would be the pristine msys make it currently is
- mozmake.exe, which would stay the same patched GNU make.
- gmake.exe, which would be the git snapshot but unpatched GNU make.

mach currently picks mozmake if it is present, i'm planning to make it pick gmake first if it is present, then fallback to mozmake if present, then pymake.

Thoughts, before I go forward writing the necessary patch?

Comment 1

[Gregory Szorc [:gps]]

Some build systems will look for both a make and gmake binary when finding make. Therefore, having GNU make 4 available as gmake.exe and older make as make.exe could lead to problems. How about make4.exe?

Comment 2

[Ryan VanderMeulen [:RyanVM]]

Is make 4.0 somehow backwards-incompatible with make 3.x? Why do we need the "pristine" MSYS version if we take 4.0?

Overall, this plan sounds overly complicated to me and I don't understand what the benefits are. Why don't we just fix the tree to use vanilla make, replace MSYS make with 4.0, and get rid of mozmake?

Comment 3

[Mike Hommey [:glandium]]

As I wrote, I'm concerned that people might be using mozilla-build as a more-or-less pristine msys. Msys make and pristine make have subtle differences.
mozmake could go away, but that would mean reverting to default to pymake for builds that don't support pristine make 4 (which is, every changeset since mozmake support landed and until bug 943728 is fixed).

Comment 4

[Mike Hommey [:glandium]]

Attachment: Add GNU make 4.0 and remove mozmake and make 3.81

A few notes:
- this is untested except manually outside the script because i couldn't get mozillabuild to build (but i didn't really try hard).
- i have doubts whether this make could actually be used to build future mozillabuild releases, so it would be better to test that before deciding that make 3.81 is actually good to go (which impacts on whether or not to ship this make as make or another name).
- there is still one patch, but it doesn't change the default behaviour of make. So, the resulting build is like a pristine gnu 4.0, except if you put ".BATCH_MODE_SHELL:" in a makefile, in which case it behaves as mozmake. I hope this will make it upstream (filed https://savannah.gnu.org/bugs/index.php?41246). The reason I did this is because the gaia makefile is basically unfixable without rewriting it entirely, and, well, i have better things to do.
- this actually *builds* make, contrary to what was done for mozmake. It builds fine with msvc10. It seems msvc8 is used to build mozillabuild, it should not matter.
- this also updates to current upstream git head, which has a few more windows fixes, although they don't matter to build firefox and firefox os.
- this was tested on birch with a horrible hack in b2g/gaia to update the gaia makefile. I'll see how things go with make upstream before actually getting the ".BATCH_MODE_SHELL:" line in gaia's makefile (in case they want a different trigger)
- i'll update bug 944650 to pick "make" if it works (instead of gmake).

The second point would need resolving before anything else.

Comment 5

[Ryan VanderMeulen [:RyanVM]]

Comment on attachment 8360920 [details] [diff] [review]
Add GNU make 4.0 and remove mozmake and make 3.81

OK, I played with this patch a bit.

Mozillabuild itself appears to build and package OK. However, I ran into a few things afterwards.

This replaces the copy of make.exe in msys\local\bin but not msys\bin, was that intentional? I can see that local\bin is used preferentially, but I could see it being possibly prolematic if someone's messing with their PATH at all. I'm thinking we should either copy the updated version to both locations or delete the one in msys\bin.

After confirming that |make --version| was giving me 4.0-36-g2462037, I attempted a build on m-c tip with |mach build| and a clean objdir. I got the following error:

The error occurred in code that was called by the mach command. This is either
a bug in the called code itself or in the way that mach is calling it.

You should consider filing a bug for this issue.

If filing a bug, please include the full output of mach, including this error
message.

The details of the failure are as follows:

TypeError: _execute_child() takes exactly 17 arguments (18 given)

  File "c:\mozbuild\src\mozilla-central\python/mozbuild/mozbuild/mach_commands.py", line 404, in build
    silent=not verbose, force_pymake=pymake)
  File "c:\mozbuild\src\mozilla-central\python/mozbuild\mozbuild\base.py", line 467, in _run_make
    return fn(**params)
  File "c:\mozbuild\src\mozilla-central\python/mozbuild\mozbuild\base.py", line 509, in _run_command_in_srcdir
    return self.run_process(cwd=self.topsrcdir, **args)
  File "c:\mozbuild\src\mozilla-central\python/mach\mach\mixin\process.py", line 126, in run_process
    p.run()
  File "c:\mozbuild\src\mozilla-central\testing/mozbase/mozprocess\mozprocess\processhandler.py", line 640, in run
    self.proc = self.Process([self.cmd] + self.args, **args)
  File "c:\mozbuild\src\mozilla-central\testing/mozbase/mozprocess\mozprocess\processhandler.py", line 96, in __init__
    universal_newlines, startupinfo, creationflags)
  File "c:\mozbuild\python\lib\subprocess.py", line 709, in __init__
    errread, errwrite)
Exception AttributeError: "'Process' object has no attribute '_handle'" in <bound method Process.__del__ of <mozprocess.
processhandler.Process object at 0x02760330>> ignored

Comment 6

[Ryan VanderMeulen [:RyanVM]]

Oh crap, that's actually a python issue. I was testing version 2.7.6 locally at the same time. I'll file a bug for that.

However, when I revert to version 2.7.5, it appears that make is using pymake over the newer version of make. I worked around it by renaming the new make.exe to mozmake.exe and it appears to work OK.

Comment 7

[Ryan VanderMeulen [:RyanVM]]

appears that *mach* is using pymake, that is

Comment 8

[Mike Hommey [:glandium]]

The patch was from before we decided to keep make 3.81 and name make 4.0 gnumake. Mach currently tries gnumake or mozmake.

Comment 9

[Ed Morley [:emorley]]

What are the next steps here? :-)

Comment 10

[Ryan VanderMeulen [:RyanVM]]

I guess I'm leaning towards sticking with mozmake for now in favor of getting 1.9 shipped soon. I'm hoping to move towards an MSYS2-based overhaul of MozillaBuild that would bring make 4.0 along for the ride as a longer-term goal.

Mike, does that seem reasonable to you?

Comment 11

[Mike Hommey [:glandium]]

I'd like to give upstream a little bit more time. That is, I'm waiting for an answer from them, and i pinged yesterday.

Comment 12

[Mike Hommey [:glandium]]

As we're not going to include gnu make in mozillabuild 1.9, let's repurpose this bug to refresh mozmake and build it from source instead of including a prebuilt binary.

Comment 13

[Mike Hommey [:glandium]]

Attachment: Refresh mozmake and build it from source

Comment 14

[Ryan VanderMeulen [:RyanVM]]

Comment on attachment 8396108 [details] [diff] [review]
Refresh mozmake and build it from source

Works great! I also removed the existing copy of mozmake.exe that had been checked in with the previous patch.

Comment 15

[Ryan VanderMeulen [:RyanVM]]

http://hg.mozilla.org/mozilla-build/rev/589f380ed5a5