-
Notifications
You must be signed in to change notification settings - Fork 2.8k
Stabilize cargo script #16569
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
base: master
Are you sure you want to change the base?
Stabilize cargo script #16569
Conversation
|
r? @weihanglo rustbot has assigned @weihanglo. Use |
src/bin/cargo/commands/run.rs
Outdated
| match manifest_path.is_file() { | ||
| true => {} |
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.
Cleaned up in the next commit
| // TODO: remove once frontmatter hits nightly | ||
| if unit.pkg.manifest().is_embedded() { |
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.
Blocked on rust-lang/rust#148051 hitting nightly
| // TODO: remove once frontmatter hits nightly | ||
| if unit.pkg.manifest().is_embedded() { |
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.
Blocked on rust-lang/rust#148051 hitting nightly
|
@rfcbot fcp merge T-cargo For me, my biggest concerns are the first two Risks listed. |
|
Team member @epage has proposed to merge this. The next step is review by the rest of the tagged team members: No concerns currently listed. Once a majority of reviewers approve (and at most 2 approvals are outstanding), this will enter its final comment period. If you spot a major issue that hasn't been raised at any point in this process, please speak up! See this document for info about what commands tagged team members can give me. |
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.
Should we inform rustfmt team that we're going to stabilize this and leave frontmatter style issue unresolved? I remembered that let-else has caused confusions and frustrations when we stabilized it in the language syntax side but rustfmt didn't format it: rust-lang/rustfmt#4914.
At least the rustfmt team should be aware of the potential incoming volume of issues regarding this.
| in the [TOML] format. It contains metadata that is needed to compile the package. Checkout | ||
| the `cargo locate-project` section for more detail on how cargo finds the manifest file. | ||
|
|
||
| The content of the manifest can also be embedded inside of the frontmatter inside of a Rust source file: |
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.
Not a blocker but not satisfied with the current stable doc. I would love to see a dedicated section for Cargo scripts with some examples, and all other mentions of "script" to link back to that section. Currently it is a bit not obvious when it is mentioned.
The original unstable docs has some good stuff in it but maybe it depends on many stuff we want to commit as "stable"
(Or it was migrated to elsewhere? The diff is too large that I could have missed it)
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.
Was unsure how to handle it. A guide? If so, where does it fit in. A reference? Does it get its own page or a section on manifests?
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.
Or a reference-like doc in src/doc/man/cargo.md, and we have another standalone guide? In manifest is not too bad but script is more than manifest.
All these teams actually |
This is worth calling out in user doc that whether the target directory of script is considered implementation details. |
This was handled as part of the Frontmatter stabilization. |
| in the [TOML] format. It contains metadata that is needed to compile the package. Checkout | ||
| the `cargo locate-project` section for more detail on how cargo finds the manifest file. | ||
|
|
||
| The content of the manifest can also be embedded inside of the frontmatter inside of a Rust source file: |
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.
Or a reference-like doc in src/doc/man/cargo.md, and we have another standalone guide? In manifest is not too bad but script is more than manifest.
### What does this PR try to resolve? As found on #16569 ### How to test and review this PR?
This is to make space for discussing scripts, see rust-lang#16569
### What does this PR try to resolve? This is to make space for discussing scripts, see #16569 ### How to test and review this PR?
Fixes #12207
RFC: https://rust-lang.github.io/rfcs/3502-cargo-script.html
Tracking issue: #12207
Frontmatter stabilization: rust-lang/rust#148051
This stabilizes an MVP of Cargo script
.rsextensionCargo.tomlpackage.nameis optional, defaults to the file stempackage.editiondefaults to current edition, not 2015 edition, with a warningpackage.build,package.links,package.default-run,package.workspace,package.auto-*are disallowedworkspace,build-dependencies, and any build-target tables are disallowedpackage.namedefaulting logicpackageUnicodeXIDcharacters with-or_contestselfdepscargo fooprecedence:.rsextension or a path component is present (e.g../foo)cargo installerrors on publishing a scriptcargo packageerrors on publishing a scriptcargo publisherrors on publishing a scriptarg0is set on a best-effort basis to the script's pathCARGO_MANIFEST_PATHwas previously stabilized in addition to the existingCARGO_MANIFEST_DIRcargo fooonly loads configCARGO_HOMEcargo runloads from current dirWhats missing:
Known issues:
cargo removeprints the "unset edition" warning twice#!and[]and not commentsRisks
build-diracross projects, then multiple scripts will point to the sameCargo.lock, fighting over what is locked in itcargo fooPATHthat makes this not workFuture possibilities
target/#13136)package.nameoptional #12689cargo Cargo.tomlsupport