925605 - Re-allow to build with GNU make on windows

Status: RESOLVED FIXED

(Firefox Build System :: General, defect)

Description

[Mike Hommey [:glandium]]

We made the build system reject non-pymake builds on windows. We should undo this. I've got a build semi-working with GNU make 4.0 with windows-style paths (NOT msys paths, thankfully) with a couple changes to security/build.

Comment 1

[(not currently active) Ted Mielczarek]

I believe the main reasons we decided to reject gmake were the -jN hang (which presumably is fixed), and the path issue. If those aren't issues then it seems fine. Perhaps we want to reject gmake < 4.0 or something like that, however?

Comment 2

[Mike Hommey [:glandium]]

Well, even plain gnu make 4.0 is not suitable. For two reasons:
  https://savannah.gnu.org/bugs/?40227 (most notably the second issue)
  https://savannah.gnu.org/bugs/?40241

And I haven't got a full build yet, but i'm more than halfway through, and I'm hopeful it will work.

Comment 3

[Mike Hommey [:glandium]]

Attachment: WIP

I have a tree that fails to build with pymake that fails similarly with gnu make. I'm currently running a new build with a fresh tree that's not supposed to fail, with this preliminary patch (it's not in a landable shape), using a MSVC-built gnu make 4.0 + two patches + a config adjustment (binary on http://people.mozilla.org/~mhommey/make.exe). I'm hopeful it will finish properly, but my Windows VM is slow, so it's going to take some time.

Comment 4

[Mike Hommey [:glandium]]

Attachment: WIP

With fixups i had forgotten to fold.

Comment 5

[Mike Hommey [:glandium]]

Attachment: WIP

This gets me to the end of a build with this mozconfig:
ac_add_options --disable-webgl
ac_add_options --disable-gamepad
ac_add_options --without-intl-api

The former 2 because i don't have the directx sdk installed, and the latter because there are still msys path issues with the icu build to fix.

It however fails at the very last command (make binaries-deps) because of the too long command line.

Comment 6

[Mike Hommey [:glandium]]

Attachment: Allow to build with a special build of GNU make on windows

Since stock GNU make doesn't work properly (yet) and we need a patched version, I figured the easiest way to ensure a working version is used is to rename the executable. I named it mozmake, but it could be anything else.

I can build in my Windows VM with this patch and http://people.mozilla.org/~mhommey/mozmake.exe , provided i build with --without-intl-api (there are still issues with the icu build). This is good enough imho to review and land. ICU can be handled in a followup (or maybe by a new build of mozmake.exe)

Comment 7

[Mike Hommey [:glandium]]

Ah, I should mention this: it actually fails at the end of the build when MOZ_PSEUDO_DERECURSE has no-skip because the command line for the binary-deps target is too long.

Comment 8

[Mike Hommey [:glandium]]

Attachment: Allow to build with a special build of GNU make on windows

Got icu to build with a small additional change.

Comment 9

[Mike Hommey [:glandium]]

Attachment: Allow to build with a special build of GNU make on windows

Sigh... broken shell quoting on windows...

And with a mach change to use mozmake when it's available.

Comment 10

[Mike Hommey [:glandium]]

Attachment: Allow to build with a special build of GNU make on windows

Sorry for the noise ; this version still allows to force pymake even when mozmake is there.

Comment 11

[Mike Hommey [:glandium]]

Attachment: Allow to build with a special build of GNU make on windows

And a stupid fixup for mach.

Comment 12

[Mike Hommey [:glandium]]

Attachment: Allow to build with a special build of GNU make on windows

So, in then end, running from a non msys command also makes $(MAKE) contain .exe.

Comment 13

[Mike Hommey [:glandium]]

Some timing I got on try with a normal build (non PGO):
- opt, with pymake: 52:32 spent in compilation ; 57:37 in make check
- opt, with mozmake: 46:38 spent in compilation ; 25:43 in make check
- debug, with pymake: 56:26 spent in compilation ; 1:30:07 in make check
- debug, with mozmake: 50:05 spent in compilation ; 36:24 in make check

Comment 14

[Gregory Szorc [:gps]]

Attachment: Default to building with #cores + 2

I'd rather not use a core # multiplier because I don't believe it will
result in much gains (we're only a few percent away from 100% during
compile) and will likely just increase context switching and system
load.

If we had anonymous reporting of build system resource usage back to
Mozilla, would could experiment with values and choose a scientifically
proven value :)

Comment 15

[Gregory Szorc [:gps]]

Comment on attachment 817176 [details] [diff] [review]
Default to building with #cores + 2

Wrong bug.

Comment 16

[Gregory Szorc [:gps]]

I wanted to test this before reviewing. When I did, I ran into an error.

Calling into link_deps to produce binaries-deps.mk produces a "Bad file number" error. I /think/ this has to do with excessively long commands (or at least an argument after the first few thousand bytes). On my machine, the auto-generated temp file holding the recipe for execution by sh.exe contains a line/command that was ~44,000 characters long!

I know there is history behind very long commands and Windows. We may even have a hack deployed on machines to accept very long commands. It sounds like we may need to have link_deps read its arguments from a file?

Comment 17

[Mike Hommey [:glandium]]

(In reply to Gregory Szorc [:gps] from comment #16)
> I know there is history behind very long commands and Windows. We may even
> have a hack deployed on machines to accept very long commands. It sounds
> like we may need to have link_deps read its arguments from a file?

See comment 7. Bug 926733 makes that work.

Comment 18

[Gregory Szorc [:gps]]

(In reply to Mike Hommey [:glandium] from comment #17)
> (In reply to Gregory Szorc [:gps] from comment #16)
> > I know there is history behind very long commands and Windows. We may even
> > have a hack deployed on machines to accept very long commands. It sounds
> > like we may need to have link_deps read its arguments from a file?
> 
> See comment 7. Bug 926733 makes that work.

That is a very hacky workaround and a time bomb if our tree ever gets too large. Hopefully we'll have autogenerated rules for producing object and library files before it detonates. I think we will.

Comment 19

[Gregory Szorc [:gps]]

Comment on attachment 816922 [details] [diff] [review]
Allow to build with a special build of GNU make on windows

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

\o/

::: security/build/Makefile.in
@@ +160,4 @@
>  endif
>  # Hack to force NSS build system to use "normal" object directories
> +DEFAULT_GMAKE_FLAGS += ABS_topsrcdir='$(ABS_topsrcdir)'
> +DEFAULT_GMAKE_FLAGS += BUILD='$(MOZ_BUILD_ROOT)/security/$$(subst $$(ABS_topsrcdir)/security/,,$$(CURDIR))'

Must we export ABS_topsrcdir? I don't see what's wrong expanding it right here.

Comment 20

[Mike Hommey [:glandium]]

(In reply to Gregory Szorc [:gps] from comment #19)
> ::: security/build/Makefile.in
> @@ +160,4 @@
> >  endif
> >  # Hack to force NSS build system to use "normal" object directories
> > +DEFAULT_GMAKE_FLAGS += ABS_topsrcdir='$(ABS_topsrcdir)'
> > +DEFAULT_GMAKE_FLAGS += BUILD='$(MOZ_BUILD_ROOT)/security/$$(subst $$(ABS_topsrcdir)/security/,,$$(CURDIR))'
> 
> Must we export ABS_topsrcdir? I don't see what's wrong expanding it right
> here.

MSYS path mangling fun. If ABS_topsrcdir is expanded directly in BUILD, MSYS fucks up both the paths (MOZ_BUILD_ROOT, too), while it doesn't when doing it this way.

Comment 21

[Mike Hommey [:glandium]]

(In reply to Gregory Szorc [:gps] from comment #18)
> (In reply to Mike Hommey [:glandium] from comment #17)
> > (In reply to Gregory Szorc [:gps] from comment #16)
> > > I know there is history behind very long commands and Windows. We may even
> > > have a hack deployed on machines to accept very long commands. It sounds
> > > like we may need to have link_deps read its arguments from a file?
> > 
> > See comment 7. Bug 926733 makes that work.
> 
> That is a very hacky workaround and a time bomb if our tree ever gets too
> large. Hopefully we'll have autogenerated rules for producing object and
> library files before it detonates. I think we will.

Something else to look forward to: when all variables related to objects, libraries and programs are in moz.build, we can skip directories that don't declare those variables even if they have a Makefile.in.

Comment 22

[Mike Hommey [:glandium]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/5f0bfb16fa4a

Comment 23

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

Backed out in https://hg.mozilla.org/integration/mozilla-inbound/rev/f6cd58cf99ff for doing this: https://tbpl.mozilla.org/php/getParsedLog.php?id=29158060&tree=Mozilla-Inbound

Comment 24

[Mike Hommey [:glandium]]

The previous patch made nsinstall.py be used by all platforms for nss. And it looks like nsinstall.py is actually broken with symlinks. Anyways, the intent was not to use nsinstall.py, and the reason this happened is because NSINSTALL and NSINSTALL_PY are defined in config.mk, and I was testing them before including rules.mk (which includes config.mk). So I moved everything NSINSTALL related after the rules.mk include (I also tries including config.mk earlier, but that breaks the build for other reasons)
https://hg.mozilla.org/integration/mozilla-inbound/rev/afae5911a1e0

Comment 25

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/afae5911a1e0

Comment 26

[cmtalbert]

Did anyone ensure that this works with releng's release locale build automation? I don't want to find ourselves in that pickle again. I'm assuming it will since it returns to how things were, but I'm not certain. Need-info'ing Aki since he had the unenviable job of writing the fixes for much of that pymake for make locales fiasco with the 24 release. Aki, feel free to hot-potato this to someone else if I have pegged you incorrectly.

Comment 27

[Aki Sasaki [:aki] (he/him) (UTC-7)]

I don't think anyone has specifically tested that yet.

The pymake setting for releases is a flag that can be disabled easily; whether it will work is another question.

I think we'll be ok if:

* RelEng continues to do a staging beta release in the final days of aurora
* we turn on gnu make for CI and release builds before requiring it.

If, as the bug summary suggests, we're allowing for gnu make without requiring it, and the staging release during the respective aurora cycle blows up, we have the option of reverting to pymake before the beta 1.

So I would suggest we verify that real live betas work with gnu make before disabling the ability to use pymake in m-c... a couple train migrations down the road.

Comment 28

[cmtalbert]

Thanks for looking into it, Aki and Glandium. I appreciate it.