-
Notifications
You must be signed in to change notification settings - Fork 1.8k
Allow overriding module name for uv build backend #11884
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
Allow overriding module name for uv build backend #11884
Conversation
a18cd50
to
762430e
Compare
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.
I think this generally looks good to me, but I have some requests, nits and questions. :-)
Cargo.lock
Outdated
@@ -1851,9 +1851,9 @@ checksum = "b248f5224d1d606005e02c97f5aa4e88eeb230488bcc03bc9ca4d7991399f2b5" | |||
|
|||
[[package]] | |||
name = "insta" | |||
version = "1.42.1" | |||
version = "1.42.2" |
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.
Is this version bump necessary? I think we usually let renovatebot do this.
Not a big deal to include it here I guess, but if it isn't necessary it's probably best to kick this out.
/// The default module name is the package name with dots and dashes replaced by underscores. | ||
/// | ||
/// Note that using this option runs the risk of creating two packages with different names but | ||
/// the same module names. Installing such packages together leads to undefined behavior, often |
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.
cc @konstin on the terminology here. I would expect this to be unspecified behavior and not undefined.
let module_name = if let Some(module_name) = settings.module_name { | ||
module_name | ||
} else { | ||
// Should never happen, the rules for package names (in dist-info formatting) are stricter |
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.
I would say, "This should never result in an error." Otherwise I think just "should never happen" is a little confusing and ambiguous. I thought it meant that settings.module_name
could never be None
(which confused me).
I would also therefore be tempted to do an expect("...")
here, but I'm not 100% on the relevant invariants here. Returning an error seems fine.
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.
We're very permissive with things that can never happen in other place (returning errors instead of unwrapping), which I'm mirroring here.
crates/uv-build-backend/src/wheel.rs
Outdated
let module_name = if let Some(module_name) = settings.module_name { | ||
module_name | ||
} else { | ||
// Should never happen, the rules for package names (in dist-info formatting) are stricter |
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.
Same comment as above applies here too I think.
#[error("An identifier must not be empty")] | ||
Empty, | ||
#[error( | ||
"Invalid first character `{first}` at for `{identifier}`, expected an underscore or an alphabetic character" |
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.
"Invalid first character `{first}` at for `{identifier}`, expected an underscore or an alphabetic character" | |
"Invalid first character `{first}` for identifier `{identifier}`, expected an underscore or an alphabetic character" |
/// (we just use Rust's `is_alphabetic`) and we don't convert to NFKC, but it's good enough | ||
/// for our validation purposes. | ||
#[derive(Debug, Clone, Hash, PartialEq, Eq, PartialOrd, Ord)] | ||
pub struct Identifier(String); |
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.
This could probably be a Box<str>
.
A minor point and perhaps premature, but these things have a tendency to build up over time.
)] | ||
InvalidFirstChar { first: char, identifier: String }, | ||
#[error( | ||
"Invalid character `{invalid_char}` at position {pos} for `{identifier}`, expected an underscore or an alphanumeric character" |
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.
"Invalid character `{invalid_char}` at position {pos} for `{identifier}`, expected an underscore or an alphanumeric character" | |
"Invalid character `{invalid_char}` at position {pos} for identifier `{identifier}`, \ | |
expected an underscore or an alphanumeric character" |
|
||
#[test] | ||
fn invalid_first_char() { | ||
assert_snapshot!(Identifier::from_str("1foo").unwrap_err(), @"Invalid first character `1` at for `1foo`, expected an underscore or an alphabetic character"); |
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.
assert_snapshot!(Identifier::from_str("1foo").unwrap_err(), @"Invalid first character `1` at for `1foo`, expected an underscore or an alphabetic character"); | |
assert_snapshot!( | |
Identifier::from_str("1foo").unwrap_err(), | |
@"Invalid first character `1` at for `1foo`, expected an underscore or an alphabetic character", | |
); |
And then do the same for the other assertions. Otherwise the lines get very long. Long lines as a result of snapshot strings are fine, because those are more annoying to fix. But otherwise, adding some line breaks, makes the code generating the snapshot a little easier to read IMO.
@@ -278,3 +279,86 @@ fn preserve_executable_bit() -> Result<()> { | |||
|
|||
Ok(()) | |||
} | |||
|
|||
/// Test `tool.uv.build-backend.module-name`. |
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.
Can you add a little more detail about what this test is doing?
Traceback (most recent call last): | ||
File "<string>", line 1, in <module> | ||
ModuleNotFoundError: No module named 'foo' | ||
"###); |
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.
Can you say why this doesn't work despite there being a src/foo/__init__.py
?
54b3a2b
to
9843e2f
Compare
fcd53d3
to
ee96f0b
Compare
) ## Summary This PR enables module name overrides for editable installs. Builds upon #11884. The `tool.uv.build-backend.module-name` option is now respected during editable build processes. ## Test Plan Added a test. --------- Co-authored-by: Charlie Marsh <[email protected]>
This MR contains the following updates: | Package | Update | Change | |---|---|---| | [astral-sh/uv](https://github.com/astral-sh/uv) | patch | `0.6.5` -> `0.6.9` | 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.6.9`](https://github.com/astral-sh/uv/blob/HEAD/CHANGELOG.md#069) [Compare Source](astral-sh/uv@0.6.8...0.6.9) ##### Enhancements - Use `keyring --mode creds` when `authenticate = "always"` ([#​12316](astral-sh/uv#12316)) - Fail with specific error message when no password is present and `authenticate = "always"` ([#​12313](astral-sh/uv#12313)) ##### Bug fixes - Add boolish value parser for `UV_MANAGED_PYTHON` flags ([#​12345](astral-sh/uv#12345)) - Make deserialization non-fatal when assessing source tree revisions ([#​12319](astral-sh/uv#12319)) - Use resolver-returned wheel over alternate cached wheel ([#​12301](astral-sh/uv#12301)) ##### Documentation - Add experimental `--torch-backend` to the PyTorch guide ([#​12317](astral-sh/uv#12317)) - Fix `#keyring-provider` references in alternative index docs ([#​12315](astral-sh/uv#12315)) - Fix `--directory` path in examples ([#​12165](astral-sh/uv#12165)) ##### Preview changes - Automatically infer the PyTorch index via `--torch-backend=auto` ([#​12070](astral-sh/uv#12070)) ### [`v0.6.8`](https://github.com/astral-sh/uv/blob/HEAD/CHANGELOG.md#068) [Compare Source](astral-sh/uv@0.6.7...0.6.8) ##### Enhancements - Add support for enabling all groups by default with `default-groups = "all"` ([#​12289](astral-sh/uv#12289)) - Add simpler `--managed-python` and `--no-managed-python` flags for toggling Python preferences ([#​12246](astral-sh/uv#12246)) ##### Performance - Avoid allocations for default cache keys ([#​12063](astral-sh/uv#12063)) ##### Bug fixes - Allow local version mismatches when validating lockfile ([#​12285](astral-sh/uv#12285)) - Allow owned string when deserializing `requires-python` ([#​12278](astral-sh/uv#12278)) - Make cache errors non-fatal in `Planner::build` ([#​12281](astral-sh/uv#12281)) ### [`v0.6.7`](https://github.com/astral-sh/uv/blob/HEAD/CHANGELOG.md#067) [Compare Source](astral-sh/uv@0.6.6...0.6.7) ##### Python - Add CPython 3.14.0a6 - Fix regression where extension modules would use wrong `CXX` compiler on Linux - Enable FTS3 enhanced query syntax for SQLite See the [`python-build-standalone` release notes](https://github.com/astral-sh/python-build-standalone/releases/tag/20250317) for more details. ##### Enhancements - Add support for `-c` constraints in `uv add` ([#​12209](astral-sh/uv#12209)) - Add support for `--global` default version in `uv python pin` ([#​12115](astral-sh/uv#12115)) - Always reinstall local source trees passed to `uv pip install` ([#​12176](astral-sh/uv#12176)) - Render token claims on publish permission error ([#​12135](astral-sh/uv#12135)) - Add pip-compatible `--group` flag to `uv pip install` and `uv pip compile` ([#​11686](astral-sh/uv#11686)) ##### Preview features - Avoid creating duplicate directory entries in built wheels ([#​12206](astral-sh/uv#12206)) - Allow overriding module names for editable builds ([#​12137](astral-sh/uv#12137)) ##### Performance - Avoid replicating core-metadata field on `File` struct ([#​12159](astral-sh/uv#12159)) ##### Bug fixes - Add `src` to default cache keys ([#​12062](astral-sh/uv#12062)) - Discard insufficient fork markers ([#​10682](astral-sh/uv#10682)) - Ensure `python pin --global` creates parent directories if missing ([#​12180](astral-sh/uv#12180)) - Fix GraalPy abi tag parsing and discovery ([#​12154](astral-sh/uv#12154)) - Remove extraneous script packages in `uv sync --script` ([#​12158](astral-sh/uv#12158)) - Remove redundant `activate.bat` output ([#​12160](astral-sh/uv#12160)) - Avoid subsequent index hint when no versions are available on the first index ([#​9332](astral-sh/uv#9332)) - Error on lockfiles with incoherent wheel versions ([#​12235](astral-sh/uv#12235)) ##### Rust API - Update `BaseClientBuild` to accept custom proxies ([#​12232](astral-sh/uv#12232)) ##### Documentation - Make testpypi index explicit in example snippet ([#​12148](astral-sh/uv#12148)) - Reverse and format the archived changelogs ([#​12099](astral-sh/uv#12099)) - Use consistent commas around i.e. and e.g. ([#​12157](astral-sh/uv#12157)) - Fix typos in MRE docs ([#​12198](astral-sh/uv#12198)) - Fix double space typo ([#​12171](astral-sh/uv#12171)) ### [`v0.6.6`](https://github.com/astral-sh/uv/blob/HEAD/CHANGELOG.md#066) [Compare Source](astral-sh/uv@0.6.5...0.6.6) ##### Python - Add support for dynamic musl Python distributions on x86-64 Linux ([#​12121](astral-sh/uv#12121)) - Allow the experimental JIT to be enabled at runtime on Python 3.13 and 3.14 on Linux - Upgrade the build toolchain to LLVM 20, improving performance See the [`python-build-standalone` release notes](https://github.com/astral-sh/python-build-standalone/releases/tag/20250311) for more details. ##### Enhancements - Add `--marker` flag to `uv add` ([#​12012](astral-sh/uv#12012)) - Allow overriding module name for uv build backend ([#​11884](astral-sh/uv#11884)) - Sync latest Python releases ([#​12120](astral-sh/uv#12120)) - Use 'Upload' instead of 'Download' in publish reporter ([#​12029](astral-sh/uv#12029)) - Add `[index].authenticate` allowing authentication to be required on an index ([#​11896](astral-sh/uv#11896)) - Add support for Windows legacy scripts in `uv tool run` ([#​12079](astral-sh/uv#12079)) - Propagate conflicting dependency groups when using `include-group` ([#​12005](astral-sh/uv#12005)) - Show ambiguous requirements when `uv add` failed ([#​12106](astral-sh/uv#12106)) ##### Performance - Cache workspace discovery ([#​12096](astral-sh/uv#12096)) - Insert dependencies into fork state prior to fetching metadata ([#​12057](astral-sh/uv#12057)) - Remove some allocations from `uv-auth` ([#​12077](astral-sh/uv#12077)) ##### Bug fixes - Avoid considering `PATH` updated when the `export` is commented in the shellrc ([#​12043](astral-sh/uv#12043)) - Fix `uv publish` retry on network failures ([#​12041](astral-sh/uv#12041)) - Use a sized stream in `uv publish` to comply with WSGI PyPI server constraints ([#​12111](astral-sh/uv#12111)) - Fix `uv python install --reinstall` when the version was not previously installed ([#​12124](astral-sh/uv#12124)) ##### Preview features - Fix `uv_build` invocation ([#​12058](astral-sh/uv#12058)) ##### Documentation - Quote versions string in `python-versions.md` ([#​12112](astral-sh/uv#12112)) - Fix tool concept page headings ([#​12053](astral-sh/uv#12053)) - Update the `[index].authenticate` docs ([#​12102](astral-sh/uv#12102)) - Update versioning policy ([#​11666](astral-sh/uv#11666)) </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:eyJjcmVhdGVkSW5WZXIiOiIzOS4xOTQuMCIsInVwZGF0ZWRJblZlciI6IjM5LjIwOS4wIiwidGFyZ2V0QnJhbmNoIjoibWFpbiIsImxhYmVscyI6WyJSZW5vdmF0ZSBCb3QiXX0=-->
Thank you for uv, it has game-changer capabilities in the field of Python package and environment maangement!
Summary
This is a small PR adding the option
module-name
(tool.uv.build-backend.module-name
) to the uv build backend ( #8779 ).Currently, the uv build backend will assume that the module name matches the (dash to underdash-transformed) package name. In some packaging scenarios this is not the case, and currently there exists no possibility to override it, which this PR addresses.
From the main issue ( #8779 ) I could not tell if there is any extensive roadmap or plans how to implement more complex scenarios, hence this PR as a suggestion for a small feature with a big impact for certain scenarios.
I am new to Rust, I hope the borrow/reference usage is correct.
Test Plan
So far I tested this at an example, if desired I can look into extending the tests.
Fixes #11428