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

Since setuptools 28.5.0, a "global-exclude .pyc" no longer matches .pyc files #849

Closed
rbarrois opened this issue Nov 15, 2016 · 4 comments · Fixed by #853
Closed

Since setuptools 28.5.0, a "global-exclude .pyc" no longer matches .pyc files #849

rbarrois opened this issue Nov 15, 2016 · 4 comments · Fixed by #853

Comments

@rbarrois
Copy link

Since commit bb45468, if a project's MANIFEST.in contains global-exclude .py[co], the archive generated with python setup.py sdist will still contain all .pyc / .pyo files.

The test below shows that this line worked in setuptools 28.4.0, and broke in setuptools 28.5.0.

I'm not sure whether this can be considered a bug: the previous implementation looked up patterns without any anchoring, whereas the new one forces users to explicitly include wildcards in their MANIFEST.in.
However, I think this change should have been mentioned in the version Changelog.

diff --cc setuptools/tests/test_manifest.py
index 6360270,ef4beda..0000000
--- a/setuptools/tests/test_manifest.py
+++ b/setuptools/tests/test_manifest.py
@@@ -425,12 -464,22 +425,19 @@@ class TestFileListTest(TempDirTestCase)
          assert file_list.files == ['b.txt']
          self.assertWarnings()
  
+         file_list = FileList()
+         file_list.files = ['a.py', 'b.txt', l('d/c.pyc'), 'e.pyo']
+         file_list.process_template_line('global-exclude .py[co]')
+         file_list.sort()
+         assert file_list.files == ['a.py', 'b.txt']
+         self.assertNoWarnings()
+ 
          # recursive-include
@jaraco
Copy link
Member

jaraco commented Nov 19, 2016

Thanks @rbarrois for tracing the issue and providing a unit test.

I agree this looks like an unintentional regression.

@jaraco
Copy link
Member

jaraco commented Nov 19, 2016

According to the changelog, #764 is implicated in this change. Since the unit tests didn't capture this behavior before, it's no surprise that it was missed during the re-write.

@timheap How difficult would it be to restore this expectation?

mx-moth added a commit to mx-moth/setuptools that referenced this issue Nov 20, 2016
After pypa#764, `global-exclude .pyc` no longer excluded `.pyc` files.
This fixes that regression, and adds a test for this behaviour.
@mx-moth
Copy link
Contributor

mx-moth commented Nov 20, 2016

@jaraco It is simple to restore this behaviour for global-include and global-exclude. I've done so over in #853. Sorry for breaking your build, @rbarrois!

However, I will argue that the old behaviour is unexpected and not desired. In my opinion, a note in the Changelog about this breaking change should be included, rather than restoring the old behaviour. The new behaviour behaves much more like globbing does in other environments (e.g. a bash shell), and is less surprising. With the old behaviour, a pattern like global-exclude bar.py would exclude bar.py, but also surprisingly foo_bar.py, and there is no simple way of stopping this from happening. Under the new behaviour, global-exclude bar.py would only exclude bar.py, and not foo_bar.py. @rbarrois has already found the simple work around to restore the old behaviour by explicitly globbing the match like global-exclude *.py[co].

@rbarrois
Copy link
Author

@timheap thanks! I do agree with you: I wouldn't consider "properly anchoring patterns" as a bug, but rather as a proper bugfix. I'm totally in favor of keeping the fixed behavior, and simply documenting "a longstanding bug has been fixed in passing while cleaning up this code — please fix your manifests.".

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants