Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

test_distutils fails on Windows in 2.6.5rc2 #52354

Closed
loewis mannequin opened this issue Mar 10, 2010 · 23 comments
Closed

test_distutils fails on Windows in 2.6.5rc2 #52354

loewis mannequin opened this issue Mar 10, 2010 · 23 comments
Assignees
Labels
release-blocker stdlib Python modules in the Lib dir type-bug An unexpected behavior, bug, or error

Comments

@loewis
Copy link
Mannequin

loewis mannequin commented Mar 10, 2010

BPO 8107
Nosy @loewis, @warsaw, @ronaldoussoren, @vstinner, @tarekziade, @ned-deily, @ezio-melotti

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 = 'https://github.com/loewis'
closed_at = <Date 2010-03-13.22:24:23.577>
created_at = <Date 2010-03-10.08:20:19.334>
labels = ['type-bug', 'library', 'release-blocker']
title = 'test_distutils fails on Windows in 2.6.5rc2'
updated_at = <Date 2010-03-13.22:34:38.927>
user = 'https://github.com/loewis'

bugs.python.org fields:

activity = <Date 2010-03-13.22:34:38.927>
actor = 'tarek'
assignee = 'loewis'
closed = True
closed_date = <Date 2010-03-13.22:24:23.577>
closer = 'barry'
components = ['Distutils']
creation = <Date 2010-03-10.08:20:19.334>
creator = 'loewis'
dependencies = []
files = []
hgrepos = []
issue_num = 8107
keywords = ['buildbot']
message_count = 23.0
messages = ['100779', '100781', '100811', '100812', '100813', '100815', '100816', '100826', '100833', '100834', '100835', '100837', '100858', '100872', '100951', '100952', '100955', '100959', '101013', '101014', '101015', '101026', '101029']
nosy_count = 7.0
nosy_names = ['loewis', 'barry', 'ronaldoussoren', 'vstinner', 'tarek', 'ned.deily', 'ezio.melotti']
pr_nums = []
priority = 'release blocker'
resolution = 'accepted'
stage = 'test needed'
status = 'closed'
superseder = None
type = 'behavior'
url = 'https://bugs.python.org/issue8107'
versions = ['Python 2.6']

@loewis
Copy link
Mannequin Author

loewis mannequin commented Mar 10, 2010

test_distutils fails like this:

test test_distutils crashed -- <type 'exceptions.TypeError'>: object of type 'NoneType' has no len()
Traceback (most recent call last):
  File "Lib\test\regrtest.py", line 560, in runtest_inner
    indirect_test()
  File "c:\Python26\lib\test\test_distutils.py", line 13, in test_main
    test.test_support.run_unittest(distutils.tests.test_suite())
  File "c:\Python26\lib\distutils\tests\__init__.py", line 30, in test_suite
    suite.addTest(module.test_suite())
  File "c:\Python26\lib\distutils\tests\test_build_ext.py", line 391, in test_suite
    src = _get_source_filename()
  File "c:\Python26\lib\distutils\tests\test_build_ext.py", line 22, in _get_sou
rce_filename
    return os.path.join(srcdir, 'Modules', 'xxmodule.c')
  File "c:\Python26\lib\ntpath.py", line 96, in join
    assert len(path) > 0
TypeError: object of type 'NoneType' has no len()

Not sure whether this is a regression from 2.6.4.

@loewis loewis mannequin assigned tarekziade Mar 10, 2010
@loewis loewis mannequin added the stdlib Python modules in the Lib dir label Mar 10, 2010
@ezio-melotti
Copy link
Member

This has been introduced in r77955, and affected all the 2.6 Windows buildbots (e.g. http://www.python.org/dev/buildbot/builders/x86%20XP-5%202.6/builds/9/steps/test/logs/stdio).
The problem is caused by the fact that "srcdir = sysconfig.get_config_var('srcdir')" returns None on Windows.

@ezio-melotti ezio-melotti added the type-bug An unexpected behavior, bug, or error label Mar 10, 2010
@warsaw
Copy link
Member

warsaw commented Mar 10, 2010

Um, this sucks because I think that that means it was introduced after 2.6.4 final. Since it's a regression, marking as a blocker for 2.6.5 final, but in order to stay a blocker it has to be a really critical bug and I'll want to review the fix before it goes it (to make sure it doesn't break anything else). Otherwise, I'll bump it down to deferred blocker and we can fix it for 2.6.6.

@vstinner
Copy link
Member

r77955 is a backport of r69304, commit made 12 months before the backport. It's related to issue bpo-4151.

@vstinner
Copy link
Member

The patch replaces sysconfig.project_base by sysconfig.get_config_var('srcdir'). You can maybe use sysconfig.project_base if sysconfig.get_config_var('srcdir') is None?

@tarekziade
Copy link
Mannequin

tarekziade mannequin commented Mar 10, 2010

This test just grabs xxmodule.c from Python to try out a compilation.

It's a lot of work just to try out a .c file in build_ext. I've already changed this in trunk by having a c file local to distutils tests, as a fallback in case the location cannot be found (depending on the execution context).

I can backport this to 2.6. It's pretty safe to do it since its just a change in the test, the distutils code is not changed. Barry let me know if you want me to do backport it in release26, and if yes, when I should do it.

@ned-deily
Copy link
Member

I'm guessing this falls into the same category as bpo-8102, that is, since the python tests are also installed and run on end-user systems by various installers, it is important that none of the tests depend on files from a python build directory or source tree. In other words, if a test needs a file, that file needs to be explicitly installed.

@warsaw
Copy link
Member

warsaw commented Mar 11, 2010

Tarek, I guess the fix is as haypo suggests? Just use sysconfig.project_base if sysconfig.get_config_var('srcdir') returns None?

If that's the only change, then please do fix this for 2.6.5 final. Thanks!

@tarekziade
Copy link
Mannequin

tarekziade mannequin commented Mar 11, 2010

No that won't work because the test tries to find a module that is only in the source dir. So not existing on python installations. The simplest thing to do is to skip the test if srcdir cannot be found.

I'll fix this asap

@tarekziade
Copy link
Mannequin

tarekziade mannequin commented Mar 11, 2010

Also, notice that srcdir can be broken on some platform. For instance, under Mac OS X Framework install:

>>> import distutils.sysconfig
>>> distutils.sysconfig.get_config_var('srcdir')
'/Volumes/Rivendell/Users/ronald/Projects/python/r264'

we get the path that was used by Ronald to compile the distribution :)

Working on it..

@ned-deily
Copy link
Member

It's not that "srcdir" is broken - that undoubtedly was the value of the source directory on the *build* machine. The problem is that "srcdir", (and likely some other build related config variables) has no meaning other than on the *build* machine at the time of the build. It (they) cannot in general be meaningfully used at execution time, particularly if the install takes place on a machine other than build machine.

@ronaldoussoren
Copy link
Contributor

IMHO stuffing xxmodule.c inside the distutils test tree (see msg100815) is the right solution because the python source tree might not be present during testing (such as when the user does a binary install and then runs the unittests to check if everything works).

Tarek: sysconfig.get_config_var('srcdir') returns the value of 'srcdir' in 'Makefile' and that value is only valid during the build of Python. It is the variable that is used in Python's makefiles to refer to files in the source tree and is needed when you run configure from a directory that is not the root of the source tree.

@warsaw
Copy link
Member

warsaw commented Mar 11, 2010

Adding xxmodule.c to the test directory is more than I want to do for 2.6.5. The actual crash happens because if sysconfig.get_config_var('srcdir') returns None, os.path.join() will traceback. It doesn't know how to handle None arguments. So my suggestion is that when srcdir is None, fallback to using sysconfig.project_base as before.

As long as that is guaranteed to be a string, we won't get the crash. _get_source_filename() will return some bogus but syntactically valid path, and then the os.path.exists() test in test_suite() will return False, so the test will be skipped. That seems like the safest change for 2.6.5 final, though I am fine with backporting the trunk fix for realzies in 2.6.6.

@tarekziade
Copy link
Mannequin

tarekziade mannequin commented Mar 11, 2010

@ronald, @barry: Yes that's what I did in trunk, so I can just merge that revision. (r78707, r78709)

The only problem I had with it is that in case xxmodule.c changes, I have to change it there too, but it's no big deal.

Barry, I'll merge back this as soon as I get a correct wifi connection this week :) (I am at a conference)

@tarekziade
Copy link
Mannequin

tarekziade mannequin commented Mar 12, 2010

done in r78877

@tarekziade tarekziade mannequin closed this as completed Mar 12, 2010
@warsaw
Copy link
Member

warsaw commented Mar 12, 2010

Tarek, r78877 is not the fix I recommended in

http://bugs.python.org/issue8107#msg100858

Are you sure you're more comfortable with the one you committed than the one I suggested?

@tarekziade
Copy link
Mannequin

tarekziade mannequin commented Mar 12, 2010

This will skip the whole test class if srcdir is not found, removing all tests run in test_build_ext.

Are you sure you want to skip the whole test class if the srcdir is not there for 2.6.5 final ? (we have important tests in there, related to options like inplace etc)

If so, I'll change it like you said (so test_build_ext is completely skipped under some environments).

@tarekziade tarekziade mannequin reopened this Mar 12, 2010
@warsaw
Copy link
Member

warsaw commented Mar 12, 2010

Hi Martin,

Can you just double check that the addition of xxmodule.c won't cause problems with the Windows installer. If it's okay (no problems), please close this bug and we'll keep the fix.

Thanks.

@warsaw warsaw assigned loewis and unassigned tarekziade Mar 12, 2010
@loewis
Copy link
Mannequin Author

loewis mannequin commented Mar 13, 2010

The patch in its current form is incomplete: xxmodule.c won't end up on the target system, since msi.py doesn't package it. So that would need to be changed as well.

With the current release26-maint branch, test_distutils now passes, both with xxmodule.c present, and with it being absent. If it is present, test_distutils prints a number of additional output lines, starting with "Creating library ...".

I also agree that the change is too heavy for the 2.6 maintenance release.

@tarekziade
Copy link
Mannequin

tarekziade mannequin commented Mar 13, 2010

Ok then, I am applying Barry's suggestion right now in release26-maint then

@tarekziade
Copy link
Mannequin

tarekziade mannequin commented Mar 13, 2010

done in r78935.

@warsaw
Copy link
Member

warsaw commented Mar 13, 2010

So, issue closed right? Or do you want to keep it bumped down and open for 2.6.6?

@warsaw warsaw closed this as completed Mar 13, 2010
@tarekziade
Copy link
Mannequin

tarekziade mannequin commented Mar 13, 2010

No I guess its fine to leave it closed.

@ezio-melotti ezio-melotti transferred this issue from another repository Apr 10, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
release-blocker stdlib Python modules in the Lib dir type-bug An unexpected behavior, bug, or error
Projects
None yet
Development

No branches or pull requests

5 participants