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.

Sign up for GitHub

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

Jump to bottom

Warn when trying to uv sync a package without build configuration #7420

Merged
merged 7 commits into from
Sep 16, 2024
Merged

Warn when trying to uv sync a package without build configuration #7420

merged 7 commits into from
Sep 16, 2024

Conversation

lucab
Copy link
Contributor

@lucab lucab commented Sep 16, 2024 •
edited
Loading

This enhances uv sync logic in order to detect and warn if it is trying to operate on a packaged project with entrypoints.

Ref: #6998 (comment)
Closes: #7034

@lucab
Warn when trying to uv run a package without build configuration
4cf007d 
This enhances `uv run` logic in order to detect and warn if it is trying
to run a packaged script without any `build-system` configuration in the
project manifest.
lucab
lucab commented Sep 16, 2024
View reviewed changes
crates/uv-workspace/src/pyproject.rs
@@ -102,7 +127,7 @@ impl AsRef<[u8]> for PyProjectToml {
/// PEP 621 project metadata (`project`).
///
/// See <https://packaging.python.org/en/latest/specifications/pyproject-toml>.
#[derive(Serialize, Deserialize, Debug, Clone, PartialEq, Eq)]
#[derive(Serialize, Deserialize, Debug, Clone, PartialEq)]
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Note: IgnoredAny doesn't implement Eq, but the trait here seemed unused and fine to drop. Alternatively, we can either 1) store TOML tables in here, or 2) fully deserialize both scripts fields.

crates/uv/src/commands/project/run.rs Outdated
@@ -451,6 +451,12 @@ pub(crate) async fn run(
.await?
};

if project.workspace().pyproject_toml().is_packaged_script()
Copy link
Contributor Author

@lucab lucab Sep 16, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

Note: sync is performed on CLI add/remove/sync/run, but it seems to annoying to warn on each command. Thus this new warning is only displayed at uv run time, which should be the most effective place. But can be expanded if we prefer.

Copy link
Member

Choose a reason for hiding this comment

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

I think it makes sense to omit at add / remove. We might want it at sync, otherwise if you sync then active the environment (instead of using uv run), you'll never see it.

Maybe with some special-casing per https://github.com/astral-sh/uv/pull/7420/files#r1761054908 we can make the warnings quieter in uv run.

konstin
konstin approved these changes Sep 16, 2024
View reviewed changes
crates/uv-workspace/src/pyproject.rs Outdated Show resolved Hide resolved
crates/uv/tests/run.rs Outdated Show resolved Hide resolved
crates/uv-workspace/src/pyproject.rs Outdated Show resolved Hide resolved
zanieb
zanieb reviewed Sep 16, 2024
View reviewed changes
crates/uv-workspace/src/pyproject.rs Outdated
///
/// This detects if a project is declared as a uv package, or if it has
/// a scripts-related section defined.
pub fn is_packaged_script(&self) -> bool {
Copy link
Member

Choose a reason for hiding this comment

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

I'd be surprised if we needed this exact combination again. I'd just add a function like has_scripts and && that with is_package.

zanieb
zanieb reviewed Sep 16, 2024
View reviewed changes
crates/uv/tests/run.rs Outdated Show resolved Hide resolved
@zanieb
Copy link
Member

zanieb commented Sep 16, 2024

@lucab thanks for the self-review comments!

zanieb
zanieb reviewed Sep 16, 2024
View reviewed changes
crates/uv/tests/sync.rs Outdated Show resolved Hide resolved
@lucab lucab mentioned this pull request Sep 16, 2024
Open
@lucab
Copy link
Contributor Author

lucab commented Sep 16, 2024 •
edited
Loading

From an out-of-band chat with @zanieb:

  • the condition detection can be simplified as a has_scripts() && !is_package() heuristic.
  • uv sync is a better place for the warning for now, as uv run could be too annoying in the general (non-script) case.
  • this could be improved by parsing the scripts tables and key-match; that allows key-matching in uv run and adding more details in uv sync. Tracked at uv: parse scripts entries and warn on non-package projects #7428.
  • The initial uv run test can stay in the codebase with a note for the future.

@lucab lucab marked this pull request as ready for review September 16, 2024 14:52
@lucab lucab changed the title Warn when trying to uv run a package without build configuration Warn when trying to uv sync a package without build configuration Sep 16, 2024
@lucab lucab merged commit 23494d8 into astral-sh:main Sep 16, 2024
58 of 59 checks passed
@lucab lucab deleted the ups/warn-script-without-build-system branch September 16, 2024 15:50
@zanieb zanieb added the cli Related to the command line interface label Sep 16, 2024
@BrewTestBot BrewTestBot mentioned this pull request Sep 18, 2024
Merged
tmeijn pushed a commit to tmeijn/dotfiles that referenced this pull request Sep 21, 2024
@tmeijn
build(deps): update astral-sh/uv to v0.4.13
c8893fb 
This MR contains the following updates:

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

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.13`](https://github.com/astral-sh/uv/blob/HEAD/CHANGELOG.md#0413)

[Compare Source](astral-sh/uv@0.4.12...0.4.13)

##### Enhancements

-   Add `socks` support ([#&#8203;7503](astral-sh/uv#7503))
-   Avoid warning about bad Python interpreter links for empty project environment directories ([#&#8203;7527](astral-sh/uv#7527))
-   Improve invalid environment warning messages ([#&#8203;7544](astral-sh/uv#7544))
-   Use more verbose spelling of "virtualenv" during creation ([#&#8203;7523](astral-sh/uv#7523))
-   Do not use a user-facing warning for "Waiting to acquire lock..." message ([#&#8203;7502](astral-sh/uv#7502))

##### Performance

-   Use a single buffer for hints on resolver errors ([#&#8203;7497](astral-sh/uv#7497))

##### Bug fixes

-   Allow Python pre-releases to be used if they are first on the `PATH` ([#&#8203;7470](astral-sh/uv#7470))
-   Avoid deleting the project environment directory if it is not a virtual environment ([#&#8203;7522](astral-sh/uv#7522))
-   Do not error if the `CACHEDIR.TAG` file exists but cannot be written to ([#&#8203;7550](astral-sh/uv#7550))
-   Treat invalid platform as more compatible than invalid Python ([#&#8203;7556](astral-sh/uv#7556))
-   Use portable paths when serializing sources ([#&#8203;7504](astral-sh/uv#7504))
-   Compute resolver hints using the final reduced derivation tree ([#&#8203;7546](astral-sh/uv#7546))
-   Bump the wheel and sdist cache versions ([#&#8203;7560](astral-sh/uv#7560))
-   Heal cache entries with missing source distributions ([#&#8203;7559](astral-sh/uv#7559))

##### Rust libraries

-   Bump minimum supported Rust version from 1.80 -> 1.81

##### Documentation

-   Add `UV_LINK_MODE` to Docker caching example ([#&#8203;7510](astral-sh/uv#7510))
-   Clarify behavior of of overrides in CLI reference ([#&#8203;7537](astral-sh/uv#7537))

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

[Compare Source](astral-sh/uv@0.4.11...0.4.12)

##### Enhancements

-   Allow users to provide pre-defined metadata for resolution ([#&#8203;7442](astral-sh/uv#7442))
-   Invalidate existing tool environments on Python interpreter mismatch ([#&#8203;7451](astral-sh/uv#7451))

##### Bug fixes

-   Avoid fatal error when searching for egg-info with missing directory ([#&#8203;7498](astral-sh/uv#7498))

##### Documentation

-   Add note on cache growth for self-hosted GitHub runners ([#&#8203;5757](astral-sh/uv#5757))

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

[Compare Source](astral-sh/uv@0.4.10...0.4.11)

##### Enhancements

-   Add `--no-editable` support to `uv sync` and `uv export` ([#&#8203;7371](astral-sh/uv#7371))
-   Add support for `--only-dev` to `uv sync` and `uv export` ([#&#8203;7367](astral-sh/uv#7367))
-   Add support for remaining pip-supported file extensions ([#&#8203;7387](astral-sh/uv#7387))
-   Generate shell completion for `uvx` ([#&#8203;7388](astral-sh/uv#7388))
-   Include `uv export` command in `requirements.txt` output ([#&#8203;7374](astral-sh/uv#7374))
-   Prune unzipped source distributions in `uv cache prune --ci` ([#&#8203;7446](astral-sh/uv#7446))
-   Warn when trying to `uv sync` a package without build configuration ([#&#8203;7420](astral-sh/uv#7420))
-   Support requests for pre-releases in the `--python` option ([#&#8203;7335](astral-sh/uv#7335))

##### Bug fixes

-   Avoid erroneous version warning for `.dist-info` directories ([#&#8203;7444](astral-sh/uv#7444))
-   Avoid removing seed packages for `uv venv --seed` environments ([#&#8203;7410](astral-sh/uv#7410))
-   Avoid unnecessary progress bar initializations ([#&#8203;7412](astral-sh/uv#7412))
-   Error when `tool.uv.sources` contains duplicate package names ([#&#8203;7383](astral-sh/uv#7383))
-   Include `--branch` et al when resolving unnamed URLs in `uv add` ([#&#8203;7447](astral-sh/uv#7447))
-   Include `dev-dependencies` in `--no-sources` invocations ([#&#8203;7408](astral-sh/uv#7408))
-   Include the parent interpreter in Python discovery when `--system` is used ([#&#8203;7440](astral-sh/uv#7440))
-   Respect `--no-sources` in PEP 723 scripts ([#&#8203;7409](astral-sh/uv#7409))
-   Respect `pyproject.toml` credentials from user-provided requirements ([#&#8203;7474](astral-sh/uv#7474))
-   Use consistent PyPI cache bucket ([#&#8203;7443](astral-sh/uv#7443))
-   Use unambiguous relative paths in `uv export` ([#&#8203;7378](astral-sh/uv#7378))

##### Documentation

-   Add documentation on platform-specific dependencies ([#&#8203;7411](astral-sh/uv#7411))
-   Add documentation for passing installer options on Linux ([#&#8203;6839](astral-sh/uv#6839))
-   Separate project data from configuration settings ([#&#8203;7053](astral-sh/uv#7053))

##### Error messages

-   Hint at missing `project.name` ([#&#8203;6803](astral-sh/uv#6803))
-   Surface dedicated `project.name` error for workspaces ([#&#8203;7399](astral-sh/uv#7399))
-   Remove duplicate warning for settings discovery errors ([#&#8203;7384](astral-sh/uv#7384))

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

[Compare Source](astral-sh/uv@0.4.9...0.4.10)

##### Enhancements

-   Allow `uv tool upgrade --all` to continue on individual upgrade failure ([#&#8203;7333](astral-sh/uv#7333))
-   Support globs as cache keys in `tool.uv.cache-keys` ([#&#8203;7268](astral-sh/uv#7268))
-   Add Python package (`__main__.py`) support to `uv run` ([#&#8203;7281](astral-sh/uv#7281))
-   Add zip application support to `uv run` ([#&#8203;7289](astral-sh/uv#7289))
-   Add `--token` option to `self update` command ([#&#8203;7279](astral-sh/uv#7279))

##### Performance

-   Use `globwalk` for `cache-keys` matching ([#&#8203;7337](astral-sh/uv#7337))

##### Bug fixes

-   Always treat archive-like requirements as local files ([#&#8203;7364](astral-sh/uv#7364))
-   Apply `--no-install` options when constructing resolution ([#&#8203;7277](astral-sh/uv#7277))
-   Avoid clobbering existing `py.typed` files contents in `uv init` ([#&#8203;7338](astral-sh/uv#7338))
-   Avoid enforcing platform compatibility when validating lockfile ([#&#8203;7305](astral-sh/uv#7305))
-   Avoid installing transitive dev dependencies ([#&#8203;7318](astral-sh/uv#7318))
-   Avoid selecting prerelease Python installations without opt-in ([#&#8203;7300](astral-sh/uv#7300))
-   Fix PPC64 page size in binary builds. ([#&#8203;7298](astral-sh/uv#7298))
-   Include pre-release Python versions in `uv python list` ([#&#8203;7290](astral-sh/uv#7290))
-   Make version ID optional for source builds ([#&#8203;7362](astral-sh/uv#7362))
-   Support relative paths in `uv add --script` ([#&#8203;7301](astral-sh/uv#7301))

##### Documentation

-   Fix documentation typos for `uv build --build-constraint` flag ([#&#8203;7330](astral-sh/uv#7330))
-   Fix grammatical error in CLI docs ([#&#8203;7353](astral-sh/uv#7353))

##### Error messages

-   Add dedicated lock errors for wheel-only distributions ([#&#8203;7307](astral-sh/uv#7307))
-   Avoid treating `.whl` sources as source distributions ([#&#8203;7303](astral-sh/uv#7303))
-   Clarify Python requirement source for script incompatibilities ([#&#8203;7339](astral-sh/uv#7339))

</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
Reviewers

@konstin konstin konstin approved these changes

@zanieb zanieb zanieb approved these changes

Assignees
No one assigned
Labels
cli Related to the command line interface
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Warn if project.scripts exists without build-system declaration (or package = true)
3 participants
@lucab @zanieb @konstin