1195299 - Remove duplicated mozbase modules from mozharness

Status: RESOLVED FIXED

Description

[Andrew Halberstadt [:ahal]]

There are copies of mozinfo, mozfile and mozprocess under testing/mozharness. These should be removed and mozharness should just use the version in testing/mozbase.

These versions are likely way out of date and some work may be needed to replace them.

Comment 1

[Armen [:armenzg] (back on May 20th, 2019)]

I believe base script should create the venv asap and install the minimum set of mozbase with open ended versions.
We can let the actual harnesses pick the exact version when they call create_virtualenv step.

For instance, for the run_command() [1] we can do the following and it works if there is a venv:
 site_packages_path = self.script_obj.query_python_site_packages_path()
 sys.path.insert(0, site_packages_path)
 from mozprocess import ProcessHandler

We can make this more generic to handle as well the remaining mozbase packages.

[1] http://hg.mozilla.org/mozilla-central/file/default/testing/mozharness/mozharness/base/script.py#l994

Comment 2

[Armen [:armenzg] (back on May 20th, 2019)]

Attachment: include_mozbase.diff

This patch shows the alternative approach which we would include each mozbase module before calling it.

Unfortunately this approach won't work unless we also pull mozbase for the test jobs.

I'm going to look again into the virtualenv approach.

Comment 3

[Armen [:armenzg] (back on May 20th, 2019)]

jlund also proposed if we did things a bit more close to other python projects.

For instance, we could drop virtualenv.py into the repo and do this in two steps before calling mozharness:
rm -rf build/venv
python external_tools/virtualenv.py build/venv
build/venv/bin/pip install mozinfo mozfile mozprocess --find-links http://pypi.pub.build.mozilla.org/pub --find-links http://pypi.pub.build.mozilla.org/pvt

We would also need to prevent clobber from clearing the venv.

We would need to call the script like this:
build/venv/python scripts/...

Calling that specific python would remove the need to modify sys.path afaik.

Comment 4

[Armen [:armenzg] (back on May 20th, 2019)]

Another approach would be to remove using mozbase modules by Mozharness.
We don't use it in that many places.
I believe this would take less work and be cleaner.

Comment 5

[Andrew Halberstadt [:ahal]]

(In reply to Armen Zambrano Gasparnian [:armenzg] from comment #4)
> Another approach would be to remove using mozbase modules by Mozharness.
> We don't use it in that many places.
> I believe this would take less work and be cleaner.

I think there was a reason we were using mozprocess, probably for the output timeout capability. Between this option and hacking sys.path, I'd rather just hack sys.path (if virtualenv is too hard).

Comment 6

[Armen [:armenzg] (back on May 20th, 2019)]

We could create a small script with bits and pieces from this patch [1]

This script could be called "mozharness_bootstrap.py" under external_tools/
It would be able to set up a minimal venv for mozharness.
It could read from configs/releng_infra_configs to determine information about where to find virtualenv on each host.
As soon as archiver is called, we could call this script and generate a "mh_venv" before calling the mozharness script.

After this minimal venv is generated we call our typical mozharness script like this:
> mh_venv/bin/python scripts/desktop_unittest.py ...

We could also take the approach of making the bootstrap script be a single call (instead of two):
> python mozharness_bootstraph.py scripts/desktop_unittest.py ...

Internally we set up the venv, take sys.argv[1:] and call that.

We could even take this one step further by making the bootstrap script to read a buildbot property which contains the whole command and arguments.
This would allow to use any builder as a generic builder since we could schedule a buildername with a property specifiying which command to execute.
Obviously job definition would live in-tree. In fact, we could teach taskcluster's decision task to schedule everything in such generic builder :)

This concept is slightly based on bug 1078362.

Nevertheless, this is a comple scope creep. Getting rid of the older mozbase modules comes first :)


[1] https://bugzilla.mozilla.org/attachment.cgi?id=8656600

Comment 7

[Jordan Lund (:jlund) PTO until June 12th]

> Nevertheless, this is a comple scope creep. Getting rid of the older mozbase
> modules comes first :)


a bootstrap script or single venv has been brought up a number of times. whether it is scope creep or not, ++ to filing and investigating this further.

imagine a world where every slave (regardless of platform) had a single mozharness_venv in a determinable path. though this comes with the cost of mozharness no longer being able to be run with a minimal subset of requirements. Maybe we could keep base/* clean from script specific modules and instead have a mozilla_requirements.txt file

Comment 8

[Andrew Halberstadt [:ahal]]

I'd like to point out that mozharness is using a very old version of mozprocess still, one prior to bug 794984. Without that bug, the stdout/stderr pipes can potentially fill up which causes any process that writes to them to block. I don't know if this is happening anywhere in practice or not, but thought it's worth mentioning that fixing this could potentially result in major perf gains (or maybe not).

I'd recommend doing this in two parts:
1) Simply update the mozbase modules under testing/mozharness to current
2) Figure out the bootstrapping problem later

Comment 9

[Andrew Halberstadt [:ahal]]

Here's a full try run with mozbase packages in mozharness updated.. I think most of those oranges are known, but the windows "CL" jobs are a bit worrisome:
https://treeherder.mozilla.org/#/jobs?repo=try&revision=1cd64de8023c9db17e84325a87ee4e3df7267717

Comment 10

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

In bug 1362070 I'm upgrading mozprocess in mozharness. I'll create a separate bug for upgrading mozinfo and mozfile, and make that a dependency of this bug too.

After those dependent bugs are resolved, we'll still have work left in this bug to remove the mozharness copies, but getting the copies first in sync should make the second step of removing the duplicate copy a little easier.

Comment 11

[Andrew Halberstadt [:ahal]]

Attachment: Bug 1195299 - [ci] Use 'mach python' to run mozharness scripts when we have a srcdir

This makes sure that the mozharness scripts have access to all the packages in
the build system virtualenv (namely mozbase).

Comment 12

[Andrew Halberstadt [:ahal]]

Attachment: Bug 1195299 - [mozharness] Remove copies of mozbase from testing/mozharness

Depends on D22184

Comment 13

[Henrik Skupin (:whimboo) [⌚️UTC+2]]

Andrew, please make sure to run a try build. I feel this is important given that the mozprocess code has been changed a lot in the last couple months, and which might cause regressions.

Comment 14

[Andrew Halberstadt [:ahal]]

I'm not planning to push this through right now. These patches are up here to demonstrate how we can remove these extra mozbase modules from a technical standpoint.

There are tons of bustages caused by this patch, though so far they are all due to mozbase not being available in a variety of contexts where mozharness is. I might chip away at this over time, but if anyone wants to work on it, please feel free (just ping me first in case I have more changes locally).

Comment 15

[Chris AtLee [:catlee]]

I did a push to Try here:

https://treeherder.mozilla.org/#/jobs?repo=try&revision=46353bca84dc5264dac319d90c462d57cd4cf832

That's based on :ahal's patches, with a small fix for Windows. I don't see any mozharness/mozbase related failures there!

Comment 16

[Chris AtLee [:catlee]]

Attachment: Bug 1195299: Use mozilla-build python to run mach

Comment 17

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/4de7f94119fd
[ci] Use 'mach python' to run mozharness scripts when we have a srcdir r=catlee
https://hg.mozilla.org/integration/autoland/rev/9645ac1a9851
[mozharness] Remove copies of mozbase from testing/mozharness r=catlee

Comment 18

[Cristina Coroiu [:ccoroiu]]

https://hg.mozilla.org/mozilla-central/rev/4de7f94119fd
https://hg.mozilla.org/mozilla-central/rev/9645ac1a9851

Comment 19

[Natalia Csoregi [:nataliaCs]]

[Tracking Requested - why for this release]:

Backed out for l10n bustages e.g. https://treeherder.mozilla.org/logviewer.html#/jobs?job_id=238357055&repo=autoland&lineNumber=364

backout: https://hg.mozilla.org/mozilla-central/rev/93075ec49df3982c26873b822d762bd3d8863fad

[task 2019-04-05T11:09:42.746Z] + python2.7 /builds/worker/workspace/build/src/testing/mozharness/scripts/desktop_l10n.py --clone-locales --list-locales --setup --repack --summary --locale=en-CA:default --locale=he:default --locale=it:default --locale=ja:default --config single_locale/firefox.py --config single_locale/linux32.py --config single_locale/tc_common.py --config single_locale/tc_linux_common.py --log-level=debug --scm-level=3 --work-dir=/builds/worker/workspace/build
[task 2019-04-05T11:09:48.503Z] Traceback (most recent call last):
[task 2019-04-05T11:09:48.503Z] File "/builds/worker/workspace/build/src/testing/mozharness/scripts/desktop_l10n.py", line 20, in <module>
[task 2019-04-05T11:09:48.503Z] from mozharness.base.script import BaseScript
[task 2019-04-05T11:09:48.503Z] File "/builds/worker/workspace/build/src/testing/mozharness/mozharness/base/script.py", line 55, in <module>
[task 2019-04-05T11:09:48.503Z] import mozinfo
[task 2019-04-05T11:09:48.503Z] ImportError: No module named mozinfo

Comment 20

[Axel Hecht [:Pike]]

FYI, the entry point here is taskcluster/scripts/builder/build-l10n.sh, which was still using python2.7 directly.

Comment 21

[Andrew Halberstadt [:ahal]]

Thanks, should be a quick fix. This is one of those patches that affects the world and is really hard to catch everything on try, so was expecting a backout tbh.

Comment 22

[Henrik Skupin (:whimboo) [⌚️UTC+2]]

Should we also try a beta simulation with this patch?

Comment 23

[Andrew Halberstadt [:ahal]]

I don't think we need to, unless there are mozharness/taskcluster scripts that exist on beta but not central? I don't think this is something that needs to be uplifted either.

Comment 24

[Henrik Skupin (:whimboo) [⌚️UTC+2]]

It's not about uplifts but checks that current m-c code would work on beta after the next merge day. If we land as is without running such a merge we could bust the weekly beta-sim jobs sheriffs most likely are still running.

Comment 25

[Andrew Halberstadt [:ahal]]

Just waiting on try:
https://treeherder.mozilla.org/#/jobs?repo=try&revision=1de59939ee5818c8b772402abb06b53d4a191057

I also ended adding the same $GECKO_PATH check to the test-macosx.sh script, even though nothing using that script has a source checkout yet, just to future proof a bit.

Comment 26

[Andrew Halberstadt [:ahal]]

(In reply to Henrik Skupin (:whimboo) [⌚️UTC+1] from comment #24)

It's not about uplifts but checks that current m-c code would work on beta after the next merge day. If we land as is without running such a merge we could bust the weekly beta-sim jobs sheriffs most likely are still running.

I'm just not sure how this change could work on central but not beta, it's all self-contained in the same commit. But I'm not very familiar with the merge process so if someone wants to run a beta sim (I'm not sure how), feel free!

Comment 27

[Andrew Halberstadt [:ahal]]

Ok, I think I ran one:
https://treeherder.mozilla.org/#/jobs?repo=try&revision=a0cdc4a1a2dde46dfb6ebb13cd846a3f32688462

But I realized this is running every task which is definitely overkill. I'll let the builds finish + a couple tests, then cancel the rest.

Comment 28

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/50495a5da9f6
[ci] Use 'mach python' to run mozharness scripts when we have a srcdir r=catlee
https://hg.mozilla.org/integration/autoland/rev/c28c538fca0a
[mozharness] Remove copies of mozbase from testing/mozharness r=catlee

Comment 29

[Andreea Pavel [:apavel]]

https://hg.mozilla.org/mozilla-central/rev/50495a5da9f6
https://hg.mozilla.org/mozilla-central/rev/c28c538fca0a

Comment 30

[Andreea Pavel [:apavel]]

https://hg.mozilla.org/mozilla-central/rev/50495a5da9f6
https://hg.mozilla.org/mozilla-central/rev/c28c538fca0a