-
Notifications
You must be signed in to change notification settings - Fork 2.4k
Fix building source distributions #6381
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
Conversation
|
HI @mgorny I think we should target something like this for next release 3.4 . As per our established criteria: https://github.com/triton-lang/triton/blob/main/RELEASE.md#release-cherry-pick-criteria we do not allow Feature work as cherry-pick for release 3.3. Also would be nice to add some unit tests to cover for these changes. |
|
Hi @mgorny to your point: Do we want to include documentation and tests as well, or just the bare essentials needed to build wheels? I believe yes, we want to include documentation and tests changes |
|
hi @mgorny we should also temporary enable upload artifact to github on PR so we can actually download both the wheels and sdist for testing. |
I think I've done that. Could you double-check? |
|
Okay, now I'm really satisfied with it. Confirmed that The difference between git archive and source distribution is: @@ -1,35 +1,10 @@
triton-3.3.0+git9fc4181c/
-triton-3.3.0+git9fc4181c/.clang-format
-triton-3.3.0+git9fc4181c/.editorconfig
-triton-3.3.0+git9fc4181c/.git-blame-ignore-revs
-triton-3.3.0+git9fc4181c/.github/
-triton-3.3.0+git9fc4181c/.github/CODEOWNERS
-triton-3.3.0+git9fc4181c/.github/ISSUE_TEMPLATE/
-triton-3.3.0+git9fc4181c/.github/ISSUE_TEMPLATE/bug.yml
-triton-3.3.0+git9fc4181c/.github/ISSUE_TEMPLATE/config.yml
-triton-3.3.0+git9fc4181c/.github/ISSUE_TEMPLATE/performance.yml
-triton-3.3.0+git9fc4181c/.github/PULL_REQUEST_TEMPLATE.md
-triton-3.3.0+git9fc4181c/.github/dependabot.yml
-triton-3.3.0+git9fc4181c/.github/workflows/
-triton-3.3.0+git9fc4181c/.github/workflows/create_release.yml
-triton-3.3.0+git9fc4181c/.github/workflows/documentation.yml
-triton-3.3.0+git9fc4181c/.github/workflows/integration-tests.yml
-triton-3.3.0+git9fc4181c/.github/workflows/integration-tests.yml.in
-triton-3.3.0+git9fc4181c/.github/workflows/llvm-build.yml
-triton-3.3.0+git9fc4181c/.github/workflows/llvm-build/
-triton-3.3.0+git9fc4181c/.github/workflows/llvm-build/almalinux.Dockerfile
-triton-3.3.0+git9fc4181c/.github/workflows/llvm-build/centos.Dockerfile
-triton-3.3.0+git9fc4181c/.github/workflows/test-backends.yml
-triton-3.3.0+git9fc4181c/.github/workflows/wheels.yml
-triton-3.3.0+git9fc4181c/.gitignore
-triton-3.3.0+git9fc4181c/.pre-commit-config.yaml
triton-3.3.0+git9fc4181c/CMakeLists.txt
-triton-3.3.0+git9fc4181c/CONTRIBUTING.md
triton-3.3.0+git9fc4181c/LICENSE
triton-3.3.0+git9fc4181c/MANIFEST.in
triton-3.3.0+git9fc4181c/Makefile
+triton-3.3.0+git9fc4181c/PKG-INFO
triton-3.3.0+git9fc4181c/README.md
-triton-3.3.0+git9fc4181c/RELEASE.md
triton-3.3.0+git9fc4181c/bin/
triton-3.3.0+git9fc4181c/bin/CMakeLists.txt
triton-3.3.0+git9fc4181c/bin/RegisterTritonDialects.h
@@ -464,18 +439,15 @@ triton-3.3.0+git9fc4181c/python/triton/t
triton-3.3.0+git9fc4181c/python/triton/tools/experimental_descriptor.py
triton-3.3.0+git9fc4181c/python/triton/tools/link.py
triton-3.3.0+git9fc4181c/python/triton/tools/mxfp.py
-triton-3.3.0+git9fc4181c/python/tutorials/
-triton-3.3.0+git9fc4181c/python/tutorials/01-vector-add.py
-triton-3.3.0+git9fc4181c/python/tutorials/02-fused-softmax.py
-triton-3.3.0+git9fc4181c/python/tutorials/03-matrix-multiplication.py
-triton-3.3.0+git9fc4181c/python/tutorials/04-low-memory-dropout.py
-triton-3.3.0+git9fc4181c/python/tutorials/05-layer-norm.py
-triton-3.3.0+git9fc4181c/python/tutorials/06-fused-attention.py
-triton-3.3.0+git9fc4181c/python/tutorials/07-extern-functions.py
-triton-3.3.0+git9fc4181c/python/tutorials/08-grouped-gemm.py
-triton-3.3.0+git9fc4181c/python/tutorials/09-persistent-matmul.py
-triton-3.3.0+git9fc4181c/python/tutorials/10-block-scaled-matmul.py
-triton-3.3.0+git9fc4181c/python/tutorials/README.rst
+triton-3.3.0+git9fc4181c/python/triton.egg-info/
+triton-3.3.0+git9fc4181c/python/triton.egg-info/PKG-INFO
+triton-3.3.0+git9fc4181c/python/triton.egg-info/SOURCES.txt
+triton-3.3.0+git9fc4181c/python/triton.egg-info/dependency_links.txt
+triton-3.3.0+git9fc4181c/python/triton.egg-info/entry_points.txt
+triton-3.3.0+git9fc4181c/python/triton.egg-info/not-zip-safe
+triton-3.3.0+git9fc4181c/python/triton.egg-info/requires.txt
+triton-3.3.0+git9fc4181c/python/triton.egg-info/top_level.txt
+triton-3.3.0+git9fc4181c/setup.cfg
triton-3.3.0+git9fc4181c/setup.py
triton-3.3.0+git9fc4181c/test/
triton-3.3.0+git9fc4181c/test/Analysis/
@@ -1166,6 +1138,3 @@ triton-3.3.0+git9fc4181c/unittest/Tools/
triton-3.3.0+git9fc4181c/unittest/Tools/LayoutUtilsTest.cpp
triton-3.3.0+git9fc4181c/unittest/Tools/LinearLayoutTest.cpp
triton-3.3.0+git9fc4181c/unittest/googletest.cmake
-triton-3.3.0+git9fc4181c/utils/
-triton-3.3.0+git9fc4181c/utils/generate-test-checks.py
-triton-3.3.0+git9fc4181c/utils/nightly.pypircPlease let me know if I should include more files. |
| export CIBW_FREE_THREADED_SUPPORT=1 | ||
| python3 -m cibuildwheel python --output-dir wheelhouse | ||
| python3 -m cibuildwheel . --output-dir wheelhouse | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
HI @mgorny
Please add this action before trying to use Azure (since its failing):
- uses: actions/upload-artifact@v4
with:
name: cibw-wheels-manylinux_2_28_${{ matrix.config.arch }}-wheels-upload
path: ./wheelhouse/*.whl
It will allow us to get the wheels from the workflow on this PR.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done.
atalman
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you, the changes looks good
|
Hi @mgorny looks like there are some failures on the integration tests here: And on H100: If you take a look at these test on main its passing: https://github.com/triton-lang/triton/actions/runs/14529664771/job/40789464219 |
|
Also looks like there is a merge conflict please rebase |
|
Okay, let's rebase first and see if that changes anything. |
Move the Python build system files to the top directory, to enable building triton via regular Python packaging tools. This will also make it possible to build source distributions that include all the files needed to build the package.
Do not download the dependencies before the CMake build command is executed. Most importantly, this avoids them being prematurely downloaded while only a source distribution is being built.
Add files necessary for the package build to source distribution. Currently, this implies some duplication, as we end up including both original `third_party` tree and the files copied from it.
Remove the `triton/_C/include` symlink that does not seem to serve any purpose right now, and causes trouble in some scenarios. Notably, `triton/_C/include` is neither included in the wheels as I build them from the git repository, or in the wheels that are published on PyPI right now. However, if `setuptools_scm` package is installed, it causes setuptools to attempt to include the symlink as a data files, which triggers an error (only regular files are allowed). Furthermore, including it in source distribution caused the symlink to be replaced by the actual files, which meant they were now included in the wheels. In place of that, add a trivial `mkdir()` that ensures that `triton._C` package directory is being present for the `setup()` call.
This one apparently is not really used either, and for some reason it causes the release workflow to put headers in `third_party/proton/proton/_C/include` rather than in `third_party/proton/csrc/include`.
Use entry points to locate Triton backends instead of iterating over the `triton.backends` package directory. This fixes support for nontrivial install layouts, particularly editable installs made using `package_dir`. Now `setup.py` keeps record of all installed backends in entry points, and `triton.backends` uses them to locate and load the backends. This also technically permits installing third-party backends without altering the Triton installation, by adding additional entry points.
Use a hybrid approach where clean `package_dir` is used for regular wheel installs, but symlinks are still used by the `develop` command and editable installs. Hopefully, this will give us the best of both worlds: clean source distributions, and reliable editable installs, given that the package loading logic used by setuptools currently is not 100% reliable and fails on CI.
| packages=get_packages(), | ||
| install_requires=[ | ||
| "setuptools>=40.8.0", | ||
| "importlib-metadata; python_version < '3.10'", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What is the reason to introduce new importlib-metadata dependency ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
importlib.metadata is used by the new backend finder. Unfortunately, the API changed in Python 3.10, and the usual way of dealing with that is to pull importlib-metadata backport for older Pythons.
|
Question about the third_party dependency changes:
|
Originally, this was needed to make the editable builds work. It's not technically necessary now that I've restore symlinks, but I figured out I'd leave it for consistency with the
This symlink is not used anywhere, and it is messing up source distributions (because of pypa/setuptools#4937). |
(and wasn't installed anyway) |
This reverts commit a782a36.
This PR adds basic mypy type checking to the codebase. Previously, type checking worked out of the box (e.g., via pylsp-mypy in Neovim with an editable install), but after [#6381](#6381), mypy started reporting: Cannot find implementation or library stub for module named 'triton'. This is likely due to a known issue with type checkers and editable installs. As a workaround, this PR introduces a mypy.ini with a [`mypy_path`](https://mypy.readthedocs.io/en/stable/config_file.html#confval-mypy_path) to restore correct module resolution. This is a small step toward making the Python codebase fully type-checkable.
triton-lang#6381 updated some building and doc but didn't fix the command to find the proper `compile_commands.json`. `python/build` would contain the old one if user has previously built the project in `./python`, which would not work now.
#6381 updated some building and doc but didn't fix the command to find the proper `compile_commands.json`. `python/build` would contain the old one if user has previously built the project in `./python`, which would not work now, since we now build the project at dir `.` <!--- The core Triton is a small number of people, and we receive many PRs (thank you!). To help us review your code more quickly, **if you are a new contributor (less than 3 PRs merged) we ask that you complete the following tasks and include the filled-out checklist in your PR description.** Complete the following tasks before sending your PR, and replace `[ ]` with `[x]` to indicate you have done them. --> # New contributor declaration - [ ] I am not making a trivial change, such as fixing a typo in a comment. - [x] I have written a PR description following these [rules](https://cbea.ms/git-commit/#why-not-how). - [ ] I have run `pre-commit run --from-ref origin/main --to-ref HEAD`. - Select one of the following. - [ ] I have added tests. - `/test` for `lit` tests - `/unittest` for C++ tests - `/python/test` for end-to-end tests - [x] This PR does not need a test because it's a trivial doc fix. - Select one of the following. - [x] I have not added any `lit` tests. - [ ] The `lit` tests I have added follow these [best practices](https://mlir.llvm.org/getting_started/TestingGuide/#filecheck-best-practices), including the "tests should be minimal" section. (Usually running Python code and using the instructions it generates is not minimal.)
This is an attempt to make it possible to build source distributions of Triton, with the goal that they should produce wheels identical to in-repo builds. The changes involve: - moving `setup.py` to top-level directory and merging `pyproject.toml`s, so the package can now be built from top directory - deferring downloads to `CMakeBuild` command execution, ensuring they don't happen unnecessarily while building source distributions - adding missing files to source distributions - removing `triton/_C/include` symlink that didn't seem to do anything right now, but broke non-isolated builds if `setuptools_scm` were installed, and caused wheels built from source distributions to include the includes - while at it, handle `wheel.bdist_wheel` being deprecated, since upstream recently announced plans to remove it TODO: - [x] The generated source distributions contain duplicate files (notably, files copied from `third_party`). Fixing this isn't going to be easy, since we're passing the output directories to `setup(packages=...)`. Not sure if we consider it a problem needing an immediate fix. - [ ] Update workflows to use new source distributions. - [x] Do we want to include documentation and tests as well, or just the bare essentials needed to build wheels? CC @ptillet @Jokeren @ThomasRaoux @atalman @tiran # New contributor declaration - [x] I am not making a trivial change, such as fixing a typo in a comment. - [x] I have written a PR description following these [rules](https://cbea.ms/git-commit/#why-not-how). - [x] I have run `pre-commit run --from-ref origin/main --to-ref HEAD`. - Select one of the following. - [ ] I have added tests. - `/test` for `lit` tests - `/unittest` for C++ tests - `/python/test` for end-to-end tests - [x] This PR does not need a test because it only changes the build system. - Select one of the following. - [x] I have not added any `lit` tests. - [ ] The `lit` tests I have added follow these [best practices](https://mlir.llvm.org/getting_started/TestingGuide/#filecheck-best-practices), including the "tests should be minimal" section. (Usually running Python code and using the instructions it generates is not minimal.) --------- Co-authored-by: Andrey Talman <[email protected]>
This PR adds basic mypy type checking to the codebase. Previously, type checking worked out of the box (e.g., via pylsp-mypy in Neovim with an editable install), but after [triton-lang#6381](triton-lang#6381), mypy started reporting: Cannot find implementation or library stub for module named 'triton'. This is likely due to a known issue with type checkers and editable installs. As a workaround, this PR introduces a mypy.ini with a [`mypy_path`](https://mypy.readthedocs.io/en/stable/config_file.html#confval-mypy_path) to restore correct module resolution. This is a small step toward making the Python codebase fully type-checkable.
This is an attempt to make it possible to build source distributions of Triton, with the goal that they should produce wheels identical to in-repo builds.
The changes involve:
setup.pyto top-level directory and mergingpyproject.tomls, so the package can now be built from top directoryCMakeBuildcommand execution, ensuring they don't happen unnecessarily while building source distributionstriton/_C/includesymlink that didn't seem to do anything right now, but broke non-isolated builds ifsetuptools_scmwere installed, and caused wheels built from source distributions to include the includeswheel.bdist_wheelbeing deprecated, since upstream recently announced plans to remove itTODO:
third_party). Fixing this isn't going to be easy, since we're passing the output directories tosetup(packages=...). Not sure if we consider it a problem needing an immediate fix.CC @ptillet @Jokeren @ThomasRaoux @atalman @tiran
New contributor declaration
I am not making a trivial change, such as fixing a typo in a comment.
I have written a PR description following these
rules.
I have run
pre-commit run --from-ref origin/main --to-ref HEAD.Select one of the following.
/testforlittests/unittestfor C++ tests/python/testfor end-to-end testsSelect one of the following.
littests.littests I have added follow these best practices,including the "tests should be minimal" section. (Usually running Python code
and using the instructions it generates is not minimal.)