Closed Bug 1727819 Opened 3 years ago Closed 3 years ago

Guard against risky virtualenv=>virtualenv activations

Categories

(Firefox Build System :: Mach Core, task)

task

Tracking

(firefox98 affected)

RESOLVED FIXED
98 Branch
Tracking Status
firefox98 --- affected

People

(Reporter: mhentges, Assigned: mhentges)

References

(Blocks 1 open bug)

Details

Attachments

(2 files)

As we're going to have per-command virtualenvs, and we can't guarantee compatibility between commands, we're going to have to remove mach_context.commands.dispatch(...) support.

Assignee: nobody → mhentges
Status: NEW → ASSIGNED

Hmm, there's a lot of cases where we're taking advantage of running subsequent commands in the same process by passing pre-parsed values directly into the called command with **kwargs.
There's cases like ./mach try that dynamically calls ./mach try again/fuzzy/auto/etc [*args], or ./mach hazards compile that ferries arguments to ./mach build.

We have couple options of solving this:

  • Add infrastructure to pass along "pre-parsed" arguments to subprocesses (assuming that the parsed variants can all be serialized).
  • Add infrastructure to convert parsed arguments back into --command --line --flags.
  • Or, the easiest solution: continue to allow .dispatch() to exist, but validate that the virtualenv_name isn't changing between the calling and callee commands.
Summary: Don't allow dispatching new Mach commands in existing process → Don't allow dispatching new Mach command with different virtualenv

Even better, tweaked this a little further to a better abstraction level: forget dispatching, just guard against activating a command virtualenv from another command virtualenv.

Summary: Don't allow dispatching new Mach command with different virtualenv → Guard against risky virtualenv=>virtualenv activations

Sphinx already runs within the ./mach doc command, which has the
docs virtualenv activated. Activating another virtualenv within a
Sphinx python file is redundant.

Pushed by mhentges@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/85c1380a7969 Remove redundant virtualenv activation for docs r=ahal
Status: ASSIGNED → RESOLVED
Closed: 3 years ago
Resolution: --- → FIXED
Target Milestone: --- → 98 Branch
Status: RESOLVED → REOPENED
Keywords: leave-open
Resolution: FIXED → ---

The Mach site is asserted to be compatible with all command sites, but
command sites are deliberately //not// asserted to be compatible with
each other - they're //supposed// to have the flexibility of being
able to be incompatible.

Accordingly, let's fail loudly if code tries to activate from one
command site to another.

This required updating the mozproxy tests, who would deliberately
activate the common site so they could call a mozproxy entry point
script. These tests were fixed by instead invoking mozproxy as a
module (-m) of the current python-test site.

Attachment #9268735 - Attachment description: WIP: Bug 1727819: Guard against command site -> another site activations → Bug 1727819: Guard against command site -> another site activations
Pushed by mhentges@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/349b88dc2b26 Guard against command site -> another site activations r=ahal
Status: REOPENED → RESOLVED
Closed: 3 years ago3 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: