msg139261 - (view) |
Author: Henry Precheur (henry.precheur) |
Date: 2011-06-27 12:33 |
The function find_executable crashes if PATH is not defined.
I admit that it's an extreme case, but it's probably better to on the safe side of things.
What about using the current directory only if PATH is not defined? This seems to be a reasonable workaround.
|
msg139273 - (view) |
Author: Éric Araujo (eric.araujo) * |
Date: 2011-06-27 13:54 |
Thanks for the report. Can you tell how you ran into this? Did you call the function directly, or did you get the bug while running a setup.py command? Also, what do you mean by crash? We use that for CPython segmentation faults, not regular misbehavior. If you could attach the exact command or script that triggered the bug and the full traceback, it would help.
I don’t think using the current directory is a good idea: it’s a security hazard.
|
msg139277 - (view) |
Author: Henry Precheur (henry.precheur) |
Date: 2011-06-27 14:10 |
Sorry "crash" wasn't the right term. It's just that distutils tests fail.
I ran into that when trying to run unit tests without any environment
variable (see #12401).
$ env -i ./python ./Lib/test/regrtest.py test_distutils
[1/1] test_distutils
test test_distutils crashed -- Traceback (most recent call last):
File "./Lib/test/regrtest.py", line 987, in runtest_inner
File "/home/henry/code/cpython/Lib/test/test_distutils.py", line 13, in test_main
test.support.run_unittest(distutils.tests.test_suite())
File "/home/henry/code/cpython/Lib/distutils/tests/__init__.py", line 29, in test_suite
__import__(modname)
File "/home/henry/code/cpython/Lib/distutils/tests/test_archive_util.py", line 33, in <module>
unittest.TestCase):
File "/home/henry/code/cpython/Lib/distutils/tests/test_archive_util.py", line 96, in ArchiveUtilTestCase
@unittest.skipUnless(find_executable('tar') and find_executable('gzip')
File "/home/henry/code/cpython/Lib/distutils/spawn.py", line 154, in find_executable
path = os.environ['PATH']
File "/home/henry/code/cpython/Lib/os.py", line 450, in __getitem__
value = self._data[self.encodekey(key)]
KeyError: b'PATH'
1 test failed:
test_distutils
[98227 refs]
Maybe it's not really a problem, and having a system without PATH
defined shouldn't be supported because it's too "weird".
|
msg139280 - (view) |
Author: Henry Precheur (henry.precheur) |
Date: 2011-06-27 14:20 |
I don't know exactly in which context find_executable should be used,
but after taking a closer look it seems that returning None when PATH is
not defined could "work".
|
msg139282 - (view) |
Author: Éric Araujo (eric.araujo) * |
Date: 2011-06-27 14:33 |
Okay, I see the original use case (#12401). I think the proper thing to do is to skip tests that rely on the environment being non-empty.
|
msg139297 - (view) |
Author: Henry Precheur (henry.precheur) |
Date: 2011-06-27 15:17 |
I think that returning None would be a better option. The function
documentation says:
Tries to find 'executable' in the directories listed in 'path'.
A string listing directories separated by 'os.pathsep'; defaults to
os.environ['PATH']. Returns the complete filename or None if not found.
If os.environ['PATH'] is empty the function returns None:
>>> from distutils.spawn import find_executable
>>> find_executable('does not exist', path='/bin:/usr/bin') is None
True
>>> find_executable('test', path='') is None
True
This would be consistent with the function definition. If PATH is
undefined, the executable cannot be found, therefor returning None seems
like the right thing to do.
On Mon, Jun 27, 2011 at 02:33:09PM +0000, ??ric Araujo wrote:
>
> ??ric Araujo <merwok@netwok.org> added the comment:
>
> Okay, I see the original use case (#12401). I think the proper thing to do is to skip tests that rely on the environment being non-empty.
>
> ----------
>
> _______________________________________
> Python tracker <report@bugs.python.org>
> <http://bugs.python.org/issue12420>
> _______________________________________
|
msg139301 - (view) |
Author: Éric Araujo (eric.araujo) * |
Date: 2011-06-27 15:30 |
To explain my position: distutils is a very brittle codebase that’s used and monkey-patched by a lot of third-party code. In the past, before the forking of distutils/packaging, Tarek tried to gradually improve distutils but he got a lot of pushback when his changes broke this third-party code that relied on known bugs or undocumented behavior or worked around it. That’s why a feature freeze is now in effect, and behavior is not changed unless it is to fix a bug.
In this case, it is not documented that distutils should run under python -E, and nobody reported it as a bug before, that’s why I have the position that the tests that require $PATH should be skipped, and the code left untouched.
For distutils2 (named packaging in the 3.3 standard library), we can improve find_executable. We even want to extract it and move it to shutil: #444582
|
msg139318 - (view) |
Author: Henry Precheur (henry.precheur) |
Date: 2011-06-27 17:27 |
OK it makes sense.
I'm working on fixing the tests. I'm not done yet, but I've attached a patch with my work in progress (some tests still fail). Do you have any comments?
I'll post a complete version later today or tomorrow.
|
msg139342 - (view) |
Author: Henry Precheur (henry.precheur) |
Date: 2011-06-28 02:57 |
I've fixed the last failing tests, but I'm unsure it's the right way to do it.
Take a look at the part for Lib/distutils/tests/test_build_ext.py. I just check that os.environ['PATH'] is defined. I'm not 100% certain that this will work on every platforms.
|
msg140131 - (view) |
Author: Éric Araujo (eric.araujo) * |
Date: 2011-07-11 15:34 |
Sorry, I don’t want to monkey-patch find_executable during tests. We want the tests to work with the real code, with the same behavior and errors as real setup.py calls. If a test does not work with python -E, then it should be skipped.
(Implementation idea:
skip_if_empty_env = unittest.skipUnless(
os.environ, 'test cannot run with an empty environment')
using sys.flags is not possible due to backward compatibility policy)
|
msg196748 - (view) |
Author: Terry J. Reedy (terry.reedy) * |
Date: 2013-09-01 22:50 |
I just ran into this issue (same I believe) running the test suite with installed, versus repository, python on win7, but get a different traceback.
Traceback (most recent call last):
...\lib\distutils\tests\test_build_ext.py"
line 156, in test_optional_extension # or
line 316, in test_get_outputs
cmd.run) # should raise an error
...\lib\unittest\case.py", line 570, in assertRaises
return context.handle('assertRaises', callableObj, args, kwargs)
...\lib\unittest\case.py", line 135, in handle
callable_obj(*args, **kwargs)
...\lib\distutils\command\build_ext.py", line 354, in run
self.build_extensions()
...\lib\distutils\command\build_ext.py", line 463, in build_extensions
self.build_extension(ext)
...\lib\distutils\command\build_ext.py", line 518, in build_extension
depends=ext.depends)
...\lib\distutils\msvc9compiler.py", line 460, in compile
self.initialize()
...\lib\distutils\msvc9compiler.py", line 371, in initialize
vc_env = query_vcvarsall(VERSION, plat_spec)
...\lib\distutils\msvc9compiler.py", line 287, in query_vcvarsall
raise ValueError(str(list(result.keys())))
ValueError: ['path']
I agree with Éric that the test should be skipped if it cannot go on.
Wrapping
self.assertRaises((UnknownFileError, CompileError),
cmd.run) # should raise an error
and
cmd.run()
with
try:...except ValueError: <skip> would fix this issue on Windows. So would skipping if sys.platform == 'win32' and '+' not in sys.version (repository builds have a '+' and the test currently work for them). However, neither solution would work on *nix, which seems to have a completely different failure path to the same end result.
Are distutils and test_distutils maintained at all? msvc9compiler.py (2005) says it also works with vc10 (2008, used for 2.7) but makes no mention of vc11 (2010, used for 3.3,3.4).
Nick, what do you think we should do with this? I think that when python is installed and working as well as we expect, the test suite should pass, so any failures indicate a problem with the particular installation. I think this is especially important on Windows, with newbies who do not know distutils from, say, itertools.
|
msg221752 - (view) |
Author: Mark Lawrence (BreamoreBoy) * |
Date: 2014-06-27 23:40 |
Can someone follow up on this please. See also #12401.
|
msg221773 - (view) |
Author: Terry J. Reedy (terry.reedy) * |
Date: 2014-06-28 05:45 |
test_disutils still fails on both 2.7 and 3.4 installs. Since PATH is defined, that is not the issue here. I am tempted to unconditionally skip the test and close this issue.
|
msg221775 - (view) |
Author: Alyssa Coghlan (ncoghlan) * |
Date: 2014-06-28 06:39 |
For Python 3, I suggest tweaking the code to use shutil.which and see if that improves matters.
For Python 2, I'm inclined not to worry about it.
|
msg221776 - (view) |
Author: Alyssa Coghlan (ncoghlan) * |
Date: 2014-06-28 06:41 |
Skipping if the compiler isn't available is problematic, since that could reflect a real failure mode for the code.
|
msg221802 - (view) |
Author: Terry J. Reedy (terry.reedy) * |
Date: 2014-06-28 16:41 |
I feel that conscientious users who test their installations should get a clean test. They cannot be expected to know that this is an 'expected failure' and therefore not really a failure.
Test_tools has the following, which indeed works to skip on installed 3.4.1 but not on built 3.4.1+.
if not sysconfig.is_python_build():
# XXX some installers do contain the tools, should we detect that
# and run the tests in that case too?
raise unittest.SkipTest('test irrelevant for an installed Python')
How about we decorate the two failing tests
line 156, in test_optional_extension # or
line 316, in test_get_outputs
with
@unittest.skipUnless(sysconfig.is_python_build(),
'test irrelevant for an installed Python') # or modify message
?
|
msg221813 - (view) |
Author: Terry J. Reedy (terry.reedy) * |
Date: 2014-06-28 19:05 |
A minor change: distutils has is own version of sysconfig*, which is already imported into disutils/tests/test_build_ext.py. It has '_python_build' instead of 'is_python_build'. With that change, the decorators work as expected to skip the tests on installed Windows 3.4.1 either when test_build_ext is run by itself as main or as part of test_distutils.
# Seems like a violation of DRY. According to hg revision history, (and visually comparing the two, some patches have been applied to just one, some to both.
|
msg250601 - (view) |
Author: Terry J. Reedy (terry.reedy) * |
Date: 2015-09-14 03:11 |
The following failures on new 3.5.0 seems related as they all end with the same message about vcvarsall.bat (slightly different from 3.4 failures).
======================================================================
ERROR: test_get_outputs (distutils.tests.test_build_ext.BuildExtTestCase)
----------------------------------------------------------------------
======================================================================
ERROR: test_optional_extension (distutils.tests.test_build_ext.BuildExtTestCase)
----------------------------------------------------------------------
======================================================================
ERROR: test_get_outputs (distutils.tests.test_build_ext.ParallelBuildExtTestCase)
----------------------------------------------------------------------
======================================================================
ERROR: test_optional_extension (distutils.tests.test_build_ext.ParallelBuildExtTestCase)
----------------------------------------------------------------------
======================================================================
ERROR: test_compiler_options (distutils.tests.test_msvccompiler.msvccompilerTestCase)
----------------------------------------------------------------------
======================================================================
ERROR: test_vcruntime_copy (distutils.tests.test_msvccompiler.msvccompilerTestCase)
----------------------------------------------------------------------
======================================================================
ERROR: test_vcruntime_skip_copy (distutils.tests.test_msvccompiler.msvccompilerTestCase)
----------------------------------------------------------------------
======================================================================
FAIL: test_customize_compiler_before_get_config_vars (distutils.tests.test_sysconfig.SysconfigTestCase)
----------------------------------------------------------------------
Traceback (most recent call last):
File "C:\Programs\Python 3.5\lib\distutils\tests\test_sysconfig.py", line 197, in test_customize_compiler_before_get_c
onfig_vars
self.assertEqual(0, p.returncode, "Subprocess failed: " + outs)
AssertionError: 0 != 1 : Subprocess failed: Traceback (most recent call last):
File "@test_5504_tmp", line 5, in <module>
rc = config.try_compile('int x;')
File "C:\Programs\Python 3.5\lib\distutils\command\config.py", line 227, in try_compile
self._compile(body, headers, include_dirs, lang)
File "C:\Programs\Python 3.5\lib\distutils\command\config.py", line 133, in _compile
self.compiler.compile([src], include_dirs=include_dirs)
File "C:\Programs\Python 3.5\lib\distutils\_msvccompiler.py", line 315, in compile
self.initialize()
File "C:\Programs\Python 3.5\lib\distutils\_msvccompiler.py", line 208, in initialize
vc_env = _get_vc_env(plat_spec)
File "C:\Programs\Python 3.5\lib\distutils\_msvccompiler.py", line 83, in _get_vc_env
raise DistutilsPlatformError("Unable to find vcvarsall.bat")
distutils.errors.DistutilsPlatformError: Unable to find vcvarsall.bat
----------------------------------------------------------------------
Ran 234 tests in 1.108s
FAILED (failures=1, errors=7, skipped=28)
Warning -- threading._dangling was modified by test_distutils
Warning -- files was modified by test_distutils
|
msg250613 - (view) |
Author: Mark Lawrence (BreamoreBoy) * |
Date: 2015-09-14 04:28 |
"Unable to find vcvarsall.bat" is raised when Visual Studio isn't installed or it's the incorrect version, so I don't believe these are the same at all.
|
msg250615 - (view) |
Author: Zachary Ware (zach.ware) * |
Date: 2015-09-14 04:34 |
Agreed with Mark; I don't see anywhere that lack of PATH would affect the failures you're seeing, Terry. Do you have VS2015 installed? If not, the failure of distutils to skip the tests that need it should be raised as a separate issue.
|
msg250621 - (view) |
Author: Terry J. Reedy (terry.reedy) * |
Date: 2015-09-14 04:44 |
No VS2015 yet. New issue #25100. Thanks.
|
msg296282 - (view) |
Author: Terry J. Reedy (terry.reedy) * |
Date: 2017-06-18 18:05 |
test_distutils now passes on 3.6, so this is out-of-date for 3.x.
On installed 2.7.13 (released Dec 2016)
ERROR: test_get_outputs (distutils.tests.test_build_ext.BuildExtTestCase)
FAIL: test_customize_compiler_before_get_config_vars (distutils.tests.test_sysconfig.SysconfigTestCase)
the error is ValueError: [u'path'] instead of ValueError: ['path'].
Nick suggested leaving 2.7 alone, I don't care about it either, and I believe Victor Stinner has worked on its tests since last December.
|
|
Date |
User |
Action |
Args |
2022-04-11 14:57:19 | admin | set | github: 56629 |
2017-06-18 18:05:59 | terry.reedy | set | status: open -> closed versions:
- Python 3.4, Python 3.5 messages:
+ msg296282
assignee: eric.araujo -> resolution: out of date stage: patch review -> resolved |
2015-09-14 04:44:11 | terry.reedy | set | messages:
+ msg250621 |
2015-09-14 04:34:34 | zach.ware | set | messages:
+ msg250615 |
2015-09-14 04:28:40 | BreamoreBoy | set | nosy:
+ dstufft messages:
+ msg250613 components:
- Distutils2
|
2015-09-14 03:11:33 | terry.reedy | set | nosy:
+ zach.ware, steve.dower messages:
+ msg250601
|
2014-06-28 19:05:47 | terry.reedy | set | files:
+ distutils-12420.diff versions:
+ Python 3.5, - Python 3.3 |
2014-06-28 19:05:19 | terry.reedy | set | messages:
+ msg221813 stage: patch review |
2014-06-28 16:41:13 | terry.reedy | set | messages:
+ msg221802 |
2014-06-28 06:41:19 | ncoghlan | set | messages:
+ msg221776 |
2014-06-28 06:39:05 | ncoghlan | set | messages:
+ msg221775 |
2014-06-28 05:45:38 | terry.reedy | set | messages:
+ msg221773 |
2014-06-27 23:40:43 | BreamoreBoy | set | nosy:
+ BreamoreBoy messages:
+ msg221752
|
2013-09-01 22:50:35 | terry.reedy | set | nosy:
+ terry.reedy, ncoghlan
messages:
+ msg196748 versions:
+ Python 3.4, - Python 3.2 |
2011-07-11 15:34:33 | eric.araujo | set | messages:
+ msg140131 |
2011-06-28 02:57:49 | henry.precheur | set | files:
+ fix_distutils_tests.diff
messages:
+ msg139342 |
2011-06-28 02:53:36 | henry.precheur | set | files:
- fix_distutils_test.diff |
2011-06-27 17:27:33 | henry.precheur | set | files:
+ fix_distutils_test.diff
messages:
+ msg139318 |
2011-06-27 15:30:37 | eric.araujo | set | messages:
+ msg139301 |
2011-06-27 15:19:00 | henry.precheur | set | title: distutils crashes if PATH is not defined -> distutils tests fail if PATH is not defined |
2011-06-27 15:17:57 | henry.precheur | set | messages:
+ msg139297 |
2011-06-27 14:33:08 | eric.araujo | set | messages:
+ msg139282 |
2011-06-27 14:20:07 | henry.precheur | set | messages:
+ msg139280 |
2011-06-27 14:10:12 | henry.precheur | set | messages:
+ msg139277 |
2011-06-27 13:54:02 | eric.araujo | set | assignee: tarek -> eric.araujo type: behavior messages:
+ msg139273 versions:
- Python 2.6, Python 3.1, Python 3.4 |
2011-06-27 12:33:17 | henry.precheur | create | |