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

Missing requirements.txt from sdist #9

Closed
jakirkham opened this issue Apr 11, 2024 · 16 comments
Closed

Missing requirements.txt from sdist #9

jakirkham opened this issue Apr 11, 2024 · 16 comments

Comments

@jakirkham
Copy link

Currently requirements.txt is missing from the sdist on PyPI. As a result building from the sdist fails as it cannot find requirements.txt in setup.py

Would it be possible to add requirements.txt to MANIFEST.in or similar to ensure the sdist includes this file when created?

@jakirkham
Copy link
Author

Here is the CI log with relevant traceback below:

Traceback (most recent call last):
    File "<string>", line 2, in <module>
    File "<pip-setuptools-caller>", line 34, in <module>
    File "/home/conda/staged-recipes/build_artifacts/jinja_partials_1712849747428/work/setup.py", line 16, in <module>
      with open('./requirements.txt', 'r', encoding='utf-8') as fin:
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  FileNotFoundError: [Errno 2] No such file or directory: './requirements.txt'
  error: subprocess-exited-with-error

@mikeckennedy
Copy link
Owner

Hey @jakirkham I just committed a change that I think should fix this issue. Can you double check please? The project literally have one requirement, jinja2, so I figure I can just put it directly into the requirements section for the setup.py.

Also: @Colelyman does the commit I just added (reference just above) fix your problem?

@mikeckennedy
Copy link
Owner

Folks, I'm also considering upgrade this to use pyproject.toml and pure wheels rather than the setup.py. Would that help or hinder your desire to add it to conda (which, thank you!)?

@Colelyman
Copy link

Thanks @mikeckennedy! I think that should fix the problem, to be completely sure a new release would need to be issued.

I can look into other conda packages that use pyproject.toml, and report back.

@mikeckennedy
Copy link
Owner

@Colelyman
Copy link

Thanks! Any chance you would be able to push it to PyPI? The v0.2.0 release from GitHub tags worked, but it was the PyPI v0.2.0 was where the requirements.txt wasn't found.

@mikeckennedy
Copy link
Owner

Hi @Colelyman Yes, I just pushed it. Thanks!

@mikeckennedy
Copy link
Owner

The sdist has the fixed setup.py and I also published the wheel but it doesn't include the setup.py file. Let me know if this causes you any trouble.

@Colelyman
Copy link

Thanks @mikeckennedy, looks like it worked! (See here: conda-forge/staged-recipes#26017)

On a different note, would you like to be added as a maintainer of the package? If so, you can comment something like "I would like to be a maintainer" in the PR and I will add you in.

@jakirkham
Copy link
Author

Thanks Mike! 🙏

That seems like a reasonable fix

Moving to pyproject.toml is a good idea generally as it is more declarative. This works well on the Conda side as well

@mikeckennedy
Copy link
Owner

Great, thanks @jakirkham I already pushed the changes for the pyproject.toml variant. But I left the setup.py there. If you do not need an sdist, then I can remove the setup.py too.

@jakirkham
Copy link
Author

It's still possible to build an sdist with pyproject.toml. Just need to use build's --sdist option

@mikeckennedy
Copy link
Owner

Sounds good. I was just trying to keep anything people may have been working with stable. I'll drop the setup.py eventually. Thanks.

@jakirkham
Copy link
Author

Yeah for sure. Appreciate that Mike

Yeah the sdist is useful on the Conda packaging side as it is a source artifact with a stable checksum

The default GitHub archives are generated on demand. So their checksums can change between downloads. Using sdists from PyPI avoids that issue

@mikeckennedy
Copy link
Owner

Got it. The latest version on PyPI has the fixed sdist we talked about here. Thanks!

@jakirkham
Copy link
Author

Yep the fix works great for us. Thank you! 🙏

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

3 participants