1304508 - Stop using pymake in mozharness

Status: RESOLVED FIXED

(Firefox Build System :: General, defect)

Description

[Gregory Szorc [:gps]]

mozharness is currently using pymake to execute `make check` on Windows. Why, I don't know. Probably a relic from an ancient era. We should have it use `mach` to run the "check" target.

Comment 1

[Gregory Szorc [:gps]]

Attachment: Bug 1304508 - Support passing --keep-going to make;

mozharness is currently making a manual `make -k` invocation. We don't
want automation calling `make` directly. So teach `mach build` to
accept a --keep-going argument that results in `make -k`.

Review commit: https://reviewboard.mozilla.org/r/80218/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/80218/

Comment 2

[Gregory Szorc [:gps]]

Attachment: Bug 1304508 - Use mach for running "check" target;

We switch mozharness to use `mach build` to invoke the "check" make
target.

This was the only consumer of the "enable_pymake" config option, so
that has been removed.

While we're nuking traces of pymake, also remove definitions of the
"make" exe in various configs, which were all specifying pymake.
Fortunately, these appear to not have been used. (If there are
used, I hope things break so we can flush out remaining users of
pymake.)

Review commit: https://reviewboard.mozilla.org/r/80220/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/80220/

Comment 3

[Gregory Szorc [:gps]]

Comment on attachment 8793495 [details]
Bug 1304508 - Use mach for running "check" target;

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/80220/diff/1-2/

Comment 4

[Gregory Szorc [:gps]]

Comment on attachment 8793495 [details]
Bug 1304508 - Use mach for running "check" target;

https://reviewboard.mozilla.org/r/80220/#review78942

OS X builds broke because of this. Apparently mach doesn't think we have a full build environment. Probably due to universal builds blowing away state or something.

Comment 5

[Chris Manchester (limited bugmail, email directly)]

Comment on attachment 8793494 [details]
Bug 1304508 - Support passing --keep-going to make;

https://reviewboard.mozilla.org/r/80218/#review79284

Comment 6

[Gregory Szorc [:gps]]

Comment on attachment 8793494 [details]
Bug 1304508 - Support passing --keep-going to make;

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/80218/diff/1-2/

Comment 7

[Gregory Szorc [:gps]]

Comment on attachment 8793495 [details]
Bug 1304508 - Use mach for running "check" target;

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/80220/diff/2-3/

Comment 8

[Gregory Szorc [:gps]]

Try push had some failures on Windows.

Comment 9

[Gregory Szorc [:gps]]

Attachment: Bug 1304508 - Remove makefile mkdir testing;

Several years ago, joey starting writing a handful of unit tests for
primitives in our make files. IIRC a lot of the impetus behind this
work was to flush out bugs between GNU make and pymake.

AFAICT the only survivor of these tests today is check_mkdir.py. The
test is a one-off and has been a bit fragile over the years,
contributing technical debt along the way. And as part of removing the
last references to pymake in automation, it is once again showing
itself and failing in a way that has to do with the way the test
is written and not an issue with the code it is testing. Enough is
enough. This commit removes the test and eliminates the technical
debt. I don't think it will be missed.

Review commit: https://reviewboard.mozilla.org/r/134122/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/134122/

Comment 10

[Gregory Szorc [:gps]]

Attachment: Bug 1304508 - Remove references to pymake in mozharness configs;

Now that mozharness is calling `mach build` to invoke the "check" target,
there are no more consumers of the "enable_pymake" config option. So we
remove it from the configs.

We also remove definitions of the "make" executable referring to pymake.
The only call sites I could find for "query_exe('make')" are in
testing/mozharness/scripts/mobile_l10n.py. So most of these definitions of
the "make" executable appear to be a cargo cult or left over from a
previous rewrite (the code we just changed to stop calling "make" wasn't
using query_exe). Since mobile_l10n.py is still using query_exe() to find
the make executable, there's a chance switching away from pymake (if this
patch even does that - I don't think we touched a config file related to
that script) could break something. I'm fine with teasing out that bug.

Review commit: https://reviewboard.mozilla.org/r/134124/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/134124/

Comment 11

[Gregory Szorc [:gps]]

Attachment: Bug 1304508 - Remove final references to pymake in mozharness;

We shouldn't be running pymake any more. So code to work around its
behavior is no longer necessary and can be removed. Good riddance.

Review commit: https://reviewboard.mozilla.org/r/134126/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/134126/

Comment 12

[Gregory Szorc [:gps]]

Comment on attachment 8793494 [details]
Bug 1304508 - Support passing --keep-going to make;

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/80218/diff/2-3/

Comment 13

[Gregory Szorc [:gps]]

Comment on attachment 8793495 [details]
Bug 1304508 - Use mach for running "check" target;

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/80220/diff/3-4/

Comment 14

[(not currently active) Ted Mielczarek]

Comment on attachment 8793494 [details]
Bug 1304508 - Support passing --keep-going to make;

https://reviewboard.mozilla.org/r/80218/#review138066

Comment 15

[(not currently active) Ted Mielczarek]

Comment on attachment 8793495 [details]
Bug 1304508 - Use mach for running "check" target;

https://reviewboard.mozilla.org/r/80220/#review138068

Comment 16

[(not currently active) Ted Mielczarek]

Comment on attachment 8862144 [details]
Bug 1304508 - Remove makefile mkdir testing;

https://reviewboard.mozilla.org/r/134122/#review138070

Comment 17

[(not currently active) Ted Mielczarek]

Comment on attachment 8862145 [details]
Bug 1304508 - Remove references to pymake in mozharness configs;

https://reviewboard.mozilla.org/r/134124/#review138072

We sure are good at cargo-culting things all over the place, aren't we?

Comment 18

[(not currently active) Ted Mielczarek]

Comment on attachment 8862146 [details]
Bug 1304508 - Remove final references to pymake in mozharness;

https://reviewboard.mozilla.org/r/134126/#review138076

Comment 19

[Mozilla Autoland]

We're sorry, Autoland could not rebase your commits for you automatically. Please manually rebase your commits and try again.

hg error in cmd: hg rebase -s ac37824d782c -d 3ef61ecb82e2: rebasing 392703:ac37824d782c "Bug 1304508 - Remove makefile mkdir testing; r=ted"
rebasing 392704:62bd2f5d742e "Bug 1304508 - Support passing --keep-going to make; r=chmanchester,ted"
rebasing 392705:7252984b6f94 "Bug 1304508 - Use mach for running "check" target; r=ted"
merging testing/mozharness/mozharness/mozilla/building/buildbase.py
rebasing 392706:cca94e7e75d2 "Bug 1304508 - Remove references to pymake in mozharness configs; r=ted"
other [source] changed testing/mozharness/configs/multi_locale/b2g_win32.py which local [dest] deleted
use (c)hanged version, leave (d)eleted, or leave (u)nresolved? u
merging testing/mozharness/configs/builds/taskcluster_firefox_win32_clang.py
merging testing/mozharness/configs/builds/taskcluster_firefox_win32_clang_debug.py
merging testing/mozharness/configs/builds/taskcluster_firefox_win64_clang.py
merging testing/mozharness/configs/builds/taskcluster_firefox_win64_clang_debug.py
unresolved conflicts (see hg resolve, then hg rebase --continue)

Comment 20

[Gregory Szorc [:gps]]

Comment on attachment 8862144 [details]
Bug 1304508 - Remove makefile mkdir testing;

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/134122/diff/1-2/

Comment 21

[Gregory Szorc [:gps]]

Comment on attachment 8793494 [details]
Bug 1304508 - Support passing --keep-going to make;

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/80218/diff/3-4/

Comment 22

[Gregory Szorc [:gps]]

Comment on attachment 8793495 [details]
Bug 1304508 - Use mach for running "check" target;

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/80220/diff/4-5/

Comment 23

[Gregory Szorc [:gps]]

Comment on attachment 8862145 [details]
Bug 1304508 - Remove references to pymake in mozharness configs;

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/134124/diff/1-2/

Comment 24

[Gregory Szorc [:gps]]

Comment on attachment 8862146 [details]
Bug 1304508 - Remove final references to pymake in mozharness;

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/134126/diff/1-2/

Comment 25

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/71c2fb6144b2
Remove makefile mkdir testing; r=ted
https://hg.mozilla.org/integration/autoland/rev/26433db5f061
Support passing --keep-going to make; r=chmanchester,ted
https://hg.mozilla.org/integration/autoland/rev/4e61e69a383c
Use mach for running "check" target; r=ted
https://hg.mozilla.org/integration/autoland/rev/2fd15ad7081e
Remove references to pymake in mozharness configs; r=ted
https://hg.mozilla.org/integration/autoland/rev/b9ae5e4533dd
Remove final references to pymake in mozharness; r=ted

Comment 26

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

https://hg.mozilla.org/mozilla-central/rev/71c2fb6144b2
https://hg.mozilla.org/mozilla-central/rev/26433db5f061
https://hg.mozilla.org/mozilla-central/rev/4e61e69a383c
https://hg.mozilla.org/mozilla-central/rev/2fd15ad7081e
https://hg.mozilla.org/mozilla-central/rev/b9ae5e4533dd

Comment 27

[Joel Maher ( :jmaher ) (UTC -8)]

I see alerts showing build time improvements:
== Change summary for alert #6298 (as of May 01 2017 17:22 UTC) ==

Improvements:

 98%  build times summary linux32 pgo taskcluster-c4.4xlarge     2876.44 -> 64.25
 97%  build times summary linux64 pgo taskcluster-c4.4xlarge     2487.45 -> 64
 97%  build times summary windows2012-64 pgo taskcluster-c4.4xlarge4353.69 -> 120.32
 97%  build times summary windows2012-32 pgo taskcluster-c4.4xlarge4020.98 -> 120.62
 96%  build times summary osx-10-7 debug buildbot-unknown        4109.26 -> 175.83
 95%  build times summary linux64-stylo opt taskcluster-c4.4xlarge1252.01 -> 62.08
 95%  build times summary osx-10-7 opt buildbot-unknown          3401.66 -> 179.08
 94%  build times summary windowsxp debug buildbot-c3.4xlarge    2090.09 -> 128.7
 94%  build times summary windows8-64 opt buildbot-c3.4xlarge    2020.5 -> 125.05
 94%  build times summary windowsxp opt buildbot-c3.4xlarge      2016.45 -> 130.09
 93%  build times summary windows8-64 debug buildbot-c3.4xlarge  1998.29 -> 133.58
 93%  build times summary linux64 asan taskcluster-c4.4xlarge    1079.12 -> 78.77
 92%  build times summary linux32 debug taskcluster-c4.4xlarge   753.54 -> 63.09
 91%  build times summary windows2012-64 opt taskcluster-c4.4xlarge1335.23 -> 120.89
 91%  build times summary linux32 opt taskcluster-c4.4xlarge     684.74 -> 63.5
 91%  build times summary linux64-stylo debug taskcluster-c4.4xlarge744.54 -> 69.75
 91%  build times summary linux64 debug taskcluster-c4.4xlarge   686.08 -> 64.33
 91%  build times summary windows2012-32 opt taskcluster-c4.4xlarge1270.93 -> 120.34
 91%  build times summary linux64 opt taskcluster-c4.4xlarge     662.06 -> 62.91
 90%  build times summary windows2012-64 debug taskcluster-c4.4xlarge1205.75 -> 120.73
 90%  build times summary windows2012-32 debug taskcluster-c4.4xlarge1188.75 -> 121.13

For up to date results, see: https://treeherder.mozilla.org/perf.html#/alerts?id=6298

Comment 28

[Gregory Szorc [:gps]]

This series broke build metrics for the "summary" reporting. I'll file a new bug.