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.

Sign up for GitHub

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 pyproject.toml with build dependencies #958

Open
wants to merge 2 commits into
base: main
Choose a base branch
from
+6 −1
Open

add pyproject.toml with build dependencies #958

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
df984ba
add pyproject.toml with build dependencies
dhellmann May 17, 2024
52224dc
update Makefile step for building sdist to use 'build'
dhellmann May 17, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 3 additions & 1 deletion Makefile
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,10 @@
clean_dist:
rm -rf dist/*

# This step assumes there is already a virtualenv active.
create_dist: clean_dist
python setup.py sdist
python -m pip install build
python -m build --sdist .

upload_package: create_dist
twine upload dist/*
3 changes: 3 additions & 0 deletions pyproject.toml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
[build-system]
requires = ["setuptools", "packaging", "psutil", "ninja", "torch", "wheel"]
Copy link
Contributor

Choose a reason for hiding this comment

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

We've been able to get away with just setuptools, torch and ninja at https://github.com/conda-forge/flash-attn-feedstock/blob/9e6cd984d2d3018ff92b3ad4cbb867641638ba39/recipe/pyproject.toml#L2 when building the flash-attn package for conda-forge.

Suggested change
requires = ["setuptools", "packaging", "psutil", "ninja", "torch", "wheel"]
requires = ["setuptools", "ninja", "torch"]

Does packaging, psutil and wheel need to be declared here? They are declared in setup_requires since #937 FWIW.

Copy link
Contributor

Choose a reason for hiding this comment

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

packaging is used directly in setup.py, so it must be declared here.

Copy link

Choose a reason for hiding this comment

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

Same with psutil and wheel. The wheel package is imported in a top-level import. psutil is imported in a local import unless the user sets MAX_JOBS env var. build-system requires must list all non-stdlib packages that are needed by the build backend or setup.py.

build-backend = "setuptools.build_meta"