Skip to content

refactor(embedded): Integrate cargo-script logic into main parser #15168

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

Merged
merged 20 commits into from
Feb 11, 2025

Conversation

epage
Copy link
Contributor

@epage epage commented Feb 10, 2025

What does this PR try to resolve?

This is a part of #12207

Now that the experiment is over and we can be intrusive, this merges all of the permanent cargo-script manifest logic into the regular manifest loading code path with the goal of reducing the number of places people need to be aware to check to know how things work so we're less likely to overlook them (e.g. package.names being optional). This should also make error reporting better as we will eventually only use the real manifest users specify, instead of one with a lot of edits by us.

This comes at the cost of some uglier code needed to check some of these cases.

How should we test and review this PR?

Additional information

@rustbot
Copy link
Collaborator

rustbot commented Feb 10, 2025

r? @ehuss

rustbot has assigned @ehuss.
They will have a look at your PR within the next two weeks and either review your PR or reassign to another reviewer.

Use r? to explicitly pick a reviewer

@rustbot rustbot added A-manifest Area: Cargo.toml issues Command-vendor S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Feb 10, 2025
@epage epage changed the title refactor(embedded): Integrate cargo-script logic into maui refactor(embedded): Integrate cargo-script logic into main parser Feb 10, 2025
Copy link
Contributor

@ehuss ehuss left a comment

Choose a reason for hiding this comment

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

Thanks!

@ehuss ehuss added this pull request to the merge queue Feb 11, 2025
Merged via the queue into rust-lang:master with commit b64750d Feb 11, 2025
21 checks passed
@epage epage deleted the normalize-script branch February 11, 2025 17:40
bors added a commit to rust-lang-ci/rust that referenced this pull request Feb 15, 2025
Update cargo

13 commits in 2928e32734b04925ee51e1ae88bea9a83d2fd451..ce948f4616e3d4277e30c75c8bb01e094910df39
2025-02-07 16:50:22 +0000 to 2025-02-14 20:32:07 +0000
- util: provide a better error message for invalid SSH URLs (rust-lang/cargo#15185)
- Fix the description of the `"root"` field of the `cargo metadata`'s output (rust-lang/cargo#15182)
- refactor: Consolidate creation of SourceId from manifest path (rust-lang/cargo#15172)
- docs(embedded): Note the shebang deviation (rust-lang/cargo#15173)
- refactor(embedded): Integrate cargo-script logic into main parser (rust-lang/cargo#15168)
- feat: implement workspace feature unification (rust-lang/cargo#15157)
- Fix race condition in panic_abort_tests (rust-lang/cargo#15169)
- Update all dependencies (rust-lang/cargo#15166)
- Update curl from 8.9.0 to 8.12.0 (rust-lang/cargo#15162)
- Update annotate-snippets from 0.11.4 to 0.11.5 (rust-lang/cargo#15165)
- Update deny.toml (rust-lang/cargo#15164)
- Update rusqlite from 0.32.1 to 0.33.0 (rust-lang/cargo#15163)
- fix: align first line of unordered list with following (rust-lang/cargo#15161)
@rustbot rustbot added this to the 1.86.0 milestone Feb 15, 2025
RalfJung pushed a commit to RalfJung/miri that referenced this pull request Feb 16, 2025
Update cargo

13 commits in 2928e32734b04925ee51e1ae88bea9a83d2fd451..ce948f4616e3d4277e30c75c8bb01e094910df39
2025-02-07 16:50:22 +0000 to 2025-02-14 20:32:07 +0000
- util: provide a better error message for invalid SSH URLs (rust-lang/cargo#15185)
- Fix the description of the `"root"` field of the `cargo metadata`'s output (rust-lang/cargo#15182)
- refactor: Consolidate creation of SourceId from manifest path (rust-lang/cargo#15172)
- docs(embedded): Note the shebang deviation (rust-lang/cargo#15173)
- refactor(embedded): Integrate cargo-script logic into main parser (rust-lang/cargo#15168)
- feat: implement workspace feature unification (rust-lang/cargo#15157)
- Fix race condition in panic_abort_tests (rust-lang/cargo#15169)
- Update all dependencies (rust-lang/cargo#15166)
- Update curl from 8.9.0 to 8.12.0 (rust-lang/cargo#15162)
- Update annotate-snippets from 0.11.4 to 0.11.5 (rust-lang/cargo#15165)
- Update deny.toml (rust-lang/cargo#15164)
- Update rusqlite from 0.32.1 to 0.33.0 (rust-lang/cargo#15163)
- fix: align first line of unordered list with following (rust-lang/cargo#15161)
github-merge-queue bot pushed a commit that referenced this pull request May 7, 2025
### What does this PR try to resolve?

Cargo script considers itself part of the workspace if the script is
located in a sub-directory of a workspace (presumably since
#15168). This becomes an issue
when using a custom registry that is defined in the `.cargo/config.toml`
within the workspace. Cargo script does not take that file into account
and fails with ``registry index was not found in any configuration:
`test-reg` ``.

### How should we test and review this PR?

This PR adds a regression test and makes cargo script ignore the
surrounding workspace.
The test ~~will fail without the fix in the second commit and~~ can be
used to reproduce the issue.

### Additional information

The issue started occurring with `nightly-2025-02-16`.
Related to #12207.

<!--
Thanks for submitting a pull request 🎉! Here are some tips for you:

* If this is your first contribution, read "Cargo Contribution Guide"
first:
  https://doc.crates.io/contrib/
* Run `cargo fmt --all` to format your code changes.
* Small commits and pull requests are always preferable and easy to
review.
* If your idea is large and needs feedback from the community, read how:
  https://doc.crates.io/contrib/process/#working-on-large-features
* Cargo takes care of compatibility. Read our design principles:
  https://doc.crates.io/contrib/design.html
* When changing help text of cargo commands, follow the steps to
generate docs:

https://github.com/rust-lang/cargo/tree/master/src/doc#building-the-man-pages
* If your PR is not finished, set it as "draft" PR or add "WIP" in its
title.
* It's ok to use the CI resources to test your PR, but please don't
abuse them.

Explain the motivation behind this change.
A clear overview along with an in-depth explanation are helpful.

You can use `Fixes #<issue number>` to associate this PR to an existing
issue.

### How should we test and review this PR?

Demonstrate how you test this change and guide reviewers through your
PR.
With a smooth review process, a pull request usually gets reviewed
quicker.

If you don't know how to write and run your tests, please read the
guide:
https://doc.crates.io/contrib/tests

### Additional information

Other information you want to mention in this PR, such as prior arts,
future extensions, an unresolved problem, or a TODO list.
-->
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-manifest Area: Cargo.toml issues Command-vendor S-waiting-on-review Status: Awaiting review from the assignee but also interested parties.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants