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

Fix dists linux #737

Merged
merged 6 commits into from
Jan 5, 2025
Merged

Fix dists linux #737

merged 6 commits into from
Jan 5, 2025

Conversation

tanghaibao
Copy link
Owner

Changes

  • Remove .so from source tarball
  • Name .whl with platform specific tag

Testing

  • On linux, manually tested with pip install on both source tarball and wheel

One caveat, I may have to install hatchling, hatch-vcs, or hatch despite them being in [build-system].

I am not sure if the standalone tarball and wheel, once hosted on PyPI, will be able to trigger installing the dependencies first on the client machine.

@tanghaibao tanghaibao requested a review from Adamtaranto January 4, 2025 16:26
@tanghaibao tanghaibao changed the title Fix tests linux Fix dists linux Jan 4, 2025
Copy link
Collaborator

@Adamtaranto Adamtaranto left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good. I have tested on MacOS and Linux using the conda env YAMLs to set up test envs.

  • Confirmed hatch is the only prerequisite to run hatch build in a clean env.
  • Wheels can be successfully pip installed on the arch they were created for.
  • Tarball can be successfully installed on Mac and Linux.
  • Can do editable install from repo with pip install -e '.[tests]'
  • Built Cython modules are no longer included in the tarball.

All tests pass if using the conda envs (inc bedtools and ImageMagick) on Mac, if on Linux you also need to install ImageMagick manually (have added a note to the readme).

I think that the abi tag in the custom wheel name is incorrect. Please check/fix before merge.

build.py Outdated
platform_tag = sysconfig.get_platform().replace("-", "_").replace(".", "_")
python_version = sysconfig.get_python_version().replace(".", "") # e.g., "310"
python_impl = "cp" # Assuming CPython. Modify if using PyPy or others.
abi_tag = f"{python_impl}{python_version}"
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think the duplicated implementation+version tag "cp312" is incorrect here.

Second instance should be Application Binary Interface (ABI) tag.

See PEP427 and PEP425.

Maybe abi3? I don't know how to automate ABI tag selection.

Example generated wheel names:
jcvi-1.4.25.dev6+gbfef2f34-cp312-cp312-linux_x86_64.whl
jcvi-1.4.25.dev6+gbfef2f34-cp312-cp312-macosx_10_9_x86_64.whl

Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

thanks. Calling a method from wheel package instead (but this adds a dependency at build_system.requires).

README.md Show resolved Hide resolved
build.py Show resolved Hide resolved
@Adamtaranto
Copy link
Collaborator

When installing from PyPi I think pip will first check for compatible wheels and if not found will download the tarball and install/build locally using the instructions in pyproject.toml. In that case pip will fetch the [build-system] deps.

hatch is a separate tool to hatchling.

@tanghaibao tanghaibao merged commit 517f937 into main Jan 5, 2025
13 checks passed
@tanghaibao tanghaibao deleted the fix_tests_linux branch January 5, 2025 04:26
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

Successfully merging this pull request may close these issues.

2 participants