build virtualenv uses system site packages and probably shouldn't

RESOLVED FIXED in Firefox 26, Firefox OS v1.2

Status

RESOLVED FIXED
5 years ago
6 months ago

People

(Reporter: k0scist, Assigned: gps)

Tracking

unspecified
mozilla27
Dependency tree / graph

Firefox Tracking Flags

(firefox26 fixed, firefox27 fixed, firefox-esr24 unaffected, b2g-v1.2 fixed)

Details

Attachments

(1 attachment)

(Reporter)

Description

5 years ago
http://mxr.mozilla.org/mozilla-central/source/build/virtualenv/populate_virtualenv.py#116

We set --system-site-packages which puts the system packages first in
sys.path . This causes bugs like bug 907476 and otherwise makes
behaviour less encapsulated and predictable.

We should probably not set this.  If we do set this, the reason should
at least be documented IMHO.
(Reporter)

Updated

5 years ago
Blocks: 907476
(Assignee)

Comment 1

5 years ago
This was added in bug 758739.
Depends on: 758739
Yeah. I think the evident ways this can break things are argument enough to remove --with-system-ply.
I'd rather have a different fix for bug 758739 than removing --with-system-ply
(Assignee)

Comment 4

5 years ago
Let's detect where ply.py is and add it to the virtualenv's sys.path somehow?
(Reporter)

Updated

5 years ago
See Also: → bug 903962
(Assignee)

Updated

5 years ago
Summary: build virtualenv uses system site packages and probably shouldnt → build virtualenv uses system site packages and probably shouldn't
(Assignee)

Comment 5

5 years ago
I think --system-ply being the thing that's forcing us to use system site-packages in our virtualenv is silly. We /really/ need to stop the virtualenv from using the system's site-packages. I don't think there's much room for debate.

The way I see it, we have two options:

1) Kill --system-ply entirely.
2) Provide a backdoor for the configuration to inject its own paths into the virtualenv. e.g. --python-include-path=/path/to/some/other/path

I would prefer we go with #1 so the virtualenv is reasonably guaranteed to be isolated from the host system. But, I'd settle for #2.

AFAICT Mike Hommey is the only person who wants --system-ply. He possibly represents the larger Debian/packaging population for downstream distros.

Mike: what are your actual requirements here?
Flags: needinfo?(mh+mozilla)
I don't want ply to be shipped in the sdk shipped in the corresponding debian package. While I could just strip those file, i also want to ensure the sdk is not broken as a result.
Flags: needinfo?(mh+mozilla)
(Assignee)

Comment 7

5 years ago
(In reply to Mike Hommey [:glandium] from comment #6)
> I don't want ply to be shipped in the sdk shipped in the corresponding
> debian package. While I could just strip those file, i also want to ensure
> the sdk is not broken as a result.

I'm still trying to understand why we (upstream Mozilla) should care. This seems like a trivial downstream action (only wanted by Debian)? It's causing us hardship, so I'm inclined to punt the problem onto the people who have created it for themselves. I'm being overly generous in giving you the opportunity to convince me otherwise. I'm just not hearing anything...
(Assignee)

Comment 8

5 years ago
Created attachment 805687 [details] [diff] [review]
Create system isolated virtualenv, remove --system-ply support

This patch removes --system-ply from configure and removes system site
packages from the virtualenv.

Patch itself is trivial. Hard part is determining whether to unsupport
--system-ply people.
Attachment #805687 - Flags: review?(ted)
(Assignee)

Updated

5 years ago
Assignee: nobody → gps
Attachment #805687 - Flags: review?(ted) → review+
(Assignee)

Comment 9

5 years ago
https://hg.mozilla.org/projects/build-system/rev/f18eae7c3b27
Status: NEW → ASSIGNED
Flags: in-testsuite-
(Assignee)

Comment 10

5 years ago
https://hg.mozilla.org/mozilla-central/rev/f18eae7c3b27
Status: ASSIGNED → RESOLVED
Last Resolved: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla27
https://hg.mozilla.org/releases/mozilla-beta/rev/0453bfbb7816
status-firefox26: --- → fixed
status-firefox27: --- → fixed
status-firefox-esr24: --- → unaffected
status-b2g-v1.2: --- → fixed

Updated

6 months ago
Product: Core → Firefox Build System
You need to log in before you can comment on or make changes to this bug.