classification
Title: distutils tests fail if PATH is not defined
Type: behavior Stage: resolved
Components: Distutils Versions: Python 2.7
process
Status: closed Resolution: out of date
Dependencies: Superseder:
Assigned To: Nosy List: BreamoreBoy, alexis, dstufft, eric.araujo, henry.precheur, ncoghlan, steve.dower, tarek, terry.reedy, zach.ware
Priority: normal Keywords: patch

Created on 2011-06-27 12:33 by henry.precheur, last changed 2017-06-18 18:05 by terry.reedy. This issue is now closed.

Files
File name Uploaded Description Edit
fix_empty_path.diff henry.precheur, 2011-06-27 12:33 review
fix_distutils_tests.diff henry.precheur, 2011-06-28 02:57 review
distutils-12420.diff terry.reedy, 2014-06-28 19:05 review
Messages (22)
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) * (Python committer) 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) * (Python committer) 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) * (Python committer) 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) * (Python committer) 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) * (Python committer) 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) * (Python committer) 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: Nick Coghlan (ncoghlan) * (Python committer) 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: Nick Coghlan (ncoghlan) * (Python committer) 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) * (Python committer) 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) * (Python committer) 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) * (Python committer) 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) * (Python committer) 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) * (Python committer) Date: 2015-09-14 04:44
No VS2015 yet.  New issue #25100.  Thanks.
msg296282 - (view) Author: Terry J. Reedy (terry.reedy) * (Python committer) 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.
History
Date User Action Args
2017-06-18 18:05:59terry.reedysetstatus: 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:11terry.reedysetmessages: + msg250621
2015-09-14 04:34:34zach.waresetmessages: + msg250615
2015-09-14 04:28:40BreamoreBoysetnosy: + dstufft
messages: + msg250613
components: - Distutils2
2015-09-14 03:11:33terry.reedysetnosy: + zach.ware, steve.dower
messages: + msg250601
2014-06-28 19:05:47terry.reedysetfiles: + distutils-12420.diff
versions: + Python 3.5, - Python 3.3
2014-06-28 19:05:19terry.reedysetmessages: + msg221813
stage: patch review
2014-06-28 16:41:13terry.reedysetmessages: + msg221802
2014-06-28 06:41:19ncoghlansetmessages: + msg221776
2014-06-28 06:39:05ncoghlansetmessages: + msg221775
2014-06-28 05:45:38terry.reedysetmessages: + msg221773
2014-06-27 23:40:43BreamoreBoysetnosy: + BreamoreBoy
messages: + msg221752
2013-09-01 22:50:35terry.reedysetnosy: + terry.reedy, ncoghlan

messages: + msg196748
versions: + Python 3.4, - Python 3.2
2011-07-11 15:34:33eric.araujosetmessages: + msg140131
2011-06-28 02:57:49henry.precheursetfiles: + fix_distutils_tests.diff

messages: + msg139342
2011-06-28 02:53:36henry.precheursetfiles: - fix_distutils_test.diff
2011-06-27 17:27:33henry.precheursetfiles: + fix_distutils_test.diff

messages: + msg139318
2011-06-27 15:30:37eric.araujosetmessages: + msg139301
2011-06-27 15:19:00henry.precheursettitle: distutils crashes if PATH is not defined -> distutils tests fail if PATH is not defined
2011-06-27 15:17:57henry.precheursetmessages: + msg139297
2011-06-27 14:33:08eric.araujosetmessages: + msg139282
2011-06-27 14:20:07henry.precheursetmessages: + msg139280
2011-06-27 14:10:12henry.precheursetmessages: + msg139277
2011-06-27 13:54:02eric.araujosetassignee: tarek -> eric.araujo
type: behavior
messages: + msg139273
versions: - Python 2.6, Python 3.1, Python 3.4
2011-06-27 12:33:17henry.precheurcreate