1278995 - build/win32/mozconfig.vs2015-win64 and build/win32/mozconfig.vs2015 should include mozmake dir in PATH

Status: RESOLVED INACTIVE

(Firefox Build System :: General, defect)

Description

[Pete Moore [:pmoore][:pete]]

TC/BB windows builds download mozmake via tooltool as part of the in-tree mozharness script. They should use this version by making sure it is in the PATH. Currently vs2015u2 binaries downloaded by tooltool are added to the PATH in build/win32/mozconfig.vs2015-win64 and build/win32/mozconfig.vs2015 so it would make sense to add the TOOLTOOL_DIR (or the absolute path this it points to in the case of a relative path) to the PATH here too.

Comment 1

[Pete Moore [:pmoore][:pete]]

Attachment: bug1278995_gecko_v1.patch

Currently Buildbot builds are using mozmake from mozilla-build, which is why make check still passes without this fix. This will:

1) Make sure BB uses the tooltool version of mozmake
2) Fix the issue for TC builds that we are about to land in bug 1244750.

Comment 2

[Pete Moore [:pmoore][:pete]]

Comment on attachment 8761509 [details] [diff] [review]
bug1278995_gecko_v1.patch

Reassigning, as I've noticed glandium already has a queue of review requests.

Comment 3

[Michael Shal [:mshal]]

Comment on attachment 8761509 [details] [diff] [review]
bug1278995_gecko_v1.patch

So our mozmake tooltool package just puts mozmake.exe in topsrcdir? (Not topsrcdir/mozmake/mozmake.exe or some such). I think that is ok, but I'd still prefer if glandium takes a look here as there might be other side-effects I'm not considering.

Comment 4

[Mike Hommey [:glandium]]

Comment on attachment 8761509 [details] [diff] [review]
bug1278995_gecko_v1.patch

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

I have two things to say about this:
- Setting MAKE to the path to mozmake /should/ work, and would avoid adding more to $PATH (which I'd rather not do).
- We have an interesting bootstrapping issue, in that mozbuild.base checks one of gmake, make, mozmake or gnumake can be found in $PATH, and also checks $MAKE, but afaik, it does so *before* any mozconfig is read. And uses that make to process client.mk, which then will execute submakes with the same make. I /think/ setting MAKE in mozconfig would at least force those submakes to use the make we intend it to use.

Comment 5

[Pete Moore [:pmoore][:pete]]

Curiously it is only `make check` that fails to find mozmake - for some reason mozmake is found in `mach build` command. Maybe something buried in a mozconfig or a mozharness config somewhere.

Is there a mach command that runs make check? Maybe if mozharness called this instead, we'd be ok, worth a try at least.

In mozharness, `make -k check` is called here: https://hg.mozilla.org/try/file/7a455380d3a4/testing/mozharness/mozharness/mozilla/building/buildbase.py#l1796

Although I notice that I can update the env via
https://hg.mozilla.org/try/file/7a455380d3a4/testing/mozharness/mozharness/mozilla/building/buildbase.py#l941

So I'll probably do that instead.

Comment 6

[Rob Thijssen [:grenade (EET/UTC+0300)]]

on the win2012 workertype, we don't get this issue because mozilla-build mozmake is in the system path (https://github.com/MozRelOps/OpenCloudConfig/blob/master/userdata/Manifest/win2012.json#L870).

This is also how come buildbot does not have this issue. I don't mind if someone wants to fix this problem by pointing to the tooltool mozmake, but I don't see a need to continue blocking bug 1244750 because of this, since there are no windows automation builds currently relying on tooltool mozmake.

Comment 7

[Gregory Szorc [:gps]]

`mach build check` should be the equivalent of `make check` :)

Comment 8

[Pete Moore [:pmoore][:pete]]

Not actively working on this at the moment.