classification
Title: distutils.filelist.findall() fails on broken symlink in Py2.x
Type: behavior Stage:
Components: Distutils Versions: Python 3.6, Python 3.5
process
Status: closed Resolution: fixed
Dependencies: Superseder:
Assigned To: jason.coombs Nosy List: Alexander.Dutton, alexis, dstufft, eric.araujo, jason.coombs, martin.panter, python-dev, tarek
Priority: normal Keywords: patch

Created on 2011-09-02 13:57 by Alexander.Dutton, last changed 2016-09-06 02:29 by python-dev. This issue is now closed.

Files
File name Uploaded Description Edit
faf37fc3b097.diff jason.coombs, 2015-08-30 18:29 review
3da42b593e1f.diff jason.coombs, 2015-09-19 16:22 review
Repositories containing patches
https://bitbucket.org/jaraco/cpython-issue12885#3.4
Messages (16)
msg143399 - (view) Author: Alexander Dutton (Alexander.Dutton) Date: 2011-09-02 13:57
If there are any broken symlinks in the same directory as a setup.py when e.g. sdist is run, findall() will fall over when attempting to os.stat() the symlink:

Traceback (most recent call last):
  File "setup.py", line 81, in run
    _sdist.run(self)
  File "/usr/lib/python2.6/distutils/command/sdist.py", line 144, in run
    self.get_file_list()
  File "/usr/lib/python2.6/distutils/command/sdist.py", line 238, in get_file_list
    self.filelist.findall()
  File "/usr/lib/python2.6/distutils/filelist.py", line 47, in findall
    self.allfiles = findall(dir)
  File "/usr/lib/python2.6/distutils/filelist.py", line 297, in findall
    stat = os.stat(fullname)
OSError: [Errno 2] No such file or directory: 'debian/tmp/usr/share/somepath/somesymlink'

Solutions would include replacing the call to os.stat() with one to os.lstat() (probably backwards-incompatible), or trying one and then the other.

This bug is present in Pythons 2.6.6 (Debian 6.0.2) and 2.7 (Fedora 14).

When attempting to reproduce in Python 3.1.2 (on Fedora) no error was encountered. However, looking at distutils/filelist.py, the same unadulterated call to os.stat() is present. I'll presume that for whatever reason distutils in Py3.1.2 never has cause to stat my broken symlink.
msg143413 - (view) Author: Éric Araujo (eric.araujo) * (Python committer) Date: 2011-09-02 16:44
Symlinks are barely supported by distutils; the only mention you can find in the doc is that the MANIFEST file can contain symlinks.  In the light of that, one could argue that broken symlinks are not supported nor ignored, and that you just should not have them.  Is there a valid use case for having broken symlinks in a Python project?

(About versions: I’m removing 2.6 which only gets security fixes, and adding 3.x to add regression tests to them, even if the bug isn’t there now.)
msg143496 - (view) Author: Alexander Dutton (Alexander.Dutton) Date: 2011-09-04 13:06
I've come across it as I'm creating a Debian package of the Python package in the same tree — I'm happy to be told this is a Bad Idea and that they should be in different places.

The broken symlinks are relative and in debian/tmp, and will point to locations provided by other Debian packages once my package is installed in the right location.

FWIW, I'm getting round it at the moment by walking the directory tree and removing the files is os.path.islink(filename) and not os.path.exists(os.path.join(filename, os.readlink(filename))).

I'm happy to provide tests and a patch if necessary.
msg143534 - (view) Author: Éric Araujo (eric.araujo) * (Python committer) Date: 2011-09-05 15:51
> I've come across it as I'm creating a Debian package of the Python
> package in the same tree

I think a lot of people are doing this.

> The broken symlinks are relative and in debian/tmp, and will point to
> locations provided by other Debian packages once my package is
> installed in the right location.

It’s too bad that filelist goes into the debian subdirectory :(  The Ubuntu-originated python-distutils-extra project had a similar problem and they switched from FileList.findall to os.walk to avoid it.

What happens if you ignore the debian dir in MANIFEST.in?
msg230021 - (view) Author: Jason R. Coombs (jason.coombs) * (Python committer) Date: 2014-10-26 00:10
This issue has been monkeypatched by setuptools for 7 years: https://bitbucket.org/pypa/setuptools/commits/4556f9d08ef7
msg249366 - (view) Author: Jason R. Coombs (jason.coombs) * (Python committer) Date: 2015-08-30 18:32
Eric, would you be willing to review the commits in the referenced patch repo (collapsed in the attached diff)? If you're happy with the implementation, I'll backport it to Python 2.7 and apply it to 3.4-3.6 and then update setuptools to only monkeypatch older distutils.
msg249384 - (view) Author: Éric Araujo (eric.araujo) * (Python committer) Date: 2015-08-31 06:38
Patch looks good and would fix the reported problem.  Being a backport from setuptools also gives confidence.

Does the patch change the behaviour for the handling of the MANIFEST file (not MANIFEST.in)?  My previous message mentions that the docs say that one can include symlinks in MANIFEST.
msg251083 - (view) Author: Jason R. Coombs (jason.coombs) * (Python committer) Date: 2015-09-19 15:00
Although the code was ported from setuptools, it wasn't ported from the long stable implementation, but was ported following the recent refactor, which had a [regression](https://bitbucket.org/pypa/setuptools/issues/425/odd-failure-on-183-missing-files-but-they) revealing that the [docstring no longer matched the implementation](https://bitbucket.org/pypa/setuptools/commits/72ec36bea07c7dd1cf48e20894864e8a4e1b480f). The latest implementation provides [a more elegant approach](https://bitbucket.org/pypa/setuptools/src/738b139f3016a05084c9f34c6364d7d20b3aac9d/setuptools/__init__.py?fileviewer=file-view-default#__init__.py-154), but continues to maintain the expectation of the setuptools implementation.

I still need to address Eric's question about what filelists are affected, but also now update the patch to ensure that the disparity between findall('.') and findall(anything_else) is captured.
msg251089 - (view) Author: Jason R. Coombs (jason.coombs) * (Python committer) Date: 2015-09-19 16:28
After porting the latest released Setuptools version, I discovered yet another regression in the Setuptools implementation, specifically around its handling for this issue, so I've created yet another release of Setuptools (18.3.2) to include tests and fixes for the expected behavior on this issue. Rebasing those changes, the patch 3da42b593e1f now reflects my latest proposal.
msg251170 - (view) Author: Jason R. Coombs (jason.coombs) * (Python committer) Date: 2015-09-20 17:04
> Does the patch change the behaviour for the handling of the MANIFEST file (not MANIFEST.in)?  My previous message mentions that the docs say that one can include symlinks in MANIFEST.

This change will not affect the content of MANIFEST.in (template) nor MANIFEST. Previously, if a broken symlink was provided, it would raise an error. With the patch, under that condition the process will not fail but will instead ignore those broken symlinks as if they do not exist (and omit them from MANIFEST).

Non-broken symlinks (those that resolve to directories or files) will continue to be treated exactly as if their targets exist in that place. To my knowledge, there is no special handling of symlinks (such as with tarfiles that store the underlying symbolic link details).
msg274191 - (view) Author: Roundup Robot (python-dev) (Python triager) Date: 2016-09-02 01:17
New changeset 79422fab2ef4 by Jason R. Coombs in branch 'default':
Issue #12885: Merge with 3.5
https://hg.python.org/cpython/rev/79422fab2ef4
msg274193 - (view) Author: Jason R. Coombs (jason.coombs) * (Python committer) Date: 2016-09-02 01:27
This code has been stable in Setuptools for some time, so I've pushed the code into the base.
msg274195 - (view) Author: Martin Panter (martin.panter) * (Python committer) Date: 2016-09-02 02:05
I think you mixed up the bug number. You should probably fix Misc/NEWS.

New changeset 941346104718 by Jason R. Coombs in branch '3.4':
Issue #12285: Add test capturing failure.
https://hg.python.org/cpython/rev/941346104718

New changeset 56c60b3d06fb by Jason R. Coombs in branch '3.4':
Issue #12285: Replace implementation of findall with implementation from Setuptools 7ce820d524db.
https://hg.python.org/cpython/rev/56c60b3d06fb

New changeset 13619a3e0737 by Jason R. Coombs in branch '3.4':
Issue #12285: Update NEWS
https://hg.python.org/cpython/rev/13619a3e0737
msg274197 - (view) Author: Roundup Robot (python-dev) (Python triager) Date: 2016-09-02 02:10
New changeset 590d0de4ff48 by Jason R. Coombs in branch '3.4':
Issue #12885: Correct issue reference in NEWS
https://hg.python.org/cpython/rev/590d0de4ff48

New changeset 74ccec0bd442 by Jason R. Coombs in branch '3.5':
Issue #12885: Merge 3.4
https://hg.python.org/cpython/rev/74ccec0bd442

New changeset bfff89ed356b by Jason R. Coombs in branch 'default':
Issue #12885: Merge with 3.5
https://hg.python.org/cpython/rev/bfff89ed356b
msg274199 - (view) Author: Roundup Robot (python-dev) (Python triager) Date: 2016-09-02 03:29
New changeset e82b995d1a5c by Jason R. Coombs in branch '3.4':
Issue #12885: Revert commits in 3.4 branch which is security-only fixes.
https://hg.python.org/cpython/rev/e82b995d1a5c

New changeset a7ce98a4e9e4 by Jason R. Coombs in branch '3.5':
Issue #12885: Merge with 3.4, retaining commits reverted there.
https://hg.python.org/cpython/rev/a7ce98a4e9e4

New changeset fa82fabe9d65 by Jason R. Coombs in branch 'default':
Issue #12885: Merge with 3.5
https://hg.python.org/cpython/rev/fa82fabe9d65
msg274501 - (view) Author: Roundup Robot (python-dev) (Python triager) Date: 2016-09-06 02:29
New changeset 556a11c11edd by Jason R. Coombs in branch '3.4':
Issue #27960: Revert state to 675e20c38fdac6, backing out all changes by developed for Issue #12885.
https://hg.python.org/cpython/rev/556a11c11edd
History
Date User Action Args
2016-09-06 02:29:57python-devsetmessages: + msg274501
2016-09-02 03:30:49jason.coombssetversions: - Python 3.4
2016-09-02 03:29:44python-devsetmessages: + msg274199
2016-09-02 02:10:55python-devsetmessages: + msg274197
2016-09-02 02:05:16martin.pantersetnosy: + martin.panter
messages: + msg274195
2016-09-02 01:27:07jason.coombssetstatus: open -> closed
resolution: fixed
messages: + msg274193

versions: - Python 2.7
2016-09-02 01:17:09python-devsetnosy: + python-dev
messages: + msg274191
2015-09-20 17:04:48jason.coombssetmessages: + msg251170
2015-09-19 16:28:47jason.coombssetmessages: + msg251089
2015-09-19 16:22:03jason.coombssetfiles: + 3da42b593e1f.diff
2015-09-19 15:00:16jason.coombssetmessages: + msg251083
2015-08-31 06:38:09eric.araujosetassignee: eric.araujo -> jason.coombs
messages: + msg249384
2015-08-30 18:32:24jason.coombssetassignee: jason.coombs -> eric.araujo
messages: + msg249366
versions: + Python 3.6
2015-08-30 18:29:11jason.coombssetfiles: + faf37fc3b097.diff
keywords: + patch
2015-08-30 18:28:19jason.coombssetassignee: jason.coombs
hgrepos: + hgrepo315
2014-10-26 00:10:17jason.coombssetversions: + Python 3.4, Python 3.5, - 3rd party, Python 3.2, Python 3.3
nosy: + dstufft, jason.coombs

messages: + msg230021

assignee: tarek -> (no value)
components: - Distutils2
2011-09-05 15:51:42eric.araujosetmessages: + msg143534
2011-09-04 13:06:12Alexander.Duttonsetmessages: + msg143496
2011-09-02 16:44:40eric.araujosetversions: + 3rd party, Python 3.2, Python 3.3, - Python 2.6, Python 3.1
nosy: + alexis

messages: + msg143413

components: + Distutils2
2011-09-02 13:57:22Alexander.Duttoncreate