classification
Title: Impossible to include file in sdist that starts with 'build' on Win32
Type: behavior Stage: resolved
Components: Distutils, Distutils2 Versions: Python 3.3, Python 3.2, Python 2.7
process
Status: closed Resolution: fixed
Dependencies: Superseder:
Assigned To: eric.araujo Nosy List: Adam.Groszer, benjamin.peterson, cjw296, eric.araujo, georg.brandl, jason.coombs, nadeem.vawda, python-dev, tarek
Priority: release blocker Keywords: patch

Created on 2009-09-11 14:28 by cjw296, last changed 2012-04-09 19:06 by python-dev. This issue is now closed.

Files
File name Uploaded Description Edit
test-buildout-inclusion.diff eric.araujo, 2010-08-13 22:20
filelist-regex-bug.diff eric.araujo, 2012-02-18 02:04
filelist-regex-bugs.diff eric.araujo, 2012-02-24 07:22
filelist-regex-v2.diff nadeem.vawda, 2012-02-24 13:41
filelist-regex-bugs-v3.diff eric.araujo, 2012-02-25 06:41
filelist-regex-v4.diff nadeem.vawda, 2012-02-25 08:07
filelist-regex-bugs-3.2.diff eric.araujo, 2012-02-25 14:51 review
fix-distutils2-manifest-bugs.diff eric.araujo, 2012-02-26 03:37
Messages (45)
msg92511 - (view) Author: Chris Withers (cjw296) * (Python committer) Date: 2009-09-11 14:28
With a simple setup.py:

from distutils.core import setup
setup(name='packagename')

And a MANIFEST.in containing:

include buildout.cfg 

The result of a debug run of python setup.py sdist on Windows is:

Distribution.parse_config_files():
options (after parsing config files):
no commands known yet
options (after parsing command line):
option dict for 'sdist' command:
  {}
running sdist
Distribution.get_command_obj(): creating 'sdist' command object
warning: sdist: missing required meta-data: version, url
warning: sdist: missing meta-data: either (author and author_email) or 
(maintainer and maintainer_email) mus
t be supplied
checking if setup.py newer than MANIFEST
warning: sdist: standard file not found: should have one of README, 
README.txt
reading manifest template 'MANIFEST.in'
include buildout.cfg
include_pattern: applying regex r'^buildout\.cfg$'
 adding buildout.cfg
Distribution.get_command_obj(): creating 'build' command object
exclude_pattern: applying regex r'^build\.*'
 removing buildout.cfg

...

The regex built is incorrect, although it is apparently correctly built 
on other platforms...
msg108005 - (view) Author: Adam Groszer (Adam.Groszer) Date: 2010-06-17 12:29
This seems to be a major flaw, noone caring about it?
msg108099 - (view) Author: Chris Withers (cjw296) * (Python committer) Date: 2010-06-18 11:16
I guess not... Maybe try catching Tarek on irc or drop him a mail?
msg111049 - (view) Author: Éric Araujo (eric.araujo) * (Python committer) Date: 2010-07-21 13:15
Sorry for the lag. There are a lot of bugs and Tarek is very busy. The steps you describe need to be turned in a new test in py3k’s test_sdist.
msg113687 - (view) Author: Éric Araujo (eric.araujo) * (Python committer) Date: 2010-08-12 15:51
I’ll write a unit test shortly.
msg113841 - (view) Author: Éric Araujo (eric.araujo) * (Python committer) Date: 2010-08-13 22:20
Attaching a unit test translation of Chris’ report. The test pass on POSIX (linux2), can someone test on win32?

Tracing the flow from sdist to filelist, it seems to be not a regex error but a logic error. A file explicitly added should not be removed by the exclude filter afterward.
msg113850 - (view) Author: Éric Araujo (eric.araujo) * (Python committer) Date: 2010-08-13 23:22
Test by haypo on win32:
- ['README', 'setup.py', 'somecode\\__init__.py']
?                                 ^^

+ ['README', 'buildout.cfg', 'setup.py', 'somecode/__init__.py']
?           ++++++++++++++++                      ^

The bug is confirmed. I’ll use os.sep to silence the second error, and look into test_filelist and filelist itself to fix it.
msg116424 - (view) Author: Chris Withers (cjw296) * (Python committer) Date: 2010-09-14 23:07
This is a regex bug, and it just bit me again :-(

Because of this bug, you cannot currently build a bdist_egg (and therefore cannot install with easy_install) http://pypi.python.org/pypi/buildout-versions on windows.

The only choice I have is to rename the 'buildout_versions' package inside the distribution to something that doesn't start with 'build'.
msg116457 - (view) Author: Chris Withers (cjw296) * (Python committer) Date: 2010-09-15 15:24
So, the workaround I gave doesn't work, because this stupid regex still knocks out all the egg-info. The only solution is to use that accidentally employed by http://svn.plone.org/svn/collective/buildout/buildout.dumppickedversions/trunk/ and put any files starting with 'build' and that need to be used on Windows in a sub-folder.

Here's a debug sdist build dump from buildout.dumppickedversions:

Running setup script 'setup.py'.
Distribution.parse_config_files():
options (after parsing config files):
no commands known yet
options (after parsing command line):
option dict for 'aliases' command:
  {}
option dict for 'sdist' command:
  {}
running sdist
Distribution.get_command_obj(): creating 'sdist' command object
running egg_info
Distribution.get_command_obj(): creating 'egg_info' command object
writing requirements to src\buildout.dumppickedversions.egg-info\requires.txt
writing src\buildout.dumppickedversions.egg-info\PKG-INFO
writing namespace_packages to src\buildout.dumppickedversions.egg-info\namespace_packages.txt
writing top-level names to src\buildout.dumppickedversions.egg-info\top_level.txt
writing dependency_links to src\buildout.dumppickedversions.egg-info\dependency_links.txt
writing entry points to src\buildout.dumppickedversions.egg-info\entry_points.txt
Distribution.get_command_obj(): creating 'build_py' command object
Distribution.get_command_obj(): creating 'build' command object
include_pattern: applying regex r'^src\\buildout\.dumppickedversions\.egg\-info\.*[^/]*\Z(?ms)'
 adding src\buildout.dumppickedversions.egg-info\dependency_links.txt
 adding src\buildout.dumppickedversions.egg-info\entry_points.txt
 adding src\buildout.dumppickedversions.egg-info\namespace_packages.txt
 adding src\buildout.dumppickedversions.egg-info\not-zip-safe
 adding src\buildout.dumppickedversions.egg-info\PKG-INFO
 adding src\buildout.dumppickedversions.egg-info\requires.txt
 adding src\buildout.dumppickedversions.egg-info\SOURCES.txt
 adding src\buildout.dumppickedversions.egg-info\top_level.txt
exclude_pattern: applying regex r'^build\.*'
 removing buildout.cfg
exclude_pattern: applying regex r'^buildout\.dumppickedversions\-0\.5\.*'
...

Now contrast with buildout_versions:

Running setup script 'setup.py'.
Distribution.parse_config_files():
options (after parsing config files):
no commands known yet
options (after parsing command line):
option dict for 'aliases' command:
  {}
option dict for 'sdist' command:
  {}
running sdist
Distribution.get_command_obj(): creating 'sdist' command object
running egg_info
Distribution.get_command_obj(): creating 'egg_info' command object
writing requirements to buildout_versions.egg-info\requires.txt
writing buildout_versions.egg-info\PKG-INFO
writing top-level names to buildout_versions.egg-info\top_level.txt
writing dependency_links to buildout_versions.egg-info\dependency_links.txt
writing entry points to buildout_versions.egg-info\entry_points.txt
Distribution.get_command_obj(): creating 'build_py' command object
Distribution.get_command_obj(): creating 'build' command object
include_pattern: applying regex r'^buildout\_versions\.egg\-info\.*[^/]*\Z(?ms)'
 adding buildout_versions.egg-info\dependency_links.txt
 adding buildout_versions.egg-info\entry_points.txt
 adding buildout_versions.egg-info\not-zip-safe
 adding buildout_versions.egg-info\PKG-INFO
 adding buildout_versions.egg-info\requires.txt
 adding buildout_versions.egg-info\SOURCES.txt
 adding buildout_versions.egg-info\top_level.txt
exclude_pattern: applying regex r'^build\.*'
 removing buildout_versions.egg-info\top_level.txt
 removing buildout_versions.egg-info\SOURCES.txt
 removing buildout_versions.egg-info\requires.txt
 removing buildout_versions.egg-info\PKG-INFO
 removing buildout_versions.egg-info\not-zip-safe
 removing buildout_versions.egg-info\entry_points.txt
 removing buildout_versions.egg-info\dependency_links.txt
 removing buildout.cfg
 removing buildout_versions.egg-info\SOURCES.txt
exclude_pattern: applying regex r'^buildout\-versions\-1\.4\.*'
msg116461 - (view) Author: Chris Withers (cjw296) * (Python committer) Date: 2010-09-15 15:50
Okay, so here's the patch:

Change line 352 on distutils/filelist.py from:

pattern_re = "^" + os.path.join(prefix_re, ".*" + pattern_re)

To:

pattern_re = "^"+prefix_re+re.escape(os.sep)+".*"+pattern_re

os.path.join is is not always the saviour ;-)
msg116494 - (view) Author: Éric Araujo (eric.araujo) * (Python committer) Date: 2010-09-15 21:59
My feeling was that there was a logic bug in filelist (something explicitly added should not be filtered out by a default exclude pattern), so I’m glad to learn it’s just a regex bug.  I’ll add a test for the add/filter logic just in case.  Thanks a lot for the diagnosis and patch, I will probably commit it on Monday.
msg137840 - (view) Author: Éric Araujo (eric.araujo) * (Python committer) Date: 2011-06-07 15:38
I’m sorry I couldn’t fix this one in 3.1.  I didn’t take the time to download and install a Windows to test this year, and right now I don’t have the bandwidth.  I’ll get to it as soon as possible.
msg153626 - (view) Author: Éric Araujo (eric.araujo) * (Python committer) Date: 2012-02-18 02:04
Can someone test this patch on Windows?
msg153627 - (view) Author: Nadeem Vawda (nadeem.vawda) * (Python committer) Date: 2012-02-18 02:25
With the patch applied, the problem from issue 14004 still occurs. There
is also a failure in test_distutils, but I'm not sure whether that's just
the test itself needing to be updated.

Anyway, it's late for me now; I'll give it a more detailed look tomorrow.
msg153628 - (view) Author: Éric Araujo (eric.araujo) * (Python committer) Date: 2012-02-18 02:39
Could you post the test output (and the output of DISTUTILS_DEBUG=1 ./python.exe setup.py sdist if you have it)?
msg153657 - (view) Author: Nadeem Vawda (nadeem.vawda) * (Python committer) Date: 2012-02-18 20:50
Test output:

    test test_distutils failed -- Traceback (most recent call last):
      File "C:\Users\Nadeem\Code\python2\python27\lib\distutils\tests\test_filelist.py", line 230, in test_process_template
        self.assertEqual(file_list.files, ['d/b.py', 'd/d/e.py'])
    AssertionError: Lists differ: [] != ['d/b.py', 'd/d/e.py']

    Second list contains 2 additional elements.
    First extra element 0:
    d/b.py

    - []
    + ['d/b.py', 'd/d/e.py']

Output of sdist:

    C:\Users\Nadeem\Code\keyring>..\python2\python27\PCbuild\python_d setup.py sdist

    C:\Users\Nadeem\Code\python2\python27\lib\distutils\dist.py:267: UserWarning: Unknown distribution option: 'extras_require'
      warnings.warn(msg)
    options (after parsing config files):
    options (after parsing command line):
    option dict for 'sdist' command:
      {}
    running sdist
    Distribution.get_command_obj(): creating 'sdist' command object
    running check
    Distribution.get_command_obj(): creating 'check' command object
    Distribution.get_command_obj(): creating 'build_py' command object
    Distribution.get_command_obj(): creating 'build' command object
    reading manifest template 'MANIFEST.in'
[... output omitted - applying other regexes ...]
    include *.txt
    include_pattern: applying regex r'^[^/]*\.txt\Z(?ms)'
     adding CHANGES.txt
     adding CONTRIBUTORS.txt
     adding .hg\last-message.txt
    exclude_pattern: applying regex r'^build\\.*'
    exclude_pattern: applying regex r'^keyring\-0\.8\\.*'
    exclude_pattern: applying regex r'(^|/|\\)(RCS|CVS|\.svn|\.hg|\.git|\.bzr|_darcs)(/|\\).*'
     removing .hg\last-message.txt
    writing manifest file 'MANIFEST'
[... output omitted - creating directories; copying and adding files ...]
    removing 'keyring-0.8' (and everything under it)

As you can see, the "include *.txt" directive still catches ".hg\last-message.txt".

The problem seems to be that while you've fixed the prefix-handling codepath in
translate_pattern(), glob_to_re() is still hardcoded to use "/". I've been able to
get the desired behavior by changing glob_to_re() as follows (note the need to
double-escape os.sep):

         # character except the special characters.
         # XXX currently the "special characters" are just slash -- i.e. this is
         # Unix-only.
    -    pattern_re = re.sub(r'((?<!\\)(\\\\)*)\.', r'\1[^/]', pattern_re)
    +    pattern_re = re.sub(r'((?<!\\)(\\\\)*)\.',
    +                        r'\1[^' + re.escape(re.escape(os.sep)) + ']',
    +                        pattern_re)
    
         return pattern_re

Of course, the comment above should also be updated.

Also, this change results in a different set of test breakages:

    ======================================================================
    FAIL: test_glob_to_re (distutils.tests.test_filelist.FileListTestCase)
    ----------------------------------------------------------------------
    Traceback (most recent call last):
      File "C:\Users\Nadeem\Code\python2\python27\lib\distutils\tests\test_filelist.py", line 42, in test_glob_to_re
        self.assertEqual(glob_to_re('foo*'), 'foo[^/]*\\Z(?ms)')
    AssertionError: 'foo[^\\\\]*\\Z(?ms)' != 'foo[^/]*\\Z(?ms)'

    ======================================================================
    FAIL: test_process_template (distutils.tests.test_filelist.FileListTestCase)
    ----------------------------------------------------------------------
    Traceback (most recent call last):
      File "C:\Users\Nadeem\Code\python2\python27\lib\distutils\tests\test_filelist.py", line 182, in test_process_template
        self.assertEqual(file_list.files, ['a.py'])
    AssertionError: Lists differ: ['a.py', 'd/c.py'] != ['a.py']

    First list contains 1 additional elements.
    First extra element 1:
    d/c.py

    - ['a.py', 'd/c.py']
    + ['a.py']

These are both clearly due to the tests using hardcoded Unix paths instead
of os.sep and os.path.join().
msg153687 - (view) Author: Éric Araujo (eric.araujo) * (Python committer) Date: 2012-02-19 09:38
It’s strange that glob_to_re needs fixing too; at this point I’m not sure if we’re still fixing my commit or if the original code never worked properly on Windows.  Could you run the same tests with a Python < 2.7?

BTW I wonder if #9691 is yet another manifestation of this bug.
msg153698 - (view) Author: Nadeem Vawda (nadeem.vawda) * (Python committer) Date: 2012-02-19 10:56
> It’s strange that glob_to_re needs fixing too; at this point I’m not sure if we’re still fixing my commit or if the original code never worked properly on Windows.  Could you run the same tests with a Python < 2.7?

The original code never worked properly on Windows - that's what the XXX
comment in glob_to_re() is about. I've been able to reproduce the problem
(erroneously including ".hg\last-message.txt") on v2.7.2 (8527427914a2).

> BTW I wonder if #9691 is yet another manifestation of this bug.

It looks like it.
msg153728 - (view) Author: Éric Araujo (eric.araujo) * (Python committer) Date: 2012-02-19 22:34
> The original code never worked properly on Windows - that's what the XXX comment in glob_to_re() is
> about. I've been able to reproduce the problem (erroneously including ".hg\last-message.txt") on
> v2.7.2 (8527427914a2).
Can you reproduce the “include buildout.cfg” bug too?  (2.7.2 doesn’t include neither Antoine’s changeset nor mine, right?)

(Want another one?  #1702551  Blergh)
msg153781 - (view) Author: Nadeem Vawda (nadeem.vawda) * (Python committer) Date: 2012-02-20 12:58
> (2.7.2 doesn’t include neither Antoine’s changeset nor mine, right?)

Correct; it dates back to June last year, before issue 13193 was filed.


> Can you reproduce the “include buildout.cfg” bug too?

Yes, that problem is present in 2.7.2, but not in the current revision of
the 2.7 branch. In 2.7.2, the offending regex is r'^build\.*' (since
translate_pattern() uses os.path.join, and doesn't escape the separator),
whereas in 2.7-head the regex is r'^build/.*' (using a hardcoded "/").
2.7-head with your patch applied gives r'^build\\.*' (the correct value).

In this case (unlike #9691 and #14004), the problem was fixed by the
changes in issue 13193, since the offending code is in the
"if prefix is not None:" codepath of translate_pattern(), rather than in
glob_to_re().


While we're talking about translate_pattern(), I have a question about
this line:

    pattern_re = "^%s/.*%s" % (prefix_re, pattern_re)

It seems that if you specify both a prefix and a (nonempty) pattern, the
resulting regexp is something like "<PREFIX>/.*<PATTERN>", using the
pattern argument as a suffix. Is this the intended behavior? I would have
expected something like "<PREFIX>/(.*/)?<PATTERN>", defaulting to
"<PREFIX>/.*" if pattern is empty. For example, it seems wrong that
"recursive-include foo bar.*" matches foo/test_bar.py.
msg154119 - (view) Author: Éric Araujo (eric.araujo) * (Python committer) Date: 2012-02-24 07:22
Thanks for testing and confirming that this code never worked.  I am now confident that we’re about to fix once and for all this basket of bugs in the correct way.

I updated the tests to make them hopefully pass on Windows, and while doing that I realized that I don’t like the unnecessary escaping of / on posix systems.  Regexes already look like grit on my monitor, so I prefer not making them harder to read.  My latest patch only escapes if os.sep is the backslash.

Should apply cleanly to 2.7, please test.


> While we're talking about translate_pattern(), I have a question about this line:
> [snip]
> For example, it seems wrong that "recursive-include foo bar.*" matches foo/test_bar.py.
Certainly.  Feel free to open another report for that, or I’ll do it.
msg154120 - (view) Author: Éric Araujo (eric.araujo) * (Python committer) Date: 2012-02-24 07:27
Escalated to release blocker, as bugfix releases are about to go out of the door and I’d prefer they weren’t published with the buggy fixes committed for #13193.
msg154136 - (view) Author: Nadeem Vawda (nadeem.vawda) * (Python committer) Date: 2012-02-24 13:41
There were bugs in two of the updated tests:

- test_glob_to_re() was doing two levels of escaping (r'\' -> r'\\\\')
  for its expected output when it should only do one (r'\' -> r'\\').

- test_process_template() was not converting some of its expected
  results to use the native directory separator.

I've attached a patch that fixes these issues. I also took the liberty of
changing the checks for whether the separator needs to be escaped - it's
better to escape everything except "/", just in case someone decides to
port Python to some platform using a weird directory separator that is
neither "/" nor r"\".

With my updated patch, all tests pass on both Windows and Linux, and I've
also manually verified that the buildout.cfg problem and the
sandbox/dummy.py problem (from issue 9691) are both fixed.

Please review (and commit, if my changes are OK).
msg154190 - (view) Author: Éric Araujo (eric.araujo) * (Python committer) Date: 2012-02-25 06:41
> - test_glob_to_re() was doing two levels of escaping (r'\' -> r'\\\\')
>   for its expected output when it should only do one (r'\' -> r'\\').
Fix merged.  I don’t fully understand why one place needs two escapes and the others just one.

> - test_process_template() was not converting some of its expected
>   results to use the native directory separator.
Overlooked paths fixed.

> I also took the liberty of changing the checks for whether the separator needs to be escaped
> - it's better to escape everything except "/", just in case someone decides to port Python to
> some platform using a weird directory separator that is neither "/" nor r"\".
I didn’t take that part.  If someone wants to port Python with a new style of os.sep or os.extsep, I’ll deal with it when they report that.

The buildout.cfg and VCS dirs issues are covered by unit tests for both filelist and sdist, but thanks for testing manually with a real setup.py too.  The third problem you mention (inclusion of unwanted module in a subdir) is also covered by test_filelist.

On a related subject, the code in sdist that excludes VCS dirs uses r'/|\\'; I thought that os.altsep was an alternate *accepted* separator, but not actually produced by OS calls.  IOW, I’m asking if os.walk (used to create filelist.allfiles) can ever give paths with '/' on Windows.  If not, then the regex is redundant, and I can clean it in packaging.  My patch removes the redundancy, just for curiosity, but I won’t actually commit that part to distutils.
msg154204 - (view) Author: Nadeem Vawda (nadeem.vawda) * (Python committer) Date: 2012-02-25 08:07
There are some minor errors in your v3 patch. I've attached a v4 that fixes
them (as usual, tested on Windows and Linux):

 - s/special/sep/ in glob_to_re()
 - Add missing l(.) to filenames in test_process_template under 'graft d'


>> - test_glob_to_re() was doing two levels of escaping (r'\' -> r'\\\\')
>>   for its expected output when it should only do one (r'\' -> r'\\').
> Fix merged.  I don’t fully understand why one place needs two escapes and the others just one.

The places that use one level of escaping are the ones that deal with the
regex string directly. In glob_to_re() itself, the string you're building
is a regex replacement pattern that *operates on* the regex that gets
returned, so it needs one level of escaping for when the output regex is
actually applied, and a second one for the immediate call to re.sub().
When re.sub is called, it eats one level of the escaping, and operates on
a string with one level, returning a string with one level.


>> I also took the liberty of changing the checks for whether the separator needs to be escaped
>> - it's better to escape everything except "/", just in case someone decides to port Python to
>> some platform using a weird directory separator that is neither "/" nor r"\".
> I didn’t take that part.  If someone wants to port Python with a new style of os.sep or os.extsep, I’ll deal with it when they report that.

If you want to leave it as it is, that's your choice. But I do think it's
better to make this Do The Right Thing now, given how easy it is.


> On a related subject, the code in sdist that excludes VCS dirs uses r'/|\\'; I thought that os.altsep was an alternate *accepted* separator, but not actually produced by OS calls.  IOW, I’m asking if os.walk (used to create filelist.allfiles) can ever give paths with '/' on Windows.  If not, then the regex is redundant, and I can clean it in packaging.  My patch removes the redundancy, just for curiosity, but I won’t actually commit that part to distutils.

Well, as far as I can make out, the current implementation always uses
r'\' to join paths. But it won't convert existing '/'s in the start
path to r'\'s, so you might still end up encountering both separators
(assuming the initial path is something you get from the user somewhere
along the line).

(These remarks are not specific to os.walk, but are relevant to anything
based on os.path.join - for example, distutils.filelist.findall and
packaging.manifest._findall)
msg154207 - (view) Author: Nadeem Vawda (nadeem.vawda) * (Python committer) Date: 2012-02-25 08:10
>> - test_glob_to_re() was doing two levels of escaping (r'\' -> r'\\\\')
>>   for its expected output when it should only do one (r'\' -> r'\\').
> Fix merged.  I don’t fully understand why one place needs two escapes and the others just one.

I agree that this code is confusing, though. Perhaps we should add a
comment in glob_to_re():

    # Warning - manipulating a regex with a regex. Here be dragons.

;-)
msg154208 - (view) Author: Éric Araujo (eric.araujo) * (Python committer) Date: 2012-02-25 08:21
Thanks a bunch.  I’ll offer you cookies when we meet :)

>> Fix merged.  I don’t fully understand why one place needs two escapes and the others just one.
> The places that use one level of escaping are the ones that deal with the regex string directly.
> In glob_to_re() itself, the string you're building is a regex replacement pattern that *operates on*
> the regex that gets returned
Makes sense.  I’ve added a comment as you suggested (although an unfunny one, without dragons; I don’t like “you’re not supposed to understand this”-like comments).

>>> I also took the liberty of changing the checks for whether the separator needs to be escaped
>>> - it's better to escape everything except "/", just in case someone decides to port Python to
>>> some platform using a weird directory separator that is neither "/" nor r"\".
>> I didn’t take that part.  If someone wants to port Python with a new style of os.sep or os.extsep,
>> I’ll deal with it when they report that.
> If you want to leave it as it is, that's your choice. But I do think it's better to make this Do The
> Right Thing now, given how easy it is.
I’d be okay with some cleanup in packaging, but for distutils I want the minimal patch.

> Well, as far as I can make out, the current implementation always uses r'\' to join paths. But it
> won't convert existing '/'s in the start path to r'\'s, so you might still end up encountering both
> separators (assuming the initial path is something you get from the user somewhere along the line).
Hm, it should come from the manifest template, i.e. using '/' that get translated to os.sep.  I’ll remove the regex redundancy in packaging, we have rather good tests for manifest and sdist now.
msg154211 - (view) Author: Nadeem Vawda (nadeem.vawda) * (Python committer) Date: 2012-02-25 08:33
>>> Fix merged.  I don’t fully understand why one place needs two escapes and the others just one.
>> The places that use one level of escaping are the ones that deal with the regex string directly.
>> In glob_to_re() itself, the string you're building is a regex replacement pattern that *operates on*
>> the regex that gets returned
> Makes sense.  I’ve added a comment as you suggested (although an unfunny one, without dragons; I don’t like “you’re not supposed to understand this”-like comments).

Hmm yeah, I had just meant the dragons bit as a joke... I wouldn't want
people to get the impression that the code should be treated as magic.


>>> I didn’t take that part.  If someone wants to port Python with a new style of os.sep or os.extsep,
>>> I’ll deal with it when they report that.
>> If you want to leave it as it is, that's your choice. But I do think it's better to make this Do The
>> Right Thing now, given how easy it is.
> I’d be okay with some cleanup in packaging, but for distutils I want the minimal patch.

That's fine by me.


>> Well, as far as I can make out, the current implementation always uses r'\' to join paths. But it
>> won't convert existing '/'s in the start path to r'\'s, so you might still end up encountering both
>> separators (assuming the initial path is something you get from the user somewhere along the line).
> Hm, it should come from the manifest template, i.e. using '/' that get translated to os.sep.  I’ll remove the regex redundancy in packaging, we have rather good tests for manifest and sdist now.

I was thinking that maybe findall()'s 'dir' argument could be specified
by the user as a command-line flag or something. But looking through
the sdist code, it seems that it always uses the current directory. So
your change shouldn't be a problem.
msg154235 - (view) Author: Georg Brandl (georg.brandl) * (Python committer) Date: 2012-02-25 10:44
Be sure to notify me when you have committed a fix, as changes made now in the 3.2 branch will *not* show up in the final release.
msg154252 - (view) Author: Éric Araujo (eric.araujo) * (Python committer) Date: 2012-02-25 14:51
I’ll commit the 2.7 patch, see how buildbots fare, then commit the 3.2 version (also attached in case someone wants to test it).
msg154255 - (view) Author: Nadeem Vawda (nadeem.vawda) * (Python committer) Date: 2012-02-25 15:11
3.2 patch looks good on Windows.
msg154259 - (view) Author: Roundup Robot (python-dev) Date: 2012-02-25 15:14
New changeset 47788c90f80b by Éric Araujo in branch '2.7':
Fix long-standing bugs with MANIFEST.in parsing on Windows (#6884).
http://hg.python.org/cpython/rev/47788c90f80b
msg154264 - (view) Author: Roundup Robot (python-dev) Date: 2012-02-25 15:32
New changeset 73aa4c9305b3 by Éric Araujo in branch '3.2':
Fix long-standing bugs with MANIFEST.in parsing on Windows (#6884).
http://hg.python.org/cpython/rev/73aa4c9305b3
msg154270 - (view) Author: Éric Araujo (eric.araujo) * (Python committer) Date: 2012-02-25 16:35
Most buildbots are green, one has a test_subprocess failure, others are still building, but I have to go soon.  Release managers, please graft this fix when the bots are all green.
msg154304 - (view) Author: Roundup Robot (python-dev) Date: 2012-02-26 03:07
New changeset 937b1613d0e6 by Éric Araujo in branch 'default':
Port the #6884 fix to packaging
http://hg.python.org/cpython/rev/937b1613d0e6
msg154308 - (view) Author: Éric Araujo (eric.araujo) * (Python committer) Date: 2012-02-26 03:37
I’ve ported the fix to packaging without testing, the buildbots will shout at me if I made a mistake.  For distutils2 however I have no tester and no bot, so could someone test this patch?  (repo is at http://hg.python.org/distutils2)

BTW if someone knows about a continuous integration service which provides Windows and Mac OS X VMs I’m all ears.
msg154318 - (view) Author: Nadeem Vawda (nadeem.vawda) * (Python committer) Date: 2012-02-26 09:35
> For distutils2 however I have no tester and no bot, so could someone test this patch?

I'll take a look at it.


> BTW if someone knows about a continuous integration service which provides Windows and Mac OS X VMs I’m all ears.

Not sure about a CI service, but if you want Windows for your own machine
I believe you can get a free MSDN subscription:

    http://mail.python.org/pipermail/python-committers/2011-August/001788.html
msg154328 - (view) Author: Adam Groszer (Adam.Groszer) Date: 2012-02-26 11:37
Hello,

On Sun, 26 Feb 2012 03:37:06 +0000 you wrote:

> BTW if someone knows about a continuous integration service which provides Windows and Mac OS X VMs I’m all ears.

ZTK (Zope toolkit) ended up with renting a windows (server) VM.
http://winbot.zope.org/
There we run a buildbot that does the windows releases of some packages 
(zope.interface, zope.proxy, etc) and runs the tests.
msg154390 - (view) Author: Nadeem Vawda (nadeem.vawda) * (Python committer) Date: 2012-02-26 21:09
Well... testing distutils2 on Windows has been quite an adventure. When
I ran the test suite before applying the patch, I got a bunch of failures
on all of the Python versions I tested (except the ones that had issues
preventing me from running the tests at all). Only one of the failures
appears to be due to the current issue.

A summary of the problems, broken down by Python version:


[python3 branch]

- 3.3/3.2: Two failures:

   * One in test_command_build_ext..test_finalize_options where the
     library search path erroneously includes some additional directories
     under the Python installation directory. This failure is not fixed
     by the patch; it continues to fail after the patch is applied.

   * One in test_manifest that looks like this:

        FAIL: test_process_template (distutils2.tests.test_manifest.ManifestTestCase)
        ----------------------------------------------------------------------
        Traceback (most recent call last):
          File "C:\Users\Nadeem\Code\distutils2\distutils2\tests\test_manifest.py", line 215, in test_process_template
            self.assertEqual(manifest.files, ['d/b.py', 'd/d/e.py'])
        AssertionError: Lists differ: [] != ['d/b.py', 'd/d/e.py']

    This one is clearly caused by the whole directory separator regex
    snafu we're trying to fix. Applying the patch fixes it, and doesn't
    introduce any new failures.

- 3.1: Same as above, plus a failure in test_util:

        FAIL: test_generate_setup_py (distutils2.tests.test_util.UtilTestCase)
        ----------------------------------------------------------------------
        Traceback (most recent call last):
          File "C:\Users\Nadeem\Code\distutils2\distutils2\tests\test_util.py", line 557, in test_generate_setup_py
            self.assertEqual(out, b'SPAM\n')
        AssertionError: b'SPAM\r\n' != b'SPAM\n'

     Like the test_finalize_options failure, this isn't fixed by the patch.


[default branch]

- 2.7: Before the tests start running, runtests.py crashes with an
       ImportError complaining that it can't find the winreg module.
       AFAIK the registry-handling module in the stdlib is called _winreg
       in 2.x, and was only renamed to winreg in 3.x.

- 2.6: Same as 2.7.

- 2.5: Twenty (20) failures, including the three I get on 3.1, some
       miscellaneous stuff that looks registry-related, and around a
       dozen errors complaining about non-ASCII characters in
       bdist_msi.py's copyright notice. As above, your patch fixes
       the one failure in test_manifest, but none of the others.

- 2.4: Can't run the tests; needs Visual C++ 2003, which (AFAICT) isn't
       freely available.


TL;DR summary: The patch itself looks good, but D2 and Windows currently
don't get along at all.

I'd like to file a separate bug for the unrelated failures - do I do that
here, or does distutils2 have a separate bugtracker/mailing list?
msg154447 - (view) Author: Éric Araujo (eric.araujo) * (Python committer) Date: 2012-02-27 09:25
[Nadeem]
> Not sure about a CI service, but if you want Windows for your own machine
> I believe you can get a free MSDN subscription:
Yep, I have one of these and downloaded an ISO, but I haven’t yet managed to set it up with qemu.

[Adam]
> ZTK (Zope toolkit) ended up with renting a windows (server) VM. http://winbot.zope.org/
> There we run a buildbot that does the windows releases of some packages
OK.  Too bad for me then.


Nadeem, thanks for your report.  We used to have one or two people working on distutils2 on Windows, but they got busy with other things or I scared them off, and now I just have to hope that my commits work, which clearly doesn’t work.  I’ll send a call for testers on some mailing lists.

Newlines are a particular sore point.  In many cases we don’t care about them and happily use splitlines to ignore platform differences; in some cases we really want LF on all platforms but there are no tests.

Please file reports for the bugs you’ve found, on this tracker.  Set the “Distutils2” component and I’ll be auto-nosy.
msg154454 - (view) Author: Nadeem Vawda (nadeem.vawda) * (Python committer) Date: 2012-02-27 11:26
> Yep, I have one of these and downloaded an ISO, but I haven’t yet managed to set it up with qemu.

You might want to give VirtualBox a try - I've found it very easy to set up.


> Please file reports for the bugs you’ve found, on this tracker.

Will do.
msg154460 - (view) Author: Roundup Robot (python-dev) Date: 2012-02-27 11:29
New changeset 4d6a9f287543 by Éric Araujo in branch 'default':
Fix bugs with MANIFEST.in parsing on Windows (#6884).
http://hg.python.org/distutils2/rev/4d6a9f287543

New changeset 7d1a7251d771 by Éric Araujo in branch 'python3':
Merge fixes for #13974, #6884 and #11841 from default
http://hg.python.org/distutils2/rev/7d1a7251d771
msg154463 - (view) Author: Éric Araujo (eric.araujo) * (Python committer) Date: 2012-02-27 11:32
I’ve been recommended VirtualBox, but I don’t have Qt libs on my machine (I don’t have much disk space and I don’t like Qt :)

Happily closing as fixed.
msg156268 - (view) Author: Roundup Robot (python-dev) Date: 2012-03-18 19:40
New changeset edcdef70c44e by Éric Araujo in branch '3.2':
Fix long-standing bugs with MANIFEST.in parsing on Windows (#6884).
http://hg.python.org/cpython/rev/edcdef70c44e
msg157878 - (view) Author: Roundup Robot (python-dev) Date: 2012-04-09 19:06
New changeset b5f0ce4ddf0c by Éric Araujo in branch '2.7':
Fix long-standing bugs with MANIFEST.in parsing on Windows (#6884).
http://hg.python.org/cpython/rev/b5f0ce4ddf0c
History
Date User Action Args
2012-04-09 19:06:00python-devsetmessages: + msg157878
2012-03-18 19:40:11python-devsetmessages: + msg156268
2012-02-27 11:32:15eric.araujosetstatus: open -> closed

messages: + msg154463
2012-02-27 11:29:48python-devsetmessages: + msg154460
2012-02-27 11:26:45nadeem.vawdasetmessages: + msg154454
2012-02-27 09:25:17eric.araujosetmessages: + msg154447
2012-02-26 21:09:07nadeem.vawdasetmessages: + msg154390
2012-02-26 11:37:20Adam.Groszersetmessages: + msg154328
2012-02-26 09:35:59nadeem.vawdasetmessages: + msg154318
2012-02-26 03:37:04eric.araujosetfiles: + fix-distutils2-manifest-bugs.diff

messages: + msg154308
2012-02-26 03:07:49python-devsetmessages: + msg154304
2012-02-25 16:35:32eric.araujosetresolution: fixed
messages: + msg154270
stage: patch review -> resolved
2012-02-25 15:32:26python-devsetmessages: + msg154264
2012-02-25 15:14:01python-devsetnosy: + python-dev
messages: + msg154259
2012-02-25 15:11:22nadeem.vawdasetmessages: + msg154255
2012-02-25 14:51:50eric.araujosetfiles: + filelist-regex-bugs-3.2.diff

messages: + msg154252
2012-02-25 10:44:05georg.brandlsetmessages: + msg154235
2012-02-25 08:33:20nadeem.vawdasetmessages: + msg154211
2012-02-25 08:21:42eric.araujosetmessages: + msg154208
2012-02-25 08:10:22nadeem.vawdasetmessages: + msg154207
2012-02-25 08:07:24nadeem.vawdasetfiles: + filelist-regex-v4.diff

messages: + msg154204
2012-02-25 06:41:02eric.araujosetfiles: + filelist-regex-bugs-v3.diff

messages: + msg154190
2012-02-24 14:47:24nadeem.vawdalinkissue14106 dependencies
2012-02-24 13:41:45nadeem.vawdasetfiles: + filelist-regex-v2.diff

messages: + msg154136
2012-02-24 07:27:39eric.araujosetpriority: normal -> release blocker
nosy: + georg.brandl, benjamin.peterson
messages: + msg154120

2012-02-24 07:22:27eric.araujosetfiles: + filelist-regex-bugs.diff
resolution: accepted -> (no value)
messages: + msg154119
2012-02-20 12:58:27nadeem.vawdasetmessages: + msg153781
2012-02-19 22:34:06eric.araujosetmessages: + msg153728
2012-02-19 11:00:57nadeem.vawdalinkissue9691 superseder
2012-02-19 10:56:32nadeem.vawdasetmessages: + msg153698
2012-02-19 09:38:43eric.araujosetmessages: + msg153687
2012-02-18 20:50:06nadeem.vawdasetmessages: + msg153657
2012-02-18 02:39:06eric.araujosetmessages: + msg153628
2012-02-18 02:25:44nadeem.vawdasetmessages: + msg153627
2012-02-18 02:04:22eric.araujosetfiles: + filelist-regex-bug.diff
nosy: + jason.coombs
messages: + msg153626

2012-02-18 02:01:53nadeem.vawdasetnosy: + nadeem.vawda
2012-02-18 01:49:16eric.araujolinkissue14004 superseder
2011-06-07 15:38:33eric.araujosetstatus: pending -> open

messages: + msg137840
versions: + Python 3.3, - Python 3.1
2010-10-19 20:12:47eric.araujolinkissue10146 superseder
2010-09-15 21:59:58eric.araujosetstatus: open -> pending
keywords: + patch, - needs review
resolution: accepted
stage: needs patch -> patch review
2010-09-15 21:59:03eric.araujosetmessages: + msg116494
2010-09-15 15:50:05cjw296setkeywords: + needs review, - patch

messages: + msg116461
2010-09-15 15:24:29cjw296setmessages: + msg116457
2010-09-14 23:07:29cjw296setmessages: + msg116424
2010-08-13 23:22:08eric.araujosetmessages: + msg113850
stage: test needed -> needs patch
2010-08-13 22:20:58eric.araujosetfiles: + test-buildout-inclusion.diff
keywords: + patch
messages: + msg113841
2010-08-12 15:51:27eric.araujosetassignee: tarek -> eric.araujo
messages: + msg113687
versions: + Python 3.1, Python 3.2, - Python 2.6
2010-07-21 13:15:44eric.araujosetnosy: + eric.araujo
messages: + msg111049
2010-06-18 19:54:03tareksetcomponents: + Distutils2
versions: - Python 2.5
2010-06-18 11:16:49cjw296setmessages: + msg108099
2010-06-17 12:29:21Adam.Groszersetversions: + Python 2.5
nosy: + Adam.Groszer

messages: + msg108005

components: + Distutils
2009-09-11 14:28:27cjw296create