593585 - Use pymake by default on Windows

Status: RESOLVED FIXED

(Release Engineering :: General, defect, P2)

Comment 1

[Kyle Huey (Exited; not receiving bugmail, old account, do not use)]

Attachment: Configure relatively

Configure relatively so that pymake will work.

Comment 2

[Kyle Huey (Exited; not receiving bugmail, old account, do not use)]

Attachment: Force a switch to pymake from gmake in client.mk

Comment 3

[Kyle Huey (Exited; not receiving bugmail, old account, do not use)]

Attachment: Go fast

Comment 4

[Kyle Huey (Exited; not receiving bugmail, old account, do not use)]

The relative configure works around the problems of switching between pymake and gmake that stalled the previous attempt in Bug 485411 for all of the targets normally used.  We should verify that update generation and such works too before making this live.  I verified it in a VM that releng loaned me but buildbot doesn't generally like me :-)

Comment 5

[Nick Thomas [:nthomas] (UTC+12)]

(In reply to comment #3)
> Created attachment 472156 [details] [diff] [review]

I'm in two minds about this. It would be useful to maintain slave specific settings for make (4-core hardware >>> 1-core VM). But we also need to deal with pymake being merged to other repos at different times (a global change to  catlee, any ideas ?

Comment 6

[Chris AtLee [:catlee]]

(In reply to comment #5)
> (In reply to comment #3)
> > Created attachment 472156 [details] [diff] [review] [details]
> 
> I'm in two minds about this. It would be useful to maintain slave specific
> settings for make (4-core hardware >>> 1-core VM). But we also need to deal
> with pymake being merged to other repos at different times (a global change to 
> catlee, any ideas ?

Well, we're currently at -j4 for VMs and -j1 for ix slaves.  We could instead make the change in choose-make-flags.  Personally, I'd rather just stop including it if it's going to give identical behaviour for all slaves.

Also, mozconfigs for windows are already broken up by branch, so we can land this on mozilla-central without affecting other branches.

Comment 7

[(not currently active) Ted Mielczarek]

Comment on attachment 472154 [details] [diff] [review]
Configure relatively

I'm really not psyched for this being a shell script. I realize you have to run it from client.mk, which doesn't have the benefit of having configure, but this is still pretty awful.

Comment 8

[Kyle Huey (Exited; not receiving bugmail, old account, do not use)]

If we can take a dependency on python 2.6 we can ditch the shell script.

Comment 9

[Ben Turner (not reading bugmail, use the needinfo flag!)]

(In reply to comment #8)
> If we can take a dependency on python 2.6 we can ditch the shell script.

Didn't we make that decision (to require 2.6) already?

Comment 10

[Kyle Huey (Exited; not receiving bugmail, old account, do not use)]

No.  We recently bumped to 2.5 for some test changes related to the no remote XUL stuff.  We've shipped python 2.6 in mozillabuild for sometime, but it has not been required.

Comment 11

[Nick Thomas [:nthomas] (UTC+12)]

(In reply to comment #6)
> Well, we're currently at -j4 for VMs and -j1 for ix slaves. We could instead
> make the change in choose-make-flags.  Personally, I'd rather just stop
> including it if it's going to give identical behaviour for all slaves.

It's possible we might be able to go higher than -j4 on the ix slaves and really work those boxes hard with pymake. eg, the standard rule would say -j6 on a four core box. Then we still desire a slave-specific config.

Presumably -j4 on VMs is an attempt to give the CPU a moderate amount of work in the face of slow I/O, although that seems a little bonkers for places like layout and content.

Comment 12

[Kyle Huey (Exited; not receiving bugmail, old account, do not use)]

Attachment: Better Patch

This may or may not break PGO.  I ran a PGO build locally last night and it appeared to complete much faster than normal, but the resulting binaries didn't depend on any of the pgo instrumentation code.  I pushed this to try to verify that PGO works.

Comment 13

[Kyle Huey (Exited; not receiving bugmail, old account, do not use)]

Comment on attachment 481802 [details] [diff] [review]
Better Patch

This doesn't work if the objdir doesn't already exist.

Comment 14

[Mike Hommey [:glandium]]

FWIW, simply defining the MAKE variable to be "$(PYTHON) $(TOPSRCDIR)/build/pymake/make.py", with no other change, worked for me.

Comment 15

[Kyle Huey (Exited; not receiving bugmail, old account, do not use)]

Locally or on tinderbox?  Things like "make package", etc, just work with that?

Comment 16

[Mike Hommey [:glandium]]

Locally. "make package" is not called through client.mk on tinderbox, so either way won't work.

Comment 17

[Kyle Huey (Exited; not receiving bugmail, old account, do not use)]

Right ... the build automation does things differently, and the point of this approach was not to have to worry about that.

Comment 18

[Kyle Huey (Exited; not receiving bugmail, old account, do not use)]

To be clear, the patch here does a relative configure, so it *does* work on Tinderbox.

It still needed some sorting out with PGO though.

Comment 19

[Mike Hommey [:glandium]]

(In reply to comment #18)
> To be clear, the patch here does a relative configure, so it *does* work on
> Tinderbox.

Not sure what need to be fixed, there, really.

(In reply to comment #17)
> Right ... the build automation does things differently, and the point of
> this approach was not to have to worry about that.

Except you don't make -f client.mk package, you make -C objdir package, which with your patch or not, will do the same thing: use gmake.

Comment 20

[Kyle Huey (Exited; not receiving bugmail, old account, do not use)]

Yes, and with a relative configure gmake and pymake can use the same objdir interchangeably.  If you do an absolute configure with pymake gmake will choke since Windows paths get baked into everything.

Comment 21

[Chris AtLee [:catlee]]

Let's try and do this in the build automation

Comment 22

[Chris AtLee [:catlee]]

pymake is failing in make package-tests with:
e:\builds\moz2_slave\m-cen-w32\build\obj-firefox\testing\mochitest\Makefile:203:0$ e:/builds/moz2_slave/m-cen-w32/build/obj-firefox/config/nsinstall.exe -D ../../dist/test-package-stage/mochitest/content/
e:\builds\moz2_slave\m-cen-w32\build\obj-firefox\testing\mochitest\Makefile:204:0$ cp -RL ../../_tests/testing/mochitest/browser ../../dist/test-package-stage/mochitest/content/
e:\builds\moz2_slave\m-cen-w32\build\obj-firefox\testing\mochitest\Makefile:205:0$ cp -RL ../../_tests/testing/mochitest/chrome ../../dist/test-package-stage/mochitest/content/
e:\builds\moz2_slave\m-cen-w32\build\obj-firefox\testing\mochitest\Makefile:207:0$ cp -RL ../../_tests/testing/mochitest/a11y ../../dist/test-package-stage/mochitest/content/
e:\builds\moz2_slave\m-cen-w32\build\obj-firefox\testing\mochitest\Makefile:210:0: command '(rm -rf ../../dist/test-package-stage/mochitest/content/)' failed, return code -127
e:\builds\moz2_slave\m-cen-w32\build\testing\testsuite-targets.mk:280:0: command 'd:/mozilla-build/python25/python.exe e:/builds/moz2_slave/m-cen-w32/build/build/pymake/pymake/../make.py -C ./testing/mochitest stage-package' failed, return code 2
[Error 2] The system cannot find the file specified

Comment 23

[Kyle Huey (Exited; not receiving bugmail, old account, do not use)]

With the patches in Bug 685655 and Bug 685666 make package-tests completes successfully here with pymake.

Comment 24

[Chris AtLee [:catlee]]

Attachment: Add ability to use pymake to buildbotcustom

Comment 25

[[e-mail] (:bhearsum)]

Comment on attachment 564946 [details] [diff] [review]
Add ability to use pymake to buildbotcustom

r+, assuming this doesn't break leak tests or other post-build steps.

Comment 26

[Justin Wood (:Callek)]

gozer/john,

Will this patch as it stands make using pymake impossible on your new try setup? (as in, do you use generateBranchObjects)?

Comment 27

[Teemu Mannermaa (:wicked)]

I've been running pymake for my local central-builds and came accross a problem with relative paths while running a check. I think pymake still requires relative paths on win due to mixing it with make for some submakes. Check fails with a "ImportError: No module named manifestparser" root error because it can't find that Python module from the build directory. Error comes from build/Makefile.in line 124 that uses a relative srcdir to set include path, which ends up not matching due to pythonpath.py using execfile() to run the end script and that seems to change the cwd.

Comment 28

[Chris AtLee [:catlee]]

waiting for dep bugs to be fixed

Comment 29

[Kyle Huey (Exited; not receiving bugmail, old account, do not use)]

Now that Bug 685665 has been fixed on mozilla-central, catlee is going to test another build with pymake and see if he runs into any more problems.

Comment 30

[Chris AtLee [:catlee]]

almost there...we're now failing on both 'make.py l10n-check' and 'make.py l10n-check MOZ_PKG_PRETTYNAMES=1'

Comment 31

[Chris AtLee [:catlee]]

Attachment: log for make.py l10n-check MOZ_PKG_PRETTYNAMES=1

Comment 32

[Kyle Huey (Exited; not receiving bugmail, old account, do not use)]

I poked at the l10n-check failure today.  It's in some hairy stuff, will take a bit to figure out.

Comment 33

[Rain [:rain1] (inactive)]

Status update: I've been working with a loaner build machine over the past few days. Here's what's left to do:

1. Fix bug 741014, bug 767833, bug 779922, bug 780222, and bug 780241. All five bugs have patches up for review.
2. Land glandium's changes in bug 774032.
3. Work with releng and find a way to run pymake on builds but not on l10n repacks. This would make bug 602908 moot (see bug 602908 comment 11).

And that should be about it, though of course new issues could crop up.

Comment 34

[Rain [:rain1] (inactive)]

Note that my preceding comment only applies to WIn32 opt builds, so hopefully we can enable pymake just for them as a first step. PGO has its own issues, which I'm working through right now.

Comment 35

[Rain [:rain1] (inactive)]

Win32 PGO works now, modulo one bug (bug 780407, fix inbound). I'm getting a ~50 minute speedup on clobber PGO builds compared to trunk, with a majority of time taken in relinking xul.dll so it can't be helped.

(I'm testing without glandium's changes in bug 774032, because they break Pymake per bug 780421.)

Comment 36

[Rain [:rain1] (inactive)]

There's two patches left (bug 741014 and bug 767833), both awaiting reviews from khuey.

Comment 37

[Rain [:rain1] (inactive)]

I just checked in the last couple of fixes to inbound. Once they land on mozilla-central, it's off to releng to switch on Pymake. We'll want to make one more change to m-c once that is done: add mk_add_options MOZ_MAKE_FLAGS="-j4" to both the Win32 mozconfigs but not the Win64 mozconfig. Well, either that or pass in -j4 over the command line for the compile steps.

In the beginning, we'll want to switch to Pymake for Win opt, Win debug and Win pgo builds -- but just the builds. l10n repacks should stay on GNU make for now.

Comment 38

[Chris Cooper [:coop] (he/him)]

Nightly PGO win32 builds are currently failing in my staging env. Log is here:

http://people.mozilla.org/~coop/pymake_w32_nightly_stdio.html

Snippet:
e:\builds\moz2_slave\m-cen-w32-ntly\build\config\rules.mk:762:0$ link -NOLOGO -OUT:mkdepend.exe -PDB:mkdepend.pdb host_cppsetup.obj host_ifparser.obj host_include.obj host_main.obj host_parse.obj host_pr.obj  -MACHINE:X86  
e:\builds\moz2_slave\m-cen-w32-ntly\build\config\rules.mk:762:0: command 'link -NOLOGO -OUT:mkdepend.exe -PDB:mkdepend.pdb host_cppsetup.obj host_ifparser.obj host_include.obj host_main.obj host_parse.obj host_pr.obj  -MACHINE:X86  ' failed, return code 1112
e:\builds\moz2_slave\m-cen-w32-ntly\build\config\makefiles\target_export.mk:31:0: command 'C:/mozilla-build/buildbotve/scripts/python.exe e:/builds/moz2_slave/m-cen-w32-ntly/build/build/pymake/pymake/../make.py -C mkdepend export' failed, return code 2
e:\builds\moz2_slave\m-cen-w32-ntly\build\config\makefiles\target_export.mk:18:0: command 'C:/mozilla-build/buildbotve/scripts/python.exe e:/builds/moz2_slave/m-cen-w32-ntly/build/build/pymake/pymake/../make.py -C config export' failed, return code 2
e:\builds\moz2_slave\m-cen-w32-ntly\build\config\rules.mk:603:0: command 'C:/mozilla-build/buildbotve/scripts/python.exe e:/builds/moz2_slave/m-cen-w32-ntly/build/build/pymake/pymake/../make.py export_tier_base' failed, return code 2
e:\builds\moz2_slave\m-cen-w32-ntly\build\config\rules.mk:569:0: command 'C:/mozilla-build/buildbotve/scripts/python.exe e:/builds/moz2_slave/m-cen-w32-ntly/build/build/pymake/pymake/../make.py  tier_base' failed, return code 2
e:\builds\moz2_slave\m-cen-w32-ntly\build\client.mk:348:0: command 'C:/mozilla-build/buildbotve/scripts/python.exe e:/builds/moz2_slave/m-cen-w32-ntly/build/build/pymake/pymake/../make.py -j1 -C obj-firefox' failed, return code 2
e:\builds\moz2_slave\m-cen-w32-ntly\build\client.mk:193:0: command 'C:/mozilla-build/buildbotve/scripts/python.exe e:/builds/moz2_slave/m-cen-w32-ntly/build/build/pymake/pymake/../make.py -f e:/builds/moz2_slave/m-cen-w32-ntly/build/client.mk realbuild MOZ_PROFILE_GENERATE=1 MOZ_PGO_INSTRUMENTED=1' failed, return code 2
e:\builds\moz2_slave\m-cen-w32-ntly\build\client.mk:149:0: command 'C:/mozilla-build/buildbotve/scripts/python.exe e:/builds/moz2_slave/m-cen-w32-ntly/build/build/pymake/pymake/../make.py -f e:/builds/moz2_slave/m-cen-w32-ntly/build/client.mk profiledbuild' failed, return code 2
program finished with exit code 2

Comment 39

[Gregory Szorc [:gps]]

e:\builds\moz2_slave\m-cen-w32-ntly\build\config\rules.mk:963:0$ cl InvokeClWithDependencyGeneration cl -Fohost_pr.obj -c -TC -nologo -Fdhost_pr.pdb -DXP_WIN32 -DXP_WIN -DWIN32 -D_WIN32 -DNO_X11 -D_CRT_SECURE_NO_WARNINGS -Ox -MT           -DINCLUDEDIR=\"/usr/include\" -DOBJSUFFIX=\".obj\"  -Ie:/builds/moz2_slave/m-cen-w32-ntly/build/config/mkdepend -I. -I../../dist/include  -Ie:/builds/moz2_slave/m-cen-w32-ntly/build/obj-firefox/dist/include/nspr -Ie:/builds/moz2_slave/m-cen-w32-ntly/build/obj-firefox/dist/include/nss     -Ie:/builds/moz2_slave/m-cen-w32-ntly/build/obj-firefox/dist/include/nspr e:/builds/moz2_slave/m-cen-w32-ntly/build/config/mkhost_cppsetup.obj : fatal error LNK1112: module machine type 'x64' conflicts with target machine type 'X86'

Looks like bad arguments to the linker.

Also, -DINCLUDEDIR=\"/usr/include\" isn't a proper path. Although, that will likely be ignored.

Comment 40

[Rain [:rain1] (inactive)]

That shouldn't even be getting built.

Comment 41

[Rain [:rain1] (inactive)]

Seems like it was an edge case of a corner case in our build system. Bug 782759 removes the --disable-auto-deps flags from Windows nightly builds, and should fix the issue. Bug 740854 (no longer blocking this bug) purges mkdepend and friends entirely.

Comment 42

[Chris Cooper [:coop] (he/him)]

Dep build is hitting another error. Log is here:

http://people.mozilla.org/~coop/pymake_w32_dep_stdio.html

Snippet:
e:\builds\moz2_slave\m-cen-w32\build\client.mk:149:0: command 'C:/mozilla-build/buildbotve/scripts/python.exe e:/builds/moz2_slave/m-cen-w32/build/build/pymake/pymake/../make.py -f e:/builds/moz2_slave/m-cen-w32/build/client.mk realbuild' failed, return code 2
crashinject.cpp

C:\Tools\sdks\v7.0\include\winnt.h(135) : fatal error C1189: #error :  "No Target Architecture"


2
Traceback (most recent call last):
  File "e:\builds\moz2_slave\m-cen-w32\build\build\pymake\pymake\process.py", line 224, in run
    rv = m.__dict__[self.method](self.argv)
  File "e:/builds/moz2_slave/m-cen-w32/build/build\cl.py", line 45, in InvokeClWithDependencyGeneration
    sys.exit(ret)
SystemExit: 2
None
program finished with exit code 2

Comment 43

[Rain [:rain1] (inactive)]

Yeah, that bug is bug 782847. The environment variables set in the mozconfig aren't being passed down properly.

Comment 44

[Rain [:rain1] (inactive)]

With the patches in bug 782759, bug 782847 and bug 782866, I was able to do a full build run on Windows using the start-msvc9-x64.bat script.

Comment 45

[Rain [:rain1] (inactive)]

Attachment: Go fast if using pymake

Since we're probably going to be deploying pymake step by step, we'd need to support both -j4 for pymake and -j1 for GNU make.

Comment 46

[Rain [:rain1] (inactive)]

Comment on attachment 655008 [details] [diff] [review]
Go fast if using pymake

https://hg.mozilla.org/integration/mozilla-inbound/rev/8dc1dad174fd

Comment 47

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/8dc1dad174fd

Comment 48

[Chris Cooper [:coop] (he/him)]

Attachment: Set makeCmd to pymake if enable_pymake is set

This essentially unbitrots catlee's old patch.

Tested on dev-staging with win32/64 nightlies, deps, and repacks.

Comment 49

[Chris Cooper [:coop] (he/him)]

Attachment: Set enable_pymake for windows platforms, turn it off for beta, release, and ESR

Comment 50

[Chris Cooper [:coop] (he/him)]

Comment on attachment 656132 [details] [diff] [review]
Set makeCmd to pymake if enable_pymake is set

https://hg.mozilla.org/build/buildbotcustom/rev/0a174ceb4df8

Comment 51

[Chris Cooper [:coop] (he/him)]

Comment on attachment 656133 [details] [diff] [review]
Set enable_pymake for windows platforms, turn it off for beta, release, and ESR

https://hg.mozilla.org/build/buildbot-configs/rev/db00706081f8

Comment 52

[[e-mail] (:bhearsum)]

Comment on attachment 656132 [details] [diff] [review]
Set makeCmd to pymake if enable_pymake is set

This got backed out because of a new failure. Not sure if it's buildbot related or build system yet. The first hunk of this patch needs to use forward slashes, too. We had landed a bustage fix for it but it was backed out.

Comment 53

[Rain [:rain1] (inactive)]

The failure was fixed by backing out bug 784262.

Comment 54

[Chris Cooper [:coop] (he/him)]

Comment on attachment 656132 [details] [diff] [review]
Set makeCmd to pymake if enable_pymake is set

Re-landed with slashes fixed:

https://hg.mozilla.org/build/buildbotcustom/rev/cae1e695a890

Comment 55

[Chris Cooper [:coop] (he/him)]

Comment on attachment 656133 [details] [diff] [review]
Set enable_pymake for windows platforms, turn it off for beta, release, and ESR

Re-landed:

https://hg.mozilla.org/build/buildbot-configs/rev/a0a265e04561

:sid0 is right: we can just pref this off in the configs next time (enable_pymake: False) if we need to iterate (or limit to a project branch, say).

Comment 56

[Chris Cooper [:coop] (he/him)]

Attachment: Only enable pymake on the build-system branch (for now)

Comment 57

[Chris Cooper [:coop] (he/him)]

Comment on attachment 656133 [details] [diff] [review]
Set enable_pymake for windows platforms, turn it off for beta, release, and ESR

Backed out again.

Comment 58

[Chris Cooper [:coop] (he/him)]

Comment on attachment 656527 [details] [diff] [review]
Only enable pymake on the build-system branch (for now)

https://hg.mozilla.org/build/buildbot-configs/rev/219aadf2983b

Comment 59

[Rain [:rain1] (inactive)]

So after bug bug 786791 and bug 786866 are fixed, the package steps finally work. Woo hoo!

https://tbpl.mozilla.org/php/getParsedLog.php?id=14824570&tree=Build-System&full=1 indicates that the check step is still using GNU Make. We need to switch that to Pymake.

Comment 60

[Rain [:rain1] (inactive)]

Attachment: Make the check step use Pymake

Is this patch correct?

Comment 61

[Chris AtLee [:catlee]]

Comment on attachment 656735 [details] [diff] [review]
Make the check step use Pymake

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

::: steps/unittest.py
@@ +399,5 @@
>          self.description = [test_name + " test"]
>          self.descriptionDone = [self.description[0] + " complete"]
>          self.super_class = ShellCommandReportTimeout
>          ShellCommandReportTimeout.__init__(self, **kwargs)
>          self.addFactoryArguments(test_name=test_name)

you need to pass makeCmd=makeCmd to addFactoryArguments. r+ otherwise

Comment 62

[Chris Cooper [:coop] (he/him)]

Comment on attachment 656735 [details] [diff] [review]
Make the check step use Pymake

https://hg.mozilla.org/build/buildbotcustom/rev/52770a4b1764

Comment 63

[[e-mail] (:bhearsum)]

Comment on attachment 656735 [details] [diff] [review]
Make the check step use Pymake

This patch made it to production.

Comment 64

[Rain [:rain1] (inactive)]

The build-system repository is now green for Win32 opt and debug.

Comment 65

[Chris Cooper [:coop] (he/him)]

Attachment: Re-enable pymake everywhere except aurora, beta, release, and ESR

Comment 66

[Chris Cooper [:coop] (he/him)]

Attachment: Re-enable pymake for win32 everywhere except aurora, beta, release, and ESR

Also leaves pymake enabled for win64 on build-system so :sid0 can continue to work on it there.

Comment 67

[[e-mail] (:bhearsum)]

Comment on attachment 657067 [details] [diff] [review]
Re-enable pymake for win32 everywhere except aurora, beta, release, and ESR

Landed + merged to production. Will be enabled shortly.

Comment 68

[[e-mail] (:bhearsum)]

Made it to production today, again.

Comment 69

[Rain [:rain1] (inactive)]

There are still a few issues which we can tackle in followups, but this looks fixed to me!

Comment 70

[Rain [:rain1] (inactive)]

Here's a patch to make Try work for Gecko 15-17 -- https://gist.github.com/3582912

If you'd like to build Gecko 15-17 on try, simply add this patch to the queue that you push.

Comment 71

[Bill Gianopoulos [:WG9s]]

But, on the otherhand, things you are landing to try should be targeted to Firefox 18.  Anything landing on a version prior to that should be based on Mozilla-central working on Firefox 18 and not based on a try build on an earlier version in any case.

Comment 72

[Rain [:rain1] (inactive)]

I'm sure people would like to use the try server to test out patches they backport.

Comment 73

[Bill Gianopoulos [:WG9s]]

Perhaps, but one would think such things should be tested against Mozilla-central first and then uplifted based on not dependent on anything else on Aurora already and being low risk.  those are the rules, so I really do not see why this is required at all.

Comment 74

[Bill Gianopoulos [:WG9s]]

Besides.  I think it is great that we finally have pymake working for mozilla-central builds.  I see no good reason to try to get it working on beta or Aurora at this point.

Comment 75

[Jordan Lund (:jlund)]

Attachment: 593585_remove_enable_pymake-140326.patch

I think it's safe to rip this loop out now. Catlee what do you think. Will anything still touch this? /me printing config.py asserts that to be the case

Granted we will be moving off pymake with https://bugzil.la/927671

Comment 76

[Chris AtLee [:catlee]]

Comment on attachment 8397270 [details] [diff] [review]
593585_remove_enable_pymake-140326.patch

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

yeah, I think it's safe. you can verify with dump masters, or add an assert False inside that loop to make sure it's not being hit.

Comment 77

[Jordan Lund (:jlund)]

Comment on attachment 8397270 [details] [diff] [review]
593585_remove_enable_pymake-140326.patch

pushed to default: https://hg.mozilla.org/build/buildbot-configs/rev/34bbfa7b4a5e

dump master yielded a 0 diff

Comment 78

[John Hopkins (:jhopkins)]

in production.