sysconfig confused by relative paths

[sbt]

BPO	15364
Nosy	@freddrake, @birkenfeld, @ronaldoussoren, @benjaminp, @tarekziade, @ned-deily, @merwok, @cjerdonek
Files	sysconf.patch sysconf.patch sysconf.patch sysconf.patch sysconf.patch

^{Note: these values reflect the state of the issue at the time it was migrated and might not reflect the current state.}

Show more details

GitHub fields:

assignee = None
closed_at = <Date 2012-08-14.00:01:25.426>
created_at = <Date 2012-07-16.10:01:38.920>
labels = ['type-bug']
title = 'sysconfig confused by relative paths'
updated_at = <Date 2012-08-14.00:01:25.426>
user = 'https://bugs.python.org/sbt'

bugs.python.org fields:

activity = <Date 2012-08-14.00:01:25.426>
actor = 'sbt'
assignee = 'none'
closed = True
closed_date = <Date 2012-08-14.00:01:25.426>
closer = 'sbt'
components = []
creation = <Date 2012-07-16.10:01:38.920>
creator = 'sbt'
dependencies = []
files = ['26407', '26417', '26422', '26426', '26427']
hgrepos = []
issue_num = 15364
keywords = ['patch']
message_count = 27.0
messages = ['165581', '165620', '165628', '165632', '165633', '165634', '165654', '165664', '165720', '165729', '165735', '165737', '165738', '165739', '165761', '165762', '165763', '165844', '166481', '166506', '166522', '166525', '166553', '166566', '166569', '166628', '166753']
nosy_count = 10.0
nosy_names = ['fdrake', 'georg.brandl', 'ronaldoussoren', 'benjamin.peterson', 'tarek', 'ned.deily', 'eric.araujo', 'chris.jerdonek', 'python-dev', 'sbt']
pr_nums = []
priority = 'normal'
resolution = 'fixed'
stage = 'resolved'
status = 'closed'
superseder = None
type = 'behavior'
url = 'https://bugs.python.org/issue15364'
versions = ['Python 3.4']

[sbt]

With unix, and a source build, sysconfig.get_config_var('srcdir') and sysconfig.get_path('include') misbehave:

  user@mint-vm ~/Repos/cpython $ cd /
  user@mint-vm / $ ~/Repos/cpython/python 
  Python 3.3.0b1 (default:671894ae19a2, Jul 16 2012, 10:43:27) 
  [GCC 4.5.2] on linux
  Type "help", "copyright", "credits" or "license" for more information.
  >>> import sysconfig
  >>> sysconfig.get_config_var('srcdir')
  '/'
  >>> sysconfig.get_path('include')
  '//Include'

The problem seems to be the else clause of

        if 'srcdir' not in _CONFIG_VARS:
            _CONFIG_VARS['srcdir'] = _PROJECT_BASE
        else:
            _CONFIG_VARS['srcdir'] = _safe_realpath(_CONFIG_VARS['srcdir'])

The else clause (in slightly modified form) was introduced in 356d0ea8ea34, and it turns a relative path into an absolute path, using the current working directory as a base.

For me, simply removing the line allows the absolute path to be calculated correctly a few lines later on.

I don't know what problem 356d0ea8ea34 was intended to fix.

[ronaldoussoren]

I don't recall what the issue was the resulted in the check-in that you mention. Sadly enough it is not-trivial to find that check-in I mention due to the migration from Subversion to Mercurial.

How was python itself configured (configure command line)? What OS are you running on? Given the shell prompt I'd say Mint Linux. And finally, is '~/Repos/cpython/python' a checkout or installed location (that is, did you run 'make install')?

Based on code inspection I'd say that your change is correct, and my change only works when running from the build directory.

[sbt]

I don't recall what the issue was the resulted in the check-in that you
mention.

I think it was http://bugs.python.org/issue8577. The issue was about having srcdir != builddir. The initial patch caused a test failure, and 356d0ea8ea34 fixed (or at least silenced) the failure.

How was python itself configured (configure command line)?

I tried it with both

./configure
make

and

mdkir release
cd release
../configure
make

It fails with both.

What OS are you running on? Given the shell prompt I'd say Mint Linux.

Yes (in a VM).

And finally, is '~/Repos/cpython/python' a checkout or installed
location (that is, did you run 'make install')?

It is a checkout.

The argument passed to _safe_realpath() (either "." or ".." in my case) should be interpreted relative to the directory containing the Makefile. But os.path.realpath() assumes that its argument is either absolute or relative to os.getcwd(). It therefore returns the wrong absolute path.

[merwok]

It is actually simple to find the revision: http://hg.python.org/lookup/r81999 :)

[merwok]

I have to add that I’m quite confused by srcdir vs. projectbase. There are a handful of open bugs related to sysconfig and built but uninstalled Pythons, and many commits changing code to use srcdir or projectbase after empirical testing or buildbot failures. :/

[ronaldoussoren]

srcdir vs. project base is quite easy: srcdir is the directory containing the source files, the project base is where you ran configure. These are the same if you run configure in the root of a checkout, but don't have to be, for example when you do:

$ mkdir buildroot
$ cd buildroot
$ ../configure # ...

Now de base directory is the 'buildroot' directory, while srcdir is its parent directory.

[cjerdonek]

bpo-15322 is a recently filed bug regarding srcdir: "sysconfig.get_config_var('srcdir') returns unexpected value"

[sbt]

In the attached patch _safe_realpath() is only called after calculating the absolute path for srcdir.

[sbt]

Updated patch which clarifies that for an installed python on a posix system, get_config_var('srcdir') == get_path('stdlib').

Eg, if sys.executable == '/usr/local/bin/python3.3' then get_config_var('srcdir') == '/usr/local/lib/python3.3'.

This is a little arbitrary, but seems as sensible a value as any other. (Or maybe it would be better to have get_config_var('srcdir') == None instead.)

[ned-deily]

This is a little arbitrary, but seems as sensible a value as any other.
(Or maybe it would be better to have get_config_var('srcdir') == None
instead.)

'srcdir' is problematic for any installed build. Once "make install" has been performed, there is no obligation to keep the original source and build dirs around anymore. For Pythons installed on other machines via a binary installer, like the python.org OS X ones, some of the paths returned by sysconfig are not meaningful on the installed machine. For an OS X framework build, the Makefile and friends are copied into a 'config' directory that is created within the installed framework. So, with the patch, the contents of that directory (returned by the patched 'srcdir' value) is:
['Makefile',
'Setup',
'Setup.config',
'Setup.local',
'config.c',
'config.c.in',
'install-sh',
'libpython3.3.a',
'libpython3.3.dylib',
'makesetup',
'python.o']

Is that a meaningful surrogate for the build 'srcdir'?

Beyond that, I don't understand why the patch behavior depends on whether the srcdir is an absolute path or not. I often use absolute paths to configure a build. If not in a build directory, 'srcdir' should always return either the proposed value based on what get_makefile_filename() returns or it should return None. I don't have a strong opinion at the moment about which. One factor should be an inspection and running of all of the tests. A quick grep of Lib/ didn't find many references to 'srcdir'.

One other point: distutils still has its own copy of sysconfig. Strong consideration should be given to making a similar change there.

[sbt]

Beyond that, I don't understand why the patch behavior depends on
whether the srcdir is an absolute path or not.

The os.path.isabs() test is actually redundant since os.path.join(base, srcdir) == srcdir if srcdir is an absolute path. I only added that test to be explicit about what happens if the original value is an absolute path.

I often use absolute paths to configure a build. If not in a build
directory, 'srcdir' should always return either the proposed value
based on what get_makefile_filename() returns or it should return
None.

Removing the isabs() test would not change the calculated value as I indicated above.

Using None would be the sanest option, but I don't know if any current code depends on the value being a string.

One other point: distutils still has its own copy of sysconfig.
Strong consideration should be given to making a similar change there.

For a source build, distutils.sysconfig.get_config_var('srcdir') gives the right answer as an absolute path, *except* when the current working directory contains sys.executable. I think it should always return an absolute path. The fact that currently it does not probably explains the "mysteriously" comment below from distutils/test/support.py:

def _get_xxmodule_path():
    srcdir = sysconfig.get_config_var('srcdir')
    candidates = [
        # use installed copy if available
        os.path.join(os.path.dirname(__file__), 'xxmodule.c'),
        # otherwise try using copy from build directory
        os.path.join(srcdir, 'Modules', 'xxmodule.c'),
        # srcdir mysteriously can be $srcdir/Lib/distutils/tests when
        # this file is run from its parent directory, so walk up the
        # tree to find the real srcdir
        os.path.join(srcdir, '..', '..', '..', 'Modules', 'xxmodule.c'),
    ]
    for path in candidates:
        if os.path.exists(path):
            return path

BTW, I was wrong in my earlier message when I claimed that srcdir == get_path('stdlib') for an installed python. That is only true if the relative srcdir is '..'. The attached patch removes that check from the unit tests.

[ned-deily]

Upon further consideration, while it sounds appealing, perhaps returning None is not the better choice. As far as I know, up until now all config vars are either of type str or type int. Adding None to the mix might be asking for trouble.

With no patch, running from an installed framework build:
>>> sys.executable
'/py/dev/default/b10.7_t10.7_x4.3_cclang_d/fw/root/bin/python3.3'
>>> import sysconfig as sc, distutils.sysconfig as dsc
>>> sc.is_python_build()
False
>>> sc.get_config_var('srcdir')
'/py/dev/default/b10.7_t10.7_x4.3_cclang_d'
>>> dsc.get_config_var('srcdir')
'.'

With the latest patch, as above:
>>> sys.executable
'/py/dev/default/b10.7_t10.7_x4.3_cclang_d/fw/root/bin/python3.3'
>>> import sysconfig as sc, distutils.sysconfig as dsc
>>> sc.is_python_build()
False
>>> sc.get_config_var('srcdir')
'/py/dev/default/b10.7_t10.7_x4.3_cclang_d/fw/root/Library/Frameworks/pytest_10_7.framework/Versions/3.3/lib/python3.3/config-3.3dm'
>>> dsc.get_config_var('srcdir')
'.'

The patched results looks OK except, of course, for the discrepancy with distutils.sysconfig. I think we should fix that and get this all into 3.3.0. Otherwise we'll be stuck with the current broken behavior until 3.4 at the earliest.

One nit comment on the currrent patch: s/"will not effect it"/"will not affect it" (one of those gotchas in English).

[ned-deily]

I should have noted that, in the above tests, distutils.sysconfig.get_makefile_filename() is already doing the right thing:

>>> dsc.get_makefile_filename()
'/py/dev/default/b10.7_t10.7_x4.3_cclang_d/fw/root/Library/Frameworks/pytest_10_7.framework/Versions/3.3/lib/python3.3/config-3.3dm/Makefile'
[70312 refs]
>>> sc.get_makefile_filename() == dsc.get_makefile_filename()
True

[merwok]

Agree on fixing this for 3.3, if it’s not possible to backport to all stable branches. Will review.

[sbt]

Here is an updated patch which also modifies distutils.sysconfig.

For "installed" pythons on posix systems it makes

get_config_var('srcdir') == os.path.dirname(get_makefile_filename())

(The older patches would give the same result only if the relative srcdir was '.')

[sbt]

Forgot to remove some unnecessary code in patch.

[ronaldoussoren]

I'd make get_config_var('srcdir') to be None for installed systems, because the source tree is not available there.

The advantage of making the value None is that this is easy to test for and would quickly break path manipulation code that tries to construct a path to a specific file in the source tree.

Making the value to be the name of the directory containing Makefile would still break code that tries to access files relative to the source tree, but could that breakage could be harder to track.

BTW. sysconfig really needs a better description of which variables are supposed to be available, it currently exports every definition in the Makefile and pyconfig.h, and not all of those definitions are usable outside of CPython's build process.

[sbt]

I'd make get_config_var('srcdir') to be None for installed systems,
because the source tree is not available there.

While playing with a version of the patch which returns None for non-source builds, I found that distutils.support._get_xxmodule_path() depends on get_config_var('srcdir') always returning a string:

def _get_xxmodule_path():
    srcdir = sysconfig.get_config_var('srcdir')
    candidates = [
        os.path.join(os.path.dirname(__file__), 'xxmodule.c'),
        os.path.join(srcdir, 'Modules', 'xxmodule.c'),
        os.path.join(srcdir, '..', '..', '..', 'Modules', 'xxmodule.c'),
    ]
    for path in candidates:
        if os.path.exists(path):
            return path

It is easy enough to modify _get_xxmodule_path() to check for None. But this does suggest that returning None might break some currently working 3rd party code.

[sbt]

Any objection if I commit the last patch before the next beta? This is the one which on installed Pythons have

get_config_var('srcdir') == os.path.dirname(get_makefile_filename())

on posix systems.

[ned-deily]

LGTM

[merwok]

Does get_config_var('srcdir') always return a string or sometimes None?

[sbt]

Does get_config_var('srcdir') always return a string or sometimes None?

Always a string.

distutils.support._get_xxmodule_path() is one place which (currently) would throw an exception if it returned None.

[python-dev]

New changeset d2f448dbc30d by Richard Oudkerk in branch 'default':
Issue bpo-15364: Fix sysconfig.get_config_var('srcdir') to be an absolute path.
http://hg.python.org/cpython/rev/d2f448dbc30d

[merwok]

Okay, so LGTM. Let’s ask the RMs if this is a new behavior or a bugfix.

[ronaldoussoren]

The current patch should be fine, although it is IMHO not the long-term solution. For installed python's get_config_var('srcdir') still lies, but now at least returns slightly more sane path.

As I noted before the real problem is that it is not clear which config_vars are usable in which situations. One example is 'srcdir': this value is only useful in the build tree and there currently is no clean way to signal that you are retrieving a value that is not useful. Another example is 'RUNSHARED', that's also only useable when building python.

I'm not sure what the right solution for the larger issue is. It might be good enough to document generally useful config_vars and warn that other's might exist but might not be safe to use, on the other hand it might be better to explicitly reduce the set of config_vars to something sane (and document which those are and when they can be used)

[python-dev]

New changeset c2618b916081 by Ned Deily in branch 'default':
Issue bpo-15364: Fix test_srcdir for the installed case.
http://hg.python.org/cpython/rev/c2618b916081

[sbt]

One example is 'srcdir': this value is only useful in the build tree and
there currently is no clean way to signal that you are retrieving a
value that is not useful.

In the particular case of 'srcdir', sysconfig.is_python_build() tells you whether the value is useful.

Concerns about sysconfig often returning values which are meaningless for the current configuration can be discussed in another issue.

Unless there are objections I will close.