Open Bug 1580625 Opened 6 years ago Updated 2 years ago

`mach mozregression` should install mozregression into its own dedicated virtualenv

Categories

(Testing :: mozregression, enhancement, P3)

Version 3
enhancement

Tracking

(Not tracked)

People

(Reporter: Gijs, Unassigned)

References

Details

See bug 1568258 comment 9 (and the associated attachment for a log) and bug 1568258 comment 10.

:wlach, are you still the right person to ask about mozregression?

Flags: needinfo?(wlachance)
Summary: `mach mozregression` on mac/windows can break build system by pulling in a different version of `mozfile` into the same virtualenv → `mach mozregression` on mac/windows can break build system ("ImportError: cannot import name which" caused by `from mozfile import which`) by pulling in a different version of `mozfile` into the same virtualenv

(In reply to :Gijs (he/him) from comment #0)

See bug 1568258 comment 9 (and the associated attachment for a log) and bug 1568258 comment 10.

:wlach, are you still the right person to ask about mozregression?

Hi, yes about mozregression in general, although I can only respond to issues on a "best effort" basis. In this case it looks like the issue is with the mach command, which was written by a contributor who is no longer with us (I can't remember who reviewed it, it wasn't me). I'd be happy to review any patches to mozregression itself needed to fix this, but probably can't help otherwise.

Flags: needinfo?(wlachance)

(In reply to William Lachance (:wlach) (use needinfo!) from comment #1)

(In reply to :Gijs (he/him) from comment #0)

See bug 1568258 comment 9 (and the associated attachment for a log) and bug 1568258 comment 10.

:wlach, are you still the right person to ask about mozregression?

Hi, yes about mozregression in general, although I can only respond to issues on a "best effort" basis. In this case it looks like the issue is with the mach command, which was written by a contributor who is no longer with us (I can't remember who reviewed it, it wasn't me). I'd be happy to review any patches to mozregression itself needed to fix this, but probably can't help otherwise.

I or someone else may be able to write a patch, but atm I'm not sure what the right solution would be - use a different virtualenv for mozregression? Just upgrade the mozfile requirement for mozregression and forget about this issue for a while until some other package conflict causes issues? Or make mozregression rely on in-tree stuff explicitly, or via running the initial "normal" virtualenv setup and then only pulling in any additional packages, verifying that this additional list doesn't overlap with the "normal" virtualenv requirements? Or something else? :-)

Andrew/William, hoping you can provide some guidance as to the best way forward... :-)

Flags: needinfo?(wlachance)
Flags: needinfo?(ahal)

Mozregression actually lives out-of-tree (the mach command bootstraps it if not detected). I'm assuming there are use cases where one might want to run it outside of mozilla-central, so we won't want to make it depend on things only in-tree (if that assumption is wrong, then we should absolutely move it in-tree and be done with it).

Which means we should install it in its own virtualenv. We have a mechanism to do this already, used by things like mach doc. Basically we can create a Pipfile in tools/mozregression similar to the doc one:
https://searchfox.org/mozilla-central/source/tools/docs/Pipfile

Then cd to that dir and run pipenv lock (after pip install pipenv) to generate the lockfile. Then commit both files and change the mach command to use activate_pipenv instead of activate_virtualenv. Again, can use mach doc as an example.

A side benefit of this is we'll pin everyone to the same version of mozregression which will help with reproducibility and prevent upstream regressions from causing bustage which we aren't able to backout.

Flags: needinfo?(wlachance)
Flags: needinfo?(ahal)

(In reply to Andrew Halberstadt [:ahal] from comment #3)

Mozregression actually lives out-of-tree (the mach command bootstraps it if not detected). I'm assuming there are use cases where one might want to run it outside of mozilla-central, so we won't want to make it depend on things only in-tree (if that assumption is wrong, then we should absolutely move it in-tree and be done with it).

Correct, I suppose we could bring it in tree like we did with mozbase but I don't think we want to impose mozilla-central's rigorous review/commit policy on mozregression.

Which means we should install it in its own virtualenv. We have a mechanism to do this already, used by things like mach doc. Basically we can create a Pipfile in tools/mozregression similar to the doc one:
https://searchfox.org/mozilla-central/source/tools/docs/Pipfile

Then cd to that dir and run pipenv lock (after pip install pipenv) to generate the lockfile. Then commit both files and change the mach command to use activate_pipenv instead of activate_virtualenv. Again, can use mach doc as an example.

A side benefit of this is we'll pin everyone to the same version of mozregression which will help with reproducibility and prevent upstream regressions from causing bustage which we aren't able to backout.

Hmm, I think this might cause more problems than it solves. We release new versions of mozregression reasonably frequently to fix bugs (version 3 just went out today with python 3 support), and I don't think we really want to have to update the tree every time we do.

I don't know how hard it would be to implement, but it seems to me like the ideal solution would be:

  1. ./mach mozregression runs in its own virtualenv so as not to interfere with anything else
  2. ./mach mozregression auto-upgrades mozregression to the latest version if it's out-of-date (including dependencies)

Is that possible?

(In reply to William Lachance (:wlach) (use needinfo!) from comment #4)

Hmm, I think this might cause more problems than it solves. We release new versions of mozregression reasonably frequently to fix bugs (version 3 just went out today with python 3 support), and I don't think we really want to have to update the tree every time we do.

I don't really agree with this sentiment. From my point of view A) needing to land a commit in central isn't a terrible burden, and B) the risk of introducing new regressions which can't be traced to a central commit outweighs this burden. Plus upgrading for upgradings sake isn't a compelling feature to me (if it aint broke don't fix it).

That said I don't have particularly strong opinions on this particular module, and as long as I'm not the maintainer am happy to defer to your preferences here.

I don't know how hard it would be to implement, but it seems to me like the ideal solution would be:

  1. ./mach mozregression runs in its own virtualenv so as not to interfere with anything else
  2. ./mach mozregression auto-upgrades mozregression to the latest version if it's out-of-date (including dependencies)

Is that possible?

I don't believe this is currently possible (when creating new virtualenvs we enforce version pinning with hashes). We could implement the ability to do this, but as stated above I'd tend to push back against opening this door.

(In reply to Andrew Halberstadt [:ahal] from comment #5)

(In reply to William Lachance (:wlach) (use needinfo!) from comment #4)

Hmm, I think this might cause more problems than it solves. We release new versions of mozregression reasonably frequently to fix bugs (version 3 just went out today with python 3 support), and I don't think we really want to have to update the tree every time we do.

I don't really agree with this sentiment. From my point of view A) needing to land a commit in central isn't a terrible burden, and B) the risk of introducing new regressions which can't be traced to a central commit outweighs this burden. Plus upgrading for upgradings sake isn't a compelling feature to me (if it aint broke don't fix it).

Most of what lands in mozregression are needed features to keep up with changes in CI, like shippable builds. "just leaving it alone" is a recipe for breakage.

I'm not sure what to suggest, it doesn't sound like either of us really wants to commit to the work required to keep this running. In the absence of someone who wants to step in and volunteer, it might be best to just take this command out.

Removing it sounds like a worse option that resolving this bug WONTFIX and continuing on as we have been :). Ftr, by "just leave it alone" I was excluding bustage fixes and was thinking more about new features. Though sounds like there aren't really any new features these days (imo mozregression is really useful and someone should be working on it at least part time.. I understand you don't have time for this and neither do I).

Anyway I won't stand in the way of auto-updating in its own virtualenv if that improves the status quo. My preference would still be A) pin it (either via vendoring/moving into tree or versioning), B) have unittests to help catch the case when in-tree changes break things. If the sticking point here is the commit needed to update mozregression after publishing it to pypi, I will gladly volunteer to handle those :).

(In reply to Andrew Halberstadt [:ahal] from comment #7)

If the sticking point here is the commit needed to update mozregression after publishing it to pypi, I will gladly volunteer to handle those :).

FWIW, we have bots that auto-update e.g. the blocklist and various other externally sourced things, and I expect we could use those here, too, if we wanted to.

(In reply to :Gijs (he/him) from comment #8)

(In reply to Andrew Halberstadt [:ahal] from comment #7)

If the sticking point here is the commit needed to update mozregression after publishing it to pypi, I will gladly volunteer to handle those :).

FWIW, we have bots that auto-update e.g. the blocklist and various other externally sourced things, and I expect we could use those here, too, if we wanted to.

That sounds ideal!

:ahal and I talked about this offline, we think this bug needs someone who can provide it with some dedicated attention (both now and in the future), but should be left open for now.

For now, I've released a new version of mozfile (bug 1582248) which should workaround the in-tree breakage for now (by syncing up the externally released mozfile with what was in mozilla-central).

For now this issue should be resolved, so we can remove from mach-busted (though it'll come up again in the future).

Also since the description of the original problem was in the title, restating it here so it's not lost:

The original failure happened because mozregression depends on mozfile, so when we install it into the build system virtualenv it pulls the latest version of mozfile from pypi and overwrites the in-tree version (which will be more recent).

Summary: `mach mozregression` on mac/windows can break build system ("ImportError: cannot import name which" caused by `from mozfile import which`) by pulling in a different version of `mozfile` into the same virtualenv → `mach mozregression` should install mozregression into its own dedicated virtualenv

The priority flag is not set for this bug.
:wlach, could you have a look please?

For more information, please visit auto_nag documentation.

Flags: needinfo?(wlachance)
No longer blocks: mach-busted
Flags: needinfo?(wlachance)
Priority: -- → P3
No longer blocks: 1568258
Severity: normal → --
Type: defect → enhancement
See Also: → 1582248
See Also: → 1678037
You need to log in before you can comment on or make changes to this bug.