regebro / tzlocal

A Python module that tries to figure out what your local timezone is
MIT License
185 stars 58 forks source link

Symlinks aren't preserved in sdist archive #146

Open mgorny opened 1 year ago

mgorny commented 1 year ago

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 commented 1 year ago

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 commented 1 year ago

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 commented 1 year ago

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 commented 11 months ago

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 commented 11 months ago

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 commented 11 months ago

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