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

Respect [tool.uv.sources] in build requirements #7172

Merged
merged 1 commit into from
Oct 15, 2024
Merged

Conversation

charliermarsh
Copy link
Member

Summary

We weren't respecting tool.uv.sources for build-requires.

Closes #7147.

@charliermarsh charliermarsh added the enhancement New feature or improvement to existing functionality label Sep 7, 2024
crates/uv/tests/sync.rs Outdated Show resolved Hide resolved
@charliermarsh
Copy link
Member Author

Ugh, the lower bound warnings are super annoying here. I guess I have to make that configurable?

@konstin
Copy link
Member

konstin commented Sep 10, 2024

The [build-system] table was created so that projects can be built by any PEP 517 build frontend. This means that you could mix and match tools, allowing people to migrate away from setuptools and for alternative to pip to being built. By reading tool.uv.sources, we support a uv-only feature, something that e.g. python -m build doesn't support. To make it clear that this doesn't build a vendor lock-in, we need to make it clear in the docs:

tool.uv.sources for the build-system table is for development only. It is our equivalent to using pip install -e ./my-setuptools-ext and then building without build isolation: It's our way of feature parity with pip without dropping you out if the uv project interface. We do only support it when building from a directory, we do not support it when building when from an index or a file sdist. Even uv build --wheel --sdist will not recognize it, but give you the default standards-only PEP 517 build experience. As a package author, you need to test that your (published) project does also builds without tool.uv.sources using only published packages.

@mmerickel
Copy link

mmerickel commented Sep 10, 2024

At some point you may want to support a find-links feature to side load a wheelhouse. You can see an example of how that works in the additional info of #7147. That would impact the lookup at all times and it does work to allow a build backend to find/resolve dependencies that are not distributed externally. As a naive user that’s basically how I think of tool.uv.sources as well.

Separately from that when you say that uv build --wheel won’t use the sources that doesn’t feel like it will be ok in the long term. Build dependencies do not need to be distributed if I’m building a wheel. Only a sdist where building happens by an external system. uv should be able to build my wheel using workspace dependencies and no one else needs them. The wheel can be installed without them.

fn build_system_requires_workspace() -> Result<()> {
let context = TestContext::new("3.12");

let build = context.temp_dir.child("build");
Copy link
Member

Choose a reason for hiding this comment

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

Maybe call it my-build or something so it's clear that this isn't about python -m build, otherwise build-system.requires = ["setuptools>=42", "build"] is easy to confuse.

@mmerickel
Copy link

Even uv build --wheel --sdist will not recognize it

I guess just to poke at this more if we go back to my issue that inspired this work #7147, I'm concerned about @konstin's assertion that uv build --wheel --package my-app wouldn't work with this new feature? Is that true that it wouldn't use the workspace dependency to build the wheel for package my-app which depends on the local workspace package my-setuptools-extension? So I could uv sync properly but not uv build?

@mmerickel
Copy link

Is there anything I can do to help move this forward?

@charliermarsh
Copy link
Member Author

I think uv build would respect this, and you'd need to use uv build --no-sources to avoid it. Are you okay with that, @konstin? I think it would be surprising if uv build didn't run the same build process as our other hooks.

@konstin
Copy link
Member

konstin commented Oct 15, 2024

Yeah that's what I'd suggest.

@charliermarsh
Copy link
Member Author

Ok, will rebase and merge this today.

@charliermarsh charliermarsh force-pushed the charlie/build branch 3 times, most recently from 629e500 to 9df08a3 Compare October 15, 2024 15:04
@charliermarsh charliermarsh enabled auto-merge (squash) October 15, 2024 15:24
@charliermarsh charliermarsh merged commit 855c191 into main Oct 15, 2024
61 checks passed
@charliermarsh charliermarsh deleted the charlie/build branch October 15, 2024 15:31
kaxil added a commit to astronomer/airflow that referenced this pull request Oct 15, 2024
Since astral-sh/uv#7172 was added we need to add this
tmeijn pushed a commit to tmeijn/dotfiles that referenced this pull request Oct 16, 2024
This MR contains the following updates:

| Package | Update | Change |
|---|---|---|
| [astral-sh/uv](https://github.com/astral-sh/uv) | patch | `0.4.21` -> `0.4.22` |

MR created with the help of [el-capitano/tools/renovate-bot](https://gitlab.com/el-capitano/tools/renovate-bot).

**Proposed changes to behavior should be submitted there as MRs.**

---

### Release Notes

<details>
<summary>astral-sh/uv (astral-sh/uv)</summary>

### [`v0.4.22`](https://github.com/astral-sh/uv/blob/HEAD/CHANGELOG.md#0422)

[Compare Source](astral-sh/uv@0.4.21...0.4.22)

##### Enhancements

-   Respect `[tool.uv.sources]` in build requirements ([#&#8203;7172](astral-sh/uv#7172))

##### Preview features

-   Add a dedicated `uv publish` error message for missing usernames ([#&#8203;8045](astral-sh/uv#8045))
-   Support interactive input in `uv publish` ([#&#8203;8158](astral-sh/uv#8158))
-   Use raw filenames in `uv publish` ([#&#8203;8204](astral-sh/uv#8204))

##### Performance

-   Reuse the result of `which git` ([#&#8203;8224](astral-sh/uv#8224))

##### Bug fixes

-   Avoid environment check optimization for `uv pip install --exact` ([#&#8203;8219](astral-sh/uv#8219))
-   Do not use free-threaded interpreters without a free-threaded request ([#&#8203;8191](astral-sh/uv#8191))
-   Don't recommend `--prerelease=allow` during build requirement resolution errors ([#&#8203;8192](astral-sh/uv#8192))
-   Prefer optimized builds for free-threaded Python downloads ([#&#8203;8196](astral-sh/uv#8196))
-   Retain old `python-build-standalone` releases ([#&#8203;8216](astral-sh/uv#8216))
-   Run `uv build` builds in the source distribution bucket ([#&#8203;8220](astral-sh/uv#8220))

</details>

---

### Configuration

📅 **Schedule**: Branch creation - At any time (no schedule defined), Automerge - At any time (no schedule defined).

🚦 **Automerge**: Disabled by config. Please merge this manually once you are satisfied.

♻ **Rebasing**: Whenever MR becomes conflicted, or you tick the rebase/retry checkbox.

🔕 **Ignore**: Close this MR and you won't be reminded about this update again.

---

 - [ ] <!-- rebase-check -->If you want to rebase/retry this MR, check this box

---

This MR has been generated by [Renovate Bot](https://github.com/renovatebot/renovate).
<!--renovate-debug:eyJjcmVhdGVkSW5WZXIiOiIzNy40NDAuNyIsInVwZGF0ZWRJblZlciI6IjM3LjQ0MC43IiwidGFyZ2V0QnJhbmNoIjoibWFpbiIsImxhYmVscyI6WyJSZW5vdmF0ZSBCb3QiXX0=-->
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or improvement to existing functionality
Projects
None yet
Development

Successfully merging this pull request may close these issues.

in a workspace local package A cannot depend on local package B as a build-system requirement
4 participants