Skip to content

Add cargo pkgid for cargo-script #14831

@0xPoe

Description

@0xPoe
Member

Problem

ref #12207 (comment)

For now, if you try to use cargo pkgid against to a cargo script file. It will generate an error:

cargo pkgid sql2.rs
error: could not find `Cargo.toml` in `/Users/rustin/code/rs-scripts` or any parent directory

Proposed Solution

We need to support cargo pkgid for cargo-script.

For instance, for itself, it would look like:

$pwd
/Users/rustin/code/rs-scripts

$ls
sql.rs  sql1.rs sql2.rs sql3.rs sql4.rs sql5.rs sql6.rs sql7.rs sql8.rs sql9.rs

$cargo pkgid sql2.rs
path+file:///Users/rustin/code/rs-scripts/sql2.rs#embedded

For its dependencies, it would look like:

cargo pkgid sql2.rs -p clap

https://github.com/rust-lang/crates.io-index#clap:4.5.21

Notes

Not sure specifying the rust file as an argument is a good idea. Because cargo pkgid itself already takes the SPEC as the argument. So it might be a very confusing syntax.

Activity

added
C-feature-requestCategory: proposal for a feature. Before PR, ping rust-lang/cargo if this is not `Feature accepted`
S-triageStatus: This issue is waiting on initial triage.
on Nov 16, 2024
weihanglo

weihanglo commented on Nov 16, 2024

@weihanglo
Member

I've tried thinking it hard but not sure about the use case of getting pkgid from a cargo script. Mind sharing more on your thought?

added
S-needs-infoStatus: Needs more info, such as a reproduction or more background for a feature request.
Z-scriptNightly: cargo script
and removed
S-triageStatus: This issue is waiting on initial triage.
on Nov 16, 2024
epage

epage commented on Nov 16, 2024

@epage
Contributor

This is less about the specific command and more about all of the places PackageIdSpec is exposed to the user and making sure we define something that we want to stabilize rather than accidently leaking the current implementation and being stuck.

weihanglo

weihanglo commented on Nov 17, 2024

@weihanglo
Member

Regarding that, I feel like this should not block the stabilization. Unless there is a clear use case we want to support, I'd suggest we make sure pkgid not leaking when stabilization.
Adding an explicit unsupported statement is also helpful, since we can mark it as a bug if we any unexpected stuff is exposed.

0xPoe

0xPoe commented on Nov 18, 2024

@0xPoe
MemberAuthor

Regarding that, I feel like this should not block the stabilization. Unless there is a clear use case we want to support, I'd suggest we make sure pkgid not leaking when stabilization.

I think a reasonable use case is that we don't include the cargo.lock file under the script file dir. So there is no good way to know which version your dependency is using. So can we just get it with this command and use that specification?

epage

epage commented on Nov 18, 2024

@epage
Contributor

@weihanglo package ids come up in

  • cargo pkgid as an output
    • We could make this command unstable for cargo-script, see also --package
  • cargo metadata as an output (fix(metadata): Stabilize id format as PackageIDSpec  #12914)
    • Taking the stabilized output and making it unstable with certain stable conditions seems like its prone to problems
  • cargo <cmd> --message-format json as an output (fix(json-msg): use pkgid spec in in JSON messages #13311)
    • Same as cargo metadata
  • cargo <cmd> --package as an input
    • While we could make this flag unstable for cargo-script, this comes up in programmatic use cases to reduce the chance of ambiguity due to either virtual manifests or dependencies with the same name as a workspace member (usually from a dependency depending on the registry version of your package)

I don't see it being practical to skip this step to stabilizing this feature.

epage

epage commented on Nov 18, 2024

@epage
Contributor
$ cargo pkgid sql2.rs
path+file:///Users/rustin/code/rs-scripts/sql2.rs#embedded

My hope is we can get away without a # or ? for this. It puts it completely on the Source to then disambiguate whether sql2.rs is a file or a directory.

$ cargo pkgid sql2.rs -p clap

https://github.com/rust-lang/crates.io-index#clap:4.5.21

One problem with this is it is inferring --manifest-path off of spec. That is a very sketchy thing to be doing. If we remove that, this becomes

$ cargo pkgid --manifest-path sql2.rs sql2.rs -p clap

https://github.com/rust-lang/crates.io-index#clap:4.5.21

At which point, the value of using a cargo-script file in a <spec> is greatly diminished because a cargo-script's workspace can only have one member.

Having a short-hand for PackageidSpec is also not required for the first release of cargo script.

I would suggest we defer out a short-hand until someone requests it of us with a use case.

weihanglo

weihanglo commented on Nov 18, 2024

@weihanglo
Member

I don't see it being practical to skip this step to stabilizing this feature.

By practical, did you mean from a technical perspective it's hard to make them out of the stabilization?
I don't object to treating them stabilization blockers, as I am fine with them being unstable for a few more months. It was just these usages don't look like worth waiting for six more months. The current feature set (without pkgid/fmt/clippy) of -Zscript has covered 80% of its use cases. I have no data though.

While we could make this flag unstable for cargo-script, this comes up in programmatic use cases to reduce the chance of ambiguity due to either virtual manifests or dependencies with the same name as a workspace member (usually from a dependency depending on the registry version of your package)

Could you share a practical use case of it? Anyway, I feel like a rename of the script can fix 90% of issues like this.

epage

epage commented on Nov 18, 2024

@epage
Contributor

Could you share a practical use case of it? Anyway, I feel like a rename of the script can fix 90% of issues like this.

To clarify, I'm not saying that using --package is needed for cargo-scripts but that some third-party tools are likely to always pass in --package because of confusion within the workspace. If we then take one of these tools and run them on a cargo-script, they will fail and either the user has to live with that or the maintainer has to detect a cargo-script is being used and special case it.

I don't know if I can name a specific concrete example of one that does this today but that I've personally run into enough --package problems by-hand and programmatically, that I view it as nearly essential for programmatic use.

weihanglo

weihanglo commented on Nov 18, 2024

@weihanglo
Member

Having a short-hand for PackageidSpec is also not required for the first release ofcargo script.

Agree we shouldn't bother short-hand at this moment.

One thing may be related to pkgid is executing a remote script. Something like cargo https://github.com/rust-lang/cargo/path/to/script, where the URL may be a representation of pkgid. Personally I would defer any kind of remote script execution, or even just ban this feature. Whoever want to execute a remote script should use curl + cargo combination, such as

curl --proto '=https' --tlsv1.2 -sSf https://example.com/cargo-script.rs | cargo

And if we ever want to support this combo, what does its pkgid look like?

epage

epage commented on Nov 18, 2024

@epage
Contributor

By practical, did you mean from a technical perspective it's hard to make them out of the stabilization?

Yes.

The current feature set (without pkgid/fmt/clippy) of -Zscript has covered 80% of its use cases. I have no data though.

The stated goal in the RFC is for first-class support. If we wanted something that half-way worked, we could have kept to cargo-script.

I personally am regularly annoyed with the lack of cargo fmt support in each of my throw-away scripts I write. I share these in bug reports and don't want to manually clean up the code for them to be presentable to the user. I've also heard of lots of reports of people who want to replace their long-lived bash and python scripts with Cargo scripts which entails wanting standard tooling compared to a throw-away script.

Granted, how much we have stable in the first release is open for discussion. That depends on what level of quality people will expect from it being stable. However, as I said before, I don't see a way for us to have the package id spec format for cargo-scripts to be unstable.

19 remaining items

Loading
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

Labels

C-feature-requestCategory: proposal for a feature. Before PR, ping rust-lang/cargo if this is not `Feature accepted`S-acceptedStatus: Issue or feature is accepted, and has a team member available to help mentor or reviewZ-scriptNightly: cargo script

Type

No type

Projects

No projects

Milestone

No milestone

Relationships

None yet

    Development

    Participants

    @epage@weihanglo@0xPoe

    Issue actions

      Add `cargo pkgid` for cargo-script · Issue #14831 · rust-lang/cargo