Skip to content

Fix settings rendering for extra-build-dependencies#15161

Merged
charliermarsh merged 1 commit intomainfrom
charlie/s
Aug 11, 2025
Merged

Fix settings rendering for extra-build-dependencies#15161
charliermarsh merged 1 commit intomainfrom
charlie/s

Conversation

@charliermarsh
Copy link
Member

@charliermarsh charliermarsh commented Aug 8, 2025

Summary

It would be nice if this rendered as [tool.uv.extra-build-dependencies] and [extra-build-dependencies] (in uv.toml), but this is at least correct.

Closes #15124.

@charliermarsh charliermarsh added the documentation Improvements or additions to documentation label Aug 8, 2025
@charliermarsh charliermarsh requested a review from zanieb August 8, 2025 09:37
@charliermarsh charliermarsh marked this pull request as ready for review August 8, 2025 09:37
flash-attn = { FLASH_ATTENTION_SKIP_CUDA_BUILD = "TRUE" }
"#
)]
pub extra_build_variables: Option<ExtraBuildVariables>,
Copy link
Member Author

Choose a reason for hiding this comment

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

I think these were unread and basically duplicated from the ResolverSettings.

@charliermarsh charliermarsh temporarily deployed to uv-test-registries August 8, 2025 09:38 — with GitHub Actions Inactive
`EXPECTED_ANYIO_VERSION` not set

hint: This usually indicates a problem with the package or the build environment.
help: `child` was included because `parent` (v0.1.0) depends on `child`
Copy link
Member Author

Choose a reason for hiding this comment

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

I guess this is "right" but it's kind of weird (we only warn, not fail, when we fail to parse options from a pyproject.toml). Before, we were both warning and erroring because these settings were parsed twice, once as pyproject.toml options and once as project settings.

Copy link
Member

Choose a reason for hiding this comment

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

Hm. I think I'd prefer the hard error still here. Does that mean we need to enforce it later? I think this is not a "parse" error.

Copy link
Member Author

Choose a reason for hiding this comment

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

I guess, yeah.

Copy link
Member Author

Choose a reason for hiding this comment

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

Oh shoot sorry, I forgot about this.

Copy link
Member Author

Choose a reason for hiding this comment

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

I opened a revert here: #15228. Unless you think this is a net improvement for the release.

Copy link
Member

Choose a reason for hiding this comment

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

I had intentionally not been merging. I'm a bit on the fence, but let's just avoid the churn for users — I merged the revert.

Copy link
Member Author

Choose a reason for hiding this comment

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

The sad thing about enforcing it "later" is that we won't be able to show you the source file because it'll no longer come from deserialization. It's nice that, right now, we can show you the exact line.

@charliermarsh charliermarsh temporarily deployed to uv-test-registries August 8, 2025 10:52 — with GitHub Actions Inactive
@charliermarsh charliermarsh merged commit 9ba1ef1 into main Aug 11, 2025
95 checks passed
@charliermarsh charliermarsh deleted the charlie/s branch August 11, 2025 21:24
@charliermarsh charliermarsh restored the charlie/s branch August 11, 2025 21:24
charliermarsh added a commit that referenced this pull request Aug 11, 2025
charliermarsh added a commit that referenced this pull request Sep 1, 2025
## Summary

It would be nice if this rendered as
`[tool.uv.extra-build-dependencies]` and `[extra-build-dependencies]`
(in `uv.toml`), but this is at least correct.

Closes #15124.
charliermarsh added a commit that referenced this pull request Sep 1, 2025
## Summary

It would be nice if this rendered as
`[tool.uv.extra-build-dependencies]` and `[extra-build-dependencies]`
(in `uv.toml`), but this is at least correct.

Closes #15124.
charliermarsh added a commit that referenced this pull request Sep 2, 2025
## Summary

This was fixed in #15161, then
reverted as it regressed the error handling. I've re-applied the change
here, but moved the error handling to the runtime, rather than
parse-time. I think this is slightly worse in that we no longer include
the originating source code snippet, but it at least gives us the
expected behavior :(

Closes #15124.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

documentation Improvements or additions to documentation

Projects

None yet

Development

Successfully merging this pull request may close these issues.

extra-build-dependencies docs example is wrong

3 participants