Title: distutils.cfg Can Break venv
Type: behavior Stage: patch review
Components: Distutils Versions: Python 3.3, Python 3.4
Status: closed Resolution: fixed
Dependencies: Superseder:
Assigned To: eric.araujo Nosy List: carljm, eric.araujo, eruart, georg.brandl, larry, nicksloan, python-dev, tarek, vinay.sajip
Priority: release blocker Keywords: patch

Created on 2013-04-14 18:10 by nicksloan, last changed 2013-05-16 17:57 by georg.brandl. This issue is now closed.

File name Uploaded Description Edit
distutilsvenv.patch nicksloan, 2013-04-15 16:05 distutils/ patch for venv review
distutilsvenv.patch nicksloan, 2013-04-15 18:16 Updated patch. review
distutilsvenv.patch nicksloan, 2013-04-16 18:31 review
distutilsvenv.patch nicksloan, 2013-04-16 18:57 review
distutilsvenv.patch nicksloan, 2013-04-19 14:32 review
distutilsvenv.patch nicksloan, 2013-04-19 16:54 review
distutilsvenv.patch nicksloan, 2013-04-22 14:04 review
Messages (24)
msg186942 - (view) Author: Nick Sloan (nicksloan) * Date: 2013-04-14 18:10
When distutils.cfg defines an install-lib, it will be used within virtual environments created using venv as well, which makes it impossible to install things with or similar.

Steps to reproduce:

Create a distutils.cfg file and set a value for install-lib.

Create a virtual environment: $pyvenv myvenv

Activate that environment: $source myvenv/bin/activate

Attempt to install something: $python

It will attempt to install in the lib-install directory, and should fail with an error that the directory is not on the python path. This issue affects python3 from the mac homebrew project, which bundles a distutil.cfg, thus breaking pyvenv by default.
msg186943 - (view) Author: Éric Araujo (eric.araujo) * (Python committer) Date: 2013-04-14 18:41
There is a --no-user-cfg option to disable reading ~/.pydistutils.cfg, but I don’t recall an option to ignore the global config.  How does virtualenv avoid this problem, if it does?
msg186945 - (view) Author: Nick Sloan (nicksloan) * Date: 2013-04-14 19:01
It looks like virtualenv includes an empty distutils.cfg at myvirtualenv/lib/python3.3/distutils/distutils.cfg. Most of myvirtualenv/lib/python3.3 is just symlinked to the base python version, but myvirtualenv/lib/python3.3/distutils/ is not, and I bet this is exactly why.
msg186973 - (view) Author: Vinay Sajip (vinay.sajip) * (Python committer) Date: 2013-04-15 11:16
This looks to me as if it will need a patch in distutils. Unlike virtualenv, which contains a patched copy of distutils (and hence allows having a .cfg adjacent to it), pyvenv does not create patched modules in the venv. It does not make sense to change this behaviour.

The correct solution would appear to be for distutils to ignore certain configuration options (install-lib, but also equivalent options for headers and scripts) when running in a venv.
msg186978 - (view) Author: Nick Sloan (nicksloan) * Date: 2013-04-15 12:22
That's along the lines of what I've been thinking as I dig into this. I'd love to take a stab at a patch for this if no one else has done so already.
msg187004 - (view) Author: Nick Sloan (nicksloan) * Date: 2013-04-15 16:05
Here is a patch that seems to fix the problem. It simply short-circuits distutils options that change directories. This is my first python patch ever, so I'm eager for comments. Is this the right approach?
msg187010 - (view) Author: Nick Sloan (nicksloan) * Date: 2013-04-15 18:16
On second thought, there is probably no good reason to ignore the build-* settings. Here is an updated patch.
msg187012 - (view) Author: Vinay Sajip (vinay.sajip) * (Python committer) Date: 2013-04-15 18:38
> On second thought, there is probably no good reason to ignore the build-* settings.

I was just about to mention this. I'm not an expert on distutils internals and whether this is the best way to accomplish what's needed, but the approach seems reasonable to me. Why the empty list and the extend, though? Why not something like the following?

if sys.prefix == sys.base_prefix:
    # not in venv
    ignore_options = []
    # in venv
    ignore_options = ['install-base', ...]

My other comments on the patch are generic, i.e. you need to look at the tests and docs, too.

Anyway, thanks for the effort you've spent to come up with a patch. Can I suggest that you sign a PSF contributor form to license your work to the PSF?
msg187018 - (view) Author: Nick Sloan (nicksloan) * Date: 2013-04-15 19:23
My thought was that perhaps there will be other circumstances where we may want to ignore options in the future. The idea was that by providing an ignore_options list that can be extended, multiple conditions with different sets of options can be stacked together easily. I was thinking a set might actually be an even better choice than a list.

Would love feedback on this approach. Am I over-designing for an unlikely case? I'm happy to use Vinay's suggestion if we don't think it is worth it to consider possible future conflicts with distutils.cfg options. If there is any reason a set would be worse to use, let me know, otherwise my next patch will use a set rather than a list.

Another question: should I be checking which config file these come from, or ignoring all occurrences of these options? Seems like setup.cfg at the very least shouldn't have any restrictions. Possibly ~/.pydistutils.cfg too.

I'll review tests to see if this necessitates any changes, add a new test that covers this, and I'll make a note in the docs.
msg187021 - (view) Author: Éric Araujo (eric.araujo) * (Python committer) Date: 2013-04-15 19:32
I would ignore options from all config files, and do the simplest thing (i.e. not add the ignore_options attribute).
msg187111 - (view) Author: Nick Sloan (nicksloan) * Date: 2013-04-16 18:31
Here is an updated patch with documentation changes and a new test. 5 tests in distutils have errors. I have left those alone for now.
msg187112 - (view) Author: Nick Sloan (nicksloan) * Date: 2013-04-16 18:35
That is, errors that pre-existed my patch.
msg187113 - (view) Author: Nick Sloan (nicksloan) * Date: 2013-04-16 18:57
Whoops. One of the options I had in my list doesn't actually exist. Here is yet another update.
msg187296 - (view) Author: Nick Sloan (nicksloan) * Date: 2013-04-18 21:13
Any feedback on this latest patch?
msg187303 - (view) Author: Éric Araujo (eric.araujo) * (Python committer) Date: 2013-04-18 22:26
Thanks for the patch, I left comments on rietveld.
msg187363 - (view) Author: Nick Sloan (nicksloan) * Date: 2013-04-19 14:32
Responded to comments with an updated patch. Thanks for all the feedback, and sorry for the silly mistakes. Should have read up more thoroughly on the docs style guide and the terminology. Hopefully the latest patch is ready to go (or at least, nearly so).
msg187376 - (view) Author: Nick Sloan (nicksloan) * Date: 2013-04-19 16:54
Here is another update. It has come to my attention that I missed some options:

prefix, exec-prefix, home, user and root

These have been added, and the docs and test have been updated to reflect the change.
msg187386 - (view) Author: Éric Araujo (eric.araujo) * (Python committer) Date: 2013-04-19 21:51
Looks good, thanks!  I assume you also tested it manually?

I’ll take care of this for the next release.
msg187563 - (view) Author: Nick Sloan (nicksloan) * Date: 2013-04-22 14:04

Fixed the mention of packages, and made a frozenset of ignore_options. Think this thing is ready to go. I have tested it, and it does the trick. Thanks for helping me get this patch merged in.
msg188446 - (view) Author: Nick Sloan (nicksloan) * Date: 2013-05-05 15:51
Just checking to see if anything else is needed from me on this.
msg188546 - (view) Author: Éric Araujo (eric.araujo) * (Python committer) Date: 2013-05-06 14:37
No, patch is good to go, I just need to find some time to commit it.  If another core dev wants to do it sooner, please feel free to do so.
msg189011 - (view) Author: Roundup Robot (python-dev) (Python triager) Date: 2013-05-12 10:47
New changeset 6c5a3d194a10 by Georg Brandl in branch '3.3':
Closes issue #17732: ignore install-directory specific options in
msg189387 - (view) Author: Roundup Robot (python-dev) (Python triager) Date: 2013-05-16 17:03
New changeset d62f71bd2192 by Brian Curtin in branch '3.3':
Add Nick Sloan for his contribution to #17732
msg189393 - (view) Author: Georg Brandl (georg.brandl) * (Python committer) Date: 2013-05-16 17:57
Thanks for the attribution, that was definitely an oversight on my part.
Date User Action Args
2013-05-16 17:57:20georg.brandlsetmessages: + msg189393
2013-05-16 17:03:53python-devsetmessages: + msg189387
2013-05-12 11:23:17georg.brandlsetstatus: open -> closed
resolution: fixed
2013-05-12 10:47:24python-devsetnosy: + python-dev
messages: + msg189011
2013-05-06 14:37:45eric.araujosetmessages: + msg188546
2013-05-05 15:51:55nicksloansetmessages: + msg188446
2013-04-22 14:04:35nicksloansetfiles: + distutilsvenv.patch

messages: + msg187563
2013-04-22 09:40:56eruartsetnosy: + eruart
2013-04-19 21:51:07eric.araujosetpriority: normal -> release blocker
nosy: + larry, georg.brandl
messages: + msg187386

2013-04-19 16:54:48nicksloansetfiles: + distutilsvenv.patch

messages: + msg187376
2013-04-19 14:32:56nicksloansetfiles: + distutilsvenv.patch

messages: + msg187363
2013-04-18 22:26:42eric.araujosetmessages: + msg187303
2013-04-18 21:13:06nicksloansetmessages: + msg187296
2013-04-16 18:57:01nicksloansetfiles: + distutilsvenv.patch

messages: + msg187113
2013-04-16 18:35:07nicksloansetmessages: + msg187112
2013-04-16 18:31:14nicksloansetfiles: + distutilsvenv.patch

messages: + msg187111
2013-04-15 19:32:32eric.araujosetassignee: vinay.sajip -> eric.araujo
messages: + msg187021
components: + Distutils, - Library (Lib)
2013-04-15 19:23:12nicksloansetmessages: + msg187018
2013-04-15 18:38:54vinay.sajipsetmessages: + msg187012
stage: patch review
2013-04-15 18:16:10nicksloansetfiles: + distutilsvenv.patch

messages: + msg187010
2013-04-15 16:05:11nicksloansetfiles: + distutilsvenv.patch
keywords: + patch
messages: + msg187004
2013-04-15 12:22:04nicksloansetmessages: + msg186978
2013-04-15 11:16:39vinay.sajipsetmessages: + msg186973
2013-04-14 19:01:10nicksloansetmessages: + msg186945
2013-04-14 18:41:56eric.araujosetversions: + Python 3.4
nosy: + carljm, vinay.sajip

messages: + msg186943

assignee: eric.araujo -> vinay.sajip
components: + Library (Lib), - Distutils
2013-04-14 18:10:32nicksloancreate