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

Symlinks aren't preserved in sdist archive #146

Open
mgorny opened this issue May 16, 2023 · 6 comments
Open

Symlinks aren't preserved in sdist archive #146

mgorny opened this issue May 16, 2023 · 6 comments

Comments

@mgorny
Copy link

mgorny commented May 16, 2023

The symlinks from the git repository are converted to regular files in sdist archive. As a result, I get a few test failures when running the test suite from it:

FAILED tests/test_tzlocal.py::test_symlink_localtime - AssertionError: assert 'local' == 'Africa/Harare'                               
FAILED tests/test_tzlocal.py::test_conflicting - AssertionError: assert 'localtime is a symlink to: Africa/Harare' in 'Multiple conflic
ting time zone configurations found:\n/tmp/p...
FAILED tests/test_tzlocal.py::test_noconflict - zoneinfo._common.ZoneInfoNotFoundError: 'Multiple conflicting time zone configurations 
found:\n/tmp/portage/dev-python/tzlocal-5.0...

In the git repository:

$ find -type l
./tests/test_data/symlink_localtime/etc/localtime
./tests/test_data/noconflict/usr/share/zoneinfo/Zulu
./tests/test_data/noconflict/usr/share/zoneinfo/UTC
./tests/test_data/noconflict/usr/share/zoneinfo/Etc/UCT
./tests/test_data/noconflict/etc/localtime
./tests/test_data/conflicting/etc/localtime

but:

$ ls -lh tzlocal-5.0.1/tests/test_data/symlink_localtime/etc/localtime
-rw-r--r-- 1 mgorny mgorny 157 05-15 14:10 tzlocal-5.0.1/tests/test_data/symlink_localtime/etc/localtime

I suppose it's a limitation of setuptools. Perhaps it'd be safer to be creating test_data dynamically in a temporary directory, e.g. via a pytest fixture using tmp_path?

@mgorny
Copy link
Author

mgorny commented May 16, 2023

That said, it's not urgent, as we're working this around by using GitHub snapshot archive. However, these aren't exactly stable, so we'd prefer being able to use sdist from PyPI.

Using another PEP517 is also an option. FWICS e.g. hatchling correctly preserves symlinks in sdist.

@regebro
Copy link
Owner

regebro commented May 16, 2023

Oh, I had this problem before and moved the tests outside of the package with the intention of not including them in the sdist in the first place. :-D See #53

But then the tests are explicitly included in the MANIFEST.in? Hmmm.

@mgorny
Copy link
Author

mgorny commented May 16, 2023

Well, we'd actually prefer having them inside the package. Git doesn't guarantee reproducible archives, and we've been recently badly bitten by tons of checksum failures when GitHub upgraded git (then they reverted that because of the fallout).

@regebro
Copy link
Owner

regebro commented Oct 22, 2023

So there is no way to fix the symlinks with setuptools, and I'm not willing to learn another build system at the moment, so making temporary directories is the only real path forward, which seems like a massive overkill, really.

And there is nothing system dependent in the tests outside of windows, so I'm not really sure if you running the test are useful, but I guess it's a matter of principle?

@mgorny
Copy link
Author

mgorny commented Oct 22, 2023

I don't know if it's "useful" either. I don't have any other way of testing whether the package is still going to work month from now, or in the next release of Python… Python stuff gets broken all the time.

@regebro
Copy link
Owner

regebro commented Oct 22, 2023

True, it could theoretically break with a new version of Python, good point.

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

No branches or pull requests

2 participants