Remove duplicated mozbase modules from mozharness
Categories
(Release Engineering :: Applications: MozharnessCore, defect)
Tracking
(firefox68+ fixed)
People
(Reporter: ahal, Assigned: ahal)
References
Details
Attachments
(3 files, 1 obsolete file)
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•9 years ago
|
||
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•9 years ago
|
||
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•9 years ago
|
||
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•9 years ago
|
||
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.
Assignee | ||
Comment 5•9 years ago
|
||
(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•9 years ago
|
||
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•9 years ago
|
||
> 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
Assignee | ||
Comment 8•8 years ago
|
||
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
Assignee | ||
Comment 9•8 years ago
|
||
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•7 years ago
|
||
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.
Assignee | ||
Comment 11•5 years ago
|
||
This makes sure that the mozharness scripts have access to all the packages in
the build system virtualenv (namely mozbase).
Assignee | ||
Comment 12•5 years ago
|
||
Depends on D22184
Comment 13•5 years ago
|
||
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.
Assignee | ||
Comment 14•5 years ago
|
||
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•5 years ago
|
||
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•5 years ago
|
||
Assignee | ||
Updated•5 years ago
|
Updated•5 years ago
|
Comment 17•5 years ago
|
||
Pushed by ahalberstadt@mozilla.com: 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•5 years ago
|
||
bugherder |
Comment 19•5 years ago
•
|
||
[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•5 years ago
|
||
FYI, the entry point here is taskcluster/scripts/builder/build-l10n.sh, which was still using python2.7 directly.
Assignee | ||
Comment 21•5 years ago
|
||
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•5 years ago
|
||
Should we also try a beta simulation with this patch?
Assignee | ||
Comment 23•5 years ago
|
||
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•5 years ago
|
||
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.
Assignee | ||
Comment 25•5 years ago
•
|
||
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.
Assignee | ||
Comment 26•5 years ago
|
||
(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!
Assignee | ||
Comment 27•5 years ago
|
||
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.
Updated•5 years ago
|
Comment 28•5 years ago
|
||
Pushed by ahalberstadt@mozilla.com: 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•5 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/50495a5da9f6
https://hg.mozilla.org/mozilla-central/rev/c28c538fca0a
Comment 30•5 years ago
|
||
bugherder |
Description
•