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

sdist renames readme.txt to README.txt #100

Closed
bb-migration opened this Issue Nov 10, 2013 · 6 comments

Comments

Projects
None yet
1 participant
@bb-migration

bb-migration commented Nov 10, 2013

Originally reported by: jaraco (Bitbucket: jaraco, GitHub: jaraco)


In svg.charts, an issue was reported that the package fails to install [on Unix systems] because it tries to open 'readme.txt', but the file is named 'README.txt'. However, you can see from the source repo that the file is clearly named 'readme.txt'.

Looking at the setuptools sdist source, it seems that it's matching the name by existence, but not actually reflecting the name of the file as it appears in the file system. This has the effect of essentially causing the file to be renamed as it is added to the sdist.

Setuptools should match the case of the filename as it is found in the system.


@bb-migration

This comment has been minimized.

bb-migration commented Sep 20, 2014

Original comment by rsyring (Bitbucket: rsyring, GitHub: rsyring):


We have the same problem, but for readme.rst.

@bb-migration

This comment has been minimized.

bb-migration commented Sep 20, 2014

Original comment by rsyring (Bitbucket: rsyring, GitHub: rsyring):


I've created pull request #81 with a fix for this.

@bb-migration

This comment has been minimized.

bb-migration commented Sep 24, 2014

Original comment by jaraco (Bitbucket: jaraco, GitHub: jaraco):


I've merge the fix (committed as fc7fd6acb2f5 and c51f56ff7653). Upon further consideration, I wonder if this is the best approach. It makes setuptools more restrictive, which in some sense is a good thing, but in another sense violates the Robustness principle. It's not liberal about allowing unusual filenames. More to the point, I believe it's inconsistent with the behavior about distutils 'include' directives. If one were to "include" FOO*, I would expect it to include 'foo.txt' as 'foo.txt' on Windows, omit it on Unix, and never rename it.

I can't quite put my finger on the issue, but when I write up the changelog entry, I'm a little uneasy about the subtle incompatibility this introduces for some Windows projects.

@bb-migration

This comment has been minimized.

bb-migration commented Sep 24, 2014

Original comment by rsyring (Bitbucket: rsyring, GitHub: rsyring):


If one were to "include" FOO*, I would expect it to include 'foo.txt' as 'foo.txt' on Windows, omit it on Unix, and never rename it.

Just curious, but is that how it works? I can test if needed, but thought I would ask first.

FWIW, I avoided this approach initially because I wasn't exactly sure how to get the real name of the file with it's natural case. Looking through the available python docs didn't lead to an immediately obvious answer. If you would rather have the functionality changed to work as you suggest above, I can put some effort into figuring it out. I just thought the BC issue was minor and I knew how to fix it this way. :)

One other thought, is it better to follow the robustness principle or better to have the same package built? If we follow the suggestion above, building an sdist on Windows and Linux will produce different packages depending on case name of some files. Is that how it works in other places in setuptools? If so, I guess I'd agree that it would be better to follow that idiom than introduce the BC problem.

Either way, just let me know if you would like me to work on a fix to this.

@bb-migration

This comment has been minimized.

bb-migration commented Sep 24, 2014

Original comment by jaraco (Bitbucket: jaraco, GitHub: jaraco):


I actually have settled that your implementation is probably preferable. I agree it would be better to provide a consistent experience across platforms. The more I thought about it, the more I liked your approach.

FWIW, I did discover that using glob.glob will resolve a name to its natural case:

>>> glob.glob('README.TXT')
['README.txt']
@bb-migration

This comment has been minimized.

bb-migration commented Sep 25, 2014

Original comment by rsyring (Bitbucket: rsyring, GitHub: rsyring):


Great, thanks for the feedback. Knowing that glob will do that is helpful.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment