Modernize build to support sdists

[zbowling]

Replace the two setup setuptools based build process with scikit_build_core.build to call cmake.

This adds support for sdists releases of this package, git install directly of this repo from pip (submodules may make that hard), and generally simplifies the build process since now all that is required is doing pip install . in the directory or "python -m build" to build the wheel and sdist with no other build steps. This also fixes some cross compiling issues.

This also means that cibuildwheel will work out of the box with github to build this for more architectures and you won't have to maintain as much custom github action CI logic.

A subset of tests run in CI as part package building now.

CUDA is optional again which fixes Mac builds. The optional triton kernel is also available as a backend but only when the triton package is also installed in the env.

Updated documentation and install instructions.

Add attestation support for uploaded wheels.

[Ubospica]

Hi @zbowling, thanks for the contribution! Modernizing the building process with scikit-build-core and cibuildwheel is definitely helpful for XGrammar. We have planned this for a long time, but there is no bandwidth to do it. So thank you for addressing this important issue!

I updated your PR to make it more suitable for XGrammar's specific situation. I'm also just starting to use these tools, so if there are any problems, feel free to point them out.

We also need to consider how to make CD pipeline work after this migration (as the previous CD depends on setup.py). I will try to work that out later, and contributions are still welcome!

[zbowling]

I'm so glad this is useful! I'll swing back and address the comments. Some of those things you pointed out are unneeded. I have simpler GitHub CI scripts too but I hadn't fully tested them in my branch so that's why this PR is still a draft. 😁

We use xgrammar for a feature in modular/max but we're running into build issues and with missing aarch64 wheel or an sdist in pypi so we couldn't build it automatically at install.

I'm also now maintaining the upstream conda-forge build of xgrammar for pure conda users. I made another patch there to fix macOS builds by making triton optional that I'll send here in a second PR. You can see the patch in https://github.com/conda-forge/xgrammar-feedstock

[zbowling]

@Ubospica This is getting closer. There was a big change I made here as part of getting the test to work again with CI is fixing the kernels to work when triton and cuda are not available to backback to CPU. Now I'm able to get all tests to pass on MacOS. Some other non-functional changes to clean up error messages to meet lint warnings got mixed in though when I refactored the API there.

[Ubospica]

Thank a lot @zbowling @maresb! I was too busy the past few days to review it. I’ll review it tomorrow and get it merged once it's ready.

[Ubospica]

@zbowling I have finished reviewing the build system section. I will check the linting part tomorrow. Overall, the PR looks great. I understand that this is a significant amount of work, but it is also highly valuable for this repository. Thanks!

Just a few questions and points to mention:

1. Can this workflow integrate well with the documentation workflow at documentation.yaml?
2. Also, thank you for maintaining the wheel on conda-forge. Is there any part of the conda-forge repository that can be moved into the xgrammar repository? If so, it would be ideal if this could help reduce your maintenance workload.
3. Please use the pre-commit hook to format the code. I noticed some extra blank lines, missing newlines at the end of files, and overly long lines. They can be easily removed with the pre-commit hook.

[zbowling]

@zbowling I have finished reviewing the build system section. I will check the linting part tomorrow. Overall, the PR looks great. I understand that this is a significant amount of work, but it is also highly valuable for this repository. Thanks!

Yeah, no problem, my pleasure. :)

1. Can this workflow integrate well with the documentation workflow at documentation.yaml?

We could yeah! It doesn't seem too difficult to merge the two.

2. Also, thank you for maintaining the wheel on conda-forge. Is there any part of the conda-forge repository that can be moved into the xgrammar repository? If so, it would be ideal if this could help reduce your maintenance workload.

I've integrated the one patch I was doing there here in this PR. Specifically, just fixing MacOS targets without CUDA by making triton optional. Once this PR lands, I can get rid of more than half of the stuff in there, so it's a pretty simple recipe. Mainly directly dealing with cmake parts of the build recipe there and it should be as simple as "pip install ."

Then, going forward, a release here to PyPI, the bot that watches PyPI for new builds, should automatically notify me within an hour with a new proposed PR to accept to release there.

The one bit of work I have to do is patch to unbundle dlpack (conda-forge requires unbundling certain static linked dependencies so they can be shared and updated together) but that is mostly just changing include paths.

I can also add you as a maintainer there on that repo :)

3. Please use the pre-commit hook to format the code. I noticed some extra blank lines, missing newlines at the end of files, and overly long lines. They can be easily removed with the pre-commit hook.

Ack, will do!

[zbowling]

This should look a bit better. Note I swapped out ruff for black in the pre-commit and fixed up some rules (I was using ruff locally in place of black and it's why it blew up some line wraps but I fixed a few things so now it won't do that which reduces a lot of the changes in this PR.

I also noticed the aarch64 and x86_64 linux builds are lot bigger. Specifically because the cuda kernel that gets compiled now I suspect.

[Ubospica]

Hi @zbowling, thanks for the update! I just finished the review and I think it is close to being complete now! Regarding the issues you mentioned:

I also noticed the aarch64 and x86_64 linux builds are lot bigger

I believe this is because the RelWithDebInfo build type will embed the debug information into the C++ library. But I think 20MB (and 5MB after compression) should not be too big. In pyproject.toml:

cmake.build-type = "RelWithDebInfo"

The one bit of work I have to do is patch to unbundle dlpack (conda-forge requires unbundling certain static linked dependencies so they can be shared and updated together) but that is mostly just changing include paths.

I can also add you as a maintainer there on that repo :)

DLpack is a header-only library. Would it influence the conda packaging? Also, how does the include path change? If the xgrammar repo can be modified to avoid doing this, I'm happy to do it. I'd also appreciate it if you added me to the maintainer of the conda repo.

I also made some changes, reverted some changes, and added a new commit to your branch. Mainly:

• Removed ruff from pre-commit (it should be enforced in CI: Add CI for XGrammar #214)
• Reverted the separated Error definition
• Leave documentation.yaml a separate workflow
• Reduce the set of rules applied by ruff
• Further format the code

I think the current PR looks great. If you have any further suggestions or notice any issues, feel free to bring them up!

[Ubospica]

Linting is certainly important, but our experience suggests that it should not be enforced in pre-commit; instead, it should be handled in CI. Running linting in pre-commit can prevent committing temporary changes, as it requires fixing all linting errors beforehand.

So we can remove this for now and add it info an workflow later.

[Ubospica]

Is there any exact issues with editable installation?

[zbowling]

It injects a proxy that tries to rebuild and compile on demand when you import and invoke it. For me, it's not working at all. Something is failing to compile at import time. Editable native deps are still experimental, so I was going to debug that later.. Non-editable builds compile more traditionally and more directly, and they work fine.

I may still poke at fixing it, but I already bit off a ton more here and figured that could be a follow-up PR once I debugged what is broken there.

[Ubospica]

It's interesting because the editable installation with

pip install -e --no-build-isolation .

works well for me. The rebuilding can be successfully triggered when the C++ codebase is changed.

But of course we can comment this first since there could still be problems. If you have any further findings, I would appreciate it for bringing it up.

[zbowling]

Oh nice! Yeah I didn't have time to debug the first pass what was failing.

[zbowling]

Oh interesting. Editable builds work on my Linux machine but not my Mac. Debugging this stack trace

[Ubospica]

I see, yeah it should work on Linux. We may need to see how Mac supports it.

[Ubospica]

@zbowling Just wanna ask if this editable installation works now. I feel it is helpful in the real development, so if it has no compatibility issues, we can enable it.

[Ubospica]

We can throw an error for this. I have done that in the new commit.

[zbowling]

I believe this is because the RelWithDebInfo build type will embed the debug information into the C++ library. But I think 20MB (and 5MB after compression) should not be too big. In pyproject.toml:

Ack. Sounds good!

DLpack is a header-only library. Would it influence the conda packaging? Also, how does the include path change? If the xgrammar repo can be modified to avoid doing this, I'm happy to do it. I'd also appreciate it if you added me to the maintainer of the conda repo.

Yeah, dlpack is in conda-forge even as a header-only dep and we use it as a build-only dependency. That way, when dlpack gets updated upstream, it triggers a bunch of scripts that transitively let other know packages that depend on it that they should be rebuilt, too, all across the conda-forge ecosystem. It's been a huge win for conda-forge to catch security issues when a dependency gets updated, even if it's statically included in another package.

For me in that conda-forge recipe, I can get around it by changing include paths to include to look at $PREFIX/include/dlpack where dlpack gets install in the dev env instead of 3rdparty/dlpack in the recipe.

One change we could do is a find_package() module search for dlpack in cmake before falling back to searching in 3rdparty/, and then I wouldn't have to inject any header search paths.

Picojson was another but this one gets an exception in conda-forge since picojson upstream has been unmaintained for several years and many different ML projects have their own hard forks of it. Googletest isn't used in the conda-forge build so I largely ignore it.

After this PR lands, the recipe there will be mostly just doing a pip install and changing a few header search paths, and no more real patches on any source anymore.

I also made some changes, reverted some changes, and added a new commit to your branch. Mainly:

• Removed ruff from pre-commit (it should be enforced in CI: Add CI for XGrammar #214)
• Reverted the separated Error definition
• Leave documentation.yaml a separate workflow
• Reduce the set of rules applied by ruff
• Further format the code

Sounds good!

I think the current PR looks great. If you have any further suggestions or notice any issues, feel free to bring them up!

Awesome! Yeah,, I have some ideas, but they don't need to be in this PR. I can send more later.

[Ubospica]

For me in that conda-forge recipe, I can get around it by changing include paths to include to look at $PREFIX/include/dlpack where dlpack gets install in the dev env instead of 3rdparty/dlpack in the recipe.

One change we could do is a find_package() module search for dlpack in cmake before falling back to searching in 3rdparty/, and then I wouldn't have to inject any header search paths.

Got it! We can do that in the next PR.

Picojson was another but this one gets an exception in conda-forge since picojson upstream has been unmaintained for several years and many different ML projects have their own hard forks of it. Googletest isn't used in the conda-forge build so I largely ignore it.

Indeed picojson has been modified a lot by us. I think it can now be treated as part of the codebase rather than an external dependency, so there’s no need to rely on the upstream Conda package.

Awesome! Yeah,, I have some ideas, but they don't need to be in this PR. I can send more later.

That sounds great. Thank you for your continuous contribution!

I think this PR is ready and I will merge it. Thanks a lot for your help @zbowling @mgorny !

[zbowling]

I changed these to be outside the statement because Ruff ignores them inside the statement. https://docs.astral.sh/ruff/formatter/#format-suppression

[Ubospica]

Thanks for bringing that up. But since we use black as the formatter instead of ruff, and black supports fmt: off by lines, this should be fine as well.