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

Add tox.ini to sdist #1743

Closed
wants to merge 1 commit into from
Closed

Conversation

mtelka
Copy link

@mtelka mtelka commented Nov 26, 2023

No description provided.

@Byron
Copy link
Member

Byron commented Nov 26, 2023

Thanks!

Could you add a little rationale? What's it good for, or which problem is it solving?

In general I see sdist as a source distribution to be enough to run the software, but if that's not what sdist is supposed to be by convention, more files might have to be added to the manifest to be less surprising.

@mtelka
Copy link
Author

mtelka commented Nov 27, 2023

AFAIK, there is no specified nor fully agreed what should go to sdist. Some projects are very strict and refuse to add anything that is not needed to run software (e.g. tests or documentation), other projects add tests and/or documentation intentionally. Some projects do not care much and leave this on their tooling.

Some build backends adds tests automatically, some also adds tox.ini. It really varies and evolves as time goes. setuptools seems to be one of most conservative backends and it does not add tox.ini to sdist by default (yet). But it adds tests, if it is able to find them automatically.

Why do I need tox.ini in sdist?

I'm packaging GitPython for OpenIndiana and during packaging we run tests. tox.ini in sdist is best way how to detect and run everything automatically because we can get the sdist URL directly from PyPI, then tox.ini allows us to obtain all test dependencies in an uniform way so we can install them automatically. And finally, tox.ini allows simple and uniform invocation of tests.

If tox.ini is not in sdist then we need some additional manual work during the packaging and this is both boring and error prone and thus increases maintenance costs.

Thank you.

@Byron
Copy link
Member

Byron commented Nov 27, 2023

Thanks for the explanation!

That's interesting as well as the manifest does recursive-exclude tests, so there should be nothing to run with tox. Further, tests are very finicky and currently do require the paren repository to work (maybe related: #1744).

Thus I would think merging this PR won't help in running tests for the purpose of packaging, particularly since these appear to be excluded.

@Byron
Copy link
Member

Byron commented Nov 27, 2023

Closing in the hopes to have one issue related to the overarching goal of packaging in OpenIndiana, if needed. A repository clone is needed to run tests along with some preparations, and from there it can be package as needed, presumably. Tags are present for each release on pypi as well.

@Byron Byron closed this Nov 27, 2023
@mtelka
Copy link
Author

mtelka commented Nov 27, 2023

You are right. Tests are not included in sdist too. Sorry, I missed that since on first glance I noticed the test-requirements.txt file in the sdist and missing tox.ini there, so I immediately switched to github tarball with my further packaging effort.

We can package from github tarball (as I already did for GitPython 3.1.40 package), but that's a bit inconvenient for us. There is also a risk that github tarball will change without notice causing troubles for reproducible builds.

For the actual testing: yes, I know the special setup is needed. I already do that. Without the pre-test setup almost all tests fails miserably.

Would you consider to accept a PR with added tests and tox.ini (and maybe some other files that I'll find are needed to run tests)?

Thank you.

@Byron
Copy link
Member

Byron commented Nov 27, 2023

It's great to see actual packaging scripts, thanks for sharing. They didn't seem to be dependent or make assumptions about archive hashes, so relying on them should be fine. The underlying machinery of git archive is definitely going to be stable as that's what git assures, so the contents of it will always be according to expectations.

Would you consider to accept a PR with added tests and tox.ini (and maybe some other files that I'll find are needed to run tests)?

Keeping the package size minimal seems more important as mitigating the perceived risk due to the usage of a github tarball, which is why I wouldn't consider such a PR. Thanks for your understanding.

@mtelka
Copy link
Author

mtelka commented Nov 27, 2023

It's great to see actual packaging scripts, thanks for sharing. They didn't seem to be dependent or make assumptions about archive hashes, so relying on them should be fine.

Indeed they do.

Would you consider to accept a PR with added tests and tox.ini (and maybe some other files that I'll find are needed to run tests)?

Keeping the package size minimal seems more important as mitigating the perceived risk due to the usage of a github tarball, which is why I wouldn't consider such a PR. Thanks for your understanding.

Okay, not a big deal. I already learned that some Python projects tend to save up to 300 kB for their sdist because storage space is very expensive these days :-).

@Byron
Copy link
Member

Byron commented Nov 27, 2023

Indeed they do.

I see. Once the hash changes again, I presume the script will fail as the input can't be validated anymore? As the GitHub tarball is generated, it definitely can't be trusted to keep its hash forever. But what can be trusted is that it keeps its content the same, forever, and I'd hope there is a way to validate that hash instead.

Okay, not a big deal. I already learned that some Python projects tend to save up to 300 kB for their sdist because storage space is very expensive these days :-).

To me it's about latency and the chance that something goes wrong which goes up the longer a download takes. Thus it's beneficial to only include in a package that is needed to fulfil its purpose.

The reason I am writing this is to say that I am aware that the needs of packagers are different from the needs of users, and thus far I have seen no package management system that would allow such differentiation. To me it would make perfect sense to also offer a 'complete' package, with all bells and whistles to make it trust-worthy, for use by integrators. Until that is a thing, I suppose tarballs of git-tags or clones of git repositories will have to suffice :/.

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

Successfully merging this pull request may close these issues.

2 participants