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.

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

eRFC: single-file packages ("cargo script") integration #3424

Merged
merged 13 commits into from
May 31, 2023

Conversation

epage
Copy link
Contributor

@epage epage commented Apr 26, 2023

This experimental RFC adds unstable support for single-file
packages in cargo so we can explore the design and resolve
questions with an implementation to collect feedback on.

Single-file packages are .rs files with an embedded
manifest. These will be accepted with just like Cargo.toml files with
--manifest-path. cargo will be modified to accept cargo <file>.rs as a
shortcut to cargo run --manifest-path <file>.rs. This allows placing
cargo in a #! line for directly running these files.

Example:

#!/usr/bin/env cargo

//! ```cargo
//! [dependencies]
//! clap = { version = "4.2", features = ["derive"] }
//! ```

use clap::Parser;

#[derive(Parser, Debug)]
#[clap(version)]
struct Args {
    #[clap(short, long, help = "Path to config")]
    config: Option<std::path::PathBuf>,
}

fn main() {
    let args = Args::parse();
    println!("{:?}", args);
}
$ ./prog --config file.toml
Args { config: Some("file.toml") }

See cargo-script-mvs for a demo.

Rendered

Tracking Issue

@epage epage added the T-cargo Relevant to the Cargo team, which will review and decide on the RFC. label Apr 26, 2023
@epage
Copy link
Contributor Author

epage commented Apr 26, 2023

Not even sure if eRFC's are still a thing but when we talked about this in the cargo team meeting, we felt this format would be better for closing out conversations on all of the various details rather than trying to track and hash them out in an upfront design.

//! ```

fn main() {
println!("Hello, world!");
Copy link
Member

Choose a reason for hiding this comment

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

Nit, but the code here in this example should actually use the added dependency.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Copy link
Contributor Author

Choose a reason for hiding this comment

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

(another concerned raised is how stable the version numbers are)

Comment on lines +24 to +27
//! ```cargo
//! [dependencies]
//! clap = { version = "4.2", features = ["derive"] }
//! ```
Copy link
Member

Choose a reason for hiding this comment

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

Does this need to be a doc comment? Could it just be an ordinary comment? If so, I think it should be, to make it more obvious that this isn't magic syntax.

Suggested change
//! ```cargo
//! [dependencies]
//! clap = { version = "4.2", features = ["derive"] }
//! ```
// ```cargo
// [dependencies]
// clap = { version = "4.2", features = ["derive"] }
// ```

Copy link
Member

Choose a reason for hiding this comment

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

The reference-level explanation should explain that this can be a regular // comment, a /// doc comment, or a //! file-level doc comment.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The strawman examples in the eRFC are using doc-comments. I've called out how we store the embedded manifest as one of the unresolved questions that we plan to resolve before stablization. During the Pre-RFC, we explored a lot of options. I left these out of the eRFC to avoid getting side tracked by things that aren't meant to be resolved at this time.

Copy link
Member

Choose a reason for hiding this comment

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

@epage We don't have to specify all of that now; I'd just like to make sure that people don't assume this has to be a doc comment.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The current proposal is for it to be doc comments (an alternative was to instead accept regular comments). @Muscraft is prototyping the initial implementation of this proposal and has switched to syn to extract the doc comments, finding that our regex parser for the doc comments was accepting cases that aren't turned into #![doc]s.

I've clarified this within the guide section.

Copy link

Choose a reason for hiding this comment

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

Wouldn't it be better to have specific syntax, or a macro, instead?

Whenever languages have comments which are parsed and have effects, it feels a bit uneasy. Things which are commented out should never have any effect, I think.

But maybe this is different, since I suppose rustc really does ignore these comments.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

For more of an idea of what syntaxes we'll consider, see the Pre-RFC. We decided to do an eRFC, rather than a full RFC, and only included a rough sketch to give people an idea of what the experiment will look like, deferring actual decisions to the experiment and approval of the experiment's result.

Comment on lines +181 to +182
Now, if `regex` gets updated, we will still build with the same revision until
we choose to `cargo update --manifest-path hello_world.rs`.
Copy link
Member

Choose a reason for hiding this comment

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

How does this work? Where is this cached? Where does the equivalent of the lockfile live?

This seems a little too opaque.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

That was intentional as lockfiles are another big area for us to try to figure out the right way to handle them is. The Pre-RFC also has a section for this

Copy link
Member

Choose a reason for hiding this comment

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

I'm not suggesting the eRFC has to fully specify embedded lockfiles or any of the many other aspects discussed in the internals thread. I just think if we're going to state that this is cached and updated, we should give some idea of whether this is stored in the cargo home directory (which I'd expect), or next to the script (which I hope it isn't), or similar.

(Also, on a separate note, can we make just cargo update hello_world.rs work?)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'm not suggesting the eRFC has to fully specify embedded lockfiles or any of the many other aspects discussed in the internals thread. I just think if we're going to state that this is cached and updated, we should give some idea of whether this is stored in the cargo home directory (which I'd expect), or next to the script (which I hope it isn't), or similar.

Done

(Also, on a separate note, can we make just cargo update hello_world.rs work?)

My understanding was that cargo intentionally avoided positional arguments (cargo add being an exception) and one reason for this is due to the heavy use of num_args(0..=1) arguments like --timings.

In the Pre-RFC, I did bring up whether we should provide a short-flag for --manifest-path because of how cumbersome it is to use for these files that aren't auto-discovered.

I'll at least call this out as another area to explore.

Copy link

@yerke yerke Apr 27, 2023

Choose a reason for hiding this comment

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

@epage I just read https://github.com/epage/rfcs/blob/cargo-script/text/3424-cargo-script.md, but couldn't find a place where it mentions what happens with lock files at all. I was searching for lock string after reading the whole document. Did I somehow miss it?

Copy link

Choose a reason for hiding this comment

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

Great. I didn’t use cargo metadata before. It will do the trick, albeit in a format that will be unfamiliar for many people.

Copy link

Choose a reason for hiding this comment

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

I looked at both cargo metadata and cargo tree to see if everything contained in a lock file is present. The one thing that I see missing is checksum field. I think it can be pretty useful.

Copy link

Choose a reason for hiding this comment

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

Turns out that checksum is internal implementation checksum useful only to cargo, and is not that useful to end users.

Choose a reason for hiding this comment

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

One thing I don't see mentioned is whether there's only a single lockfile per system, or one per script. If there's a lockfile per script, what happens if a script is renamed? Does that reset all the dependency versions? What about moving a script file to a different directory without renaming it?

Phrased differently, the document says "Cargo will cache the exact information" but doesn't specify what key the cache is indexed by.

Choose a reason for hiding this comment

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

That sounds like we'll need cargo venv lol

}
```
```console
$ ./prog --config file.toml

Choose a reason for hiding this comment

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

Does this console invocation need to have the .rs extension on the filename?

Suggested change
$ ./prog --config file.toml
$ ./prog.rs --config file.toml

I was following along earlier in the pre-rfc thread on IRLO, but wasn't sure whether the .rs extension was still an unresolved question. (I see the other hello_world.rs example does have the .rs filename extension)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Does this console invocation need to have the .rs extension on the filename?

Either one is fine here. For the utility workflows, people don't always like to put extensions on their scripts (independent of the language).

Granted, that can affect what we do for cargo test --manifest-path ./prog. We might not be able to key just off of the extension.

I was following along earlier in the pre-rfc thread on IRLO, but wasn't sure whether the .rs extension was still an unresolved question. (I see the other hello_world.rs example does have the .rs filename extension)

Most of that extension discussion is about what extension would be used when it is present and not so much on the use cases for when to drop it.

Copy link
Member

Choose a reason for hiding this comment

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

Either one is fine here.

How is this supposed to work in general? If I name the file run, then cargo will be invoked as cargo run, which looks like the subcommand and not like a request to execute a file. cargo <file>.rs is at least unambiguous.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The exact precedence rules are left to be explored with the implementation. This is recorded in "Unresolved questions" as

If cargo, what precedence does it have compared to built-in commands, aliases, and external commands.

In epage/cargo-script-mvs#154, I've started looking into prior art among other build tools that support <tool> <file>.

dlang's dub is of particular interest

  1. If the <positional> is - or ends with .rs, treat it as a single-file package
  2. If <positional> matches an existing subcommand, run it
  3. If <positional> exists, treat it as a single-file package
  4. If <positional>.d exists, treat it as a single-file package
  5. Otherwise, treat <positional> as a non-existent subcommand

For me, some tweaks I can see making

  • Don't support "<positional>.d exists" check (4)
  • For (1), check if <positional> has path separators (at least on my system, #! causes an absolute path to be passed to the command)

Copy link
Member

Choose a reason for hiding this comment

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

This is mentioned in the first unresolved question, but the idea is to figure out the precedence. For the prototype I am working on, it has the precedence as:

  • cmd.ends_with(".rs") || 1 < Path::new(cmd).components().count()
  • built-in subcommand/alias
  • external subcommand
  • Path::new(cmd).exists() && is_executable(&path)

Copy link
Member

Choose a reason for hiding this comment

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

  • For (1), check if <positional> has path separators (at least on my system, #! causes an absolute path to be passed to the command)

it doesn't on my system (Android).

Copy link
Member

Choose a reason for hiding this comment

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

Anything involving paths is likely to be system specific. Or at least they'll be edge case systems.

Copy link

@SOF3 SOF3 May 5, 2023

Choose a reason for hiding this comment

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

Either one is fine here. For the utility workflows, people don't always like to put extensions on their scripts (independent of the language).

People can just use a symlink though. In the case of the PHP PHAR format, PHP denies loading phar files that do not have a .phar extension for some reason, so some installations of composer are performed by ln -s composer.phar /usr/bin/composer (citation needed: I know several people who do so, but I am not sure if this is a common practice).

Choose a reason for hiding this comment

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

If I name the file run, then cargo will be invoked as cargo run, which looks like the subcommand and not like a request to execute a file.

cargo -- run?

Does this work?

#!/usr/bin/env cargo -- --

@veber-alex
Copy link

I think it's important to have cargo add support for .rs files, otherwise adding dependencies is not very ergonomic (having to remember the syntax and lookup the version numbers)

@Diggsey
Copy link
Contributor

Diggsey commented May 4, 2023

Of note is that rustc already ignores a shebang at the beginning of a file despite it not being valid Rust code - I hadn't realised that.

@epage
Copy link
Contributor Author

epage commented May 17, 2023

As this is just agreement to do an experiment, interest seems high, and conversation has died down, I feel like its time:

@rfcbot merge

@rfcbot
Copy link
Collaborator

rfcbot commented May 17, 2023

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.

@rfcbot rfcbot added proposed-final-comment-period Currently awaiting signoff of all team members in order to enter the final comment period. disposition-merge This RFC is in PFCP or FCP with a disposition to merge it. final-comment-period Will be merged/postponed/closed in ~10 calendar days unless new substational objections are raised. labels May 17, 2023
@rfcbot rfcbot added finished-final-comment-period The final comment period is finished for this RFC. to-announce and removed final-comment-period Will be merged/postponed/closed in ~10 calendar days unless new substational objections are raised. labels May 31, 2023
@rfcbot
Copy link
Collaborator

rfcbot commented May 31, 2023

The final comment period, with a disposition to merge, as per the review above, is now complete.

As the automated representative of the governance process, I would like to thank the author for their work and everyone else who contributed.

This will be merged soon.

@ehuss
Copy link
Contributor

ehuss commented May 31, 2023

Huzzah! The @rust-lang/cargo team has decided to accept this RFC.

To track further discussion, subscribe to the tracking issue here:
rust-lang/cargo#12207

@ehuss ehuss merged commit 832a070 into rust-lang:master May 31, 2023
@epage epage deleted the cargo-script branch May 31, 2023 13:38
Haskell
- [`runghc` / `runhaskell`](https://downloads.haskell.org/ghc/latest/docs/users_guide/runghc.html)
- Users can use the file stem (ie leave off the extension) when passing it in
- [cabal's single-file haskel script](https://cabal.readthedocs.io/en/stable/getting-started.html#run-a-single-file-haskell-script)
Copy link

Choose a reason for hiding this comment

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

typo here

-haskel
+haskell

@epage
Copy link
Contributor Author

epage commented Jun 27, 2023

Background: we are looking to do a mid-point evaluation of this experiment to see how its going and what might need to be changed about the direction

Testing instructions

Requires

  • nightly: nightly-2023-06-25 or newer
  • beta
  • stable

What is supported

  • cargo being in the shebang line for both .rs and Cargo.toml files
  • Using cargo <cmd> --manifest-path <file.rs> for basic commands (check, test, etc)

What is not supported

  • Some commands like publish and add
  • Workspace support
  • foo.rs being libs

For more see unstable reference documentation

In particular, we are looking to understand:

  • How easy is it to write scripts? Where are the pain points?
  • How easy is it to share scripts? Where are the pain points?
  • Is the default cargo output when running scripts intrusive?
  • Does respecting the local rustup toolchain file cause problems?
  • How is the perceived first build performance? Performance of subsequent runs?
  • Anything that seemed odd or unexpected with the behavior?
  • If you need to run the scripts in release mode and the experience for doing so?

In addition, areas still being explored:

  • How to embed manifests in source
  • How to balance low overhead with not breaking scripts on edition updates
  • Do we need [lib] support? Are a script being [lib] and [[bin]]` mutually exclusive or do we need to support them together?

Please leave feedback on rust-lang/cargo#12207

Examples:

Simplest example is:

#!/usr/bin/env -S cargo +nightly -Zscript

fn main() {}

With dependencies (nightly-2023-10-01 to today):

#!/usr/bin/env -S cargo +nightly -Zscript
```cargo
[dependencies]
clap = { version = "4.2", features = ["derive"] }
```

use clap::Parser;

#[derive(Parser, Debug)]
#[clap(version)]
struct Args {
    #[clap(short, long, help = "Path to config")]
    config: Option<std::path::PathBuf>,
}

fn main() {
    let args = Args::parse();
    println!("{:?}", args);
}

With dependencies (nightly-2023-06-25 to nightly-2023-01-01):

#!/usr/bin/env -S cargo +nightly -Zscript

//! ```cargo
//! [dependencies]
//! clap = { version = "4.2", features = ["derive"] }
//! ```

use clap::Parser;

#[derive(Parser, Debug)]
#[clap(version)]
struct Args {
    #[clap(short, long, help = "Path to config")]
    config: Option<std::path::PathBuf>,
}

fn main() {
    let args = Args::parse();
    println!("{:?}", args);
}

Note: not all env implementations support -S. One workaround is to create a wrapper script. I call mine nargo (nightly + cargo). This would also better simulate the stable experience.

@epage epage added the call-for-testing Add this label + test notes in comments to be included in TWiR `Call for Testing` section. label Jun 27, 2023
@victor-timofei
Copy link

Just migrated from rust-script and it works like a charm. https://git.vtimothy.com/vtimofei/dotfiles/commit/5eaf73a001f86cda73e5332937a6c6187a0f47a5

@victor-timofei
Copy link

I tried to use a library with the below structure:

#!/usr/bin/env -S cargo +nightly -Zscript

//! ```cargo
//! [dependencies]
//! hello_lib = { path = "./hello_lib" }
//! ```

use hello_lib::greet;

fn main() {

    println!("{}", greet());
}
$ tree
.
├── hello_lib
│   ├── Cargo.lock
│   ├── Cargo.toml
│   ├── src
│   │   └── lib.rs
└── script.rs
$ cat hello_lib/src/lib.rs
pub fn greet() -> String {
    "Howdy".into()
}

I think using libraries this way is a viable option, and support for [lib] wouldn't be required.

@U007D U007D removed the call-for-testing Add this label + test notes in comments to be included in TWiR `Call for Testing` section. label Jun 28, 2023
@U007D
Copy link

U007D commented Jun 28, 2023

This RFC will appear in the Call for Testing section of the next issue (# 501) of This Week in Rust (TWiR).

I have removed the call-for-testing label. Please feel free to re-add the label if you would like this RFC to appear again in another issue of TWiR.

@epage
Copy link
Contributor Author

epage commented Jun 28, 2023

@victor-timofei

I think using libraries this way is a viable option, and support for [lib] wouldn't be required.

The idea for single-file lib packages is to reduce the boilerplate for writing [lib] packages, especially in workspaces.

@victor-timofei
Copy link

@epage thanks for clearing this out. That would be indeed handy, but I see it mostly as a nice to have.

BTW, where would be a good place to give feedback for this?

@epage
Copy link
Contributor Author

epage commented Jun 28, 2023

@victor-timofei

BTW, where would be a good place to give feedback for this?

Most activity should be on rust-lang/cargo#12207. I posted the testing instructions here because this is a repo TWiR monitors.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
disposition-merge This RFC is in PFCP or FCP with a disposition to merge it. finished-final-comment-period The final comment period is finished for this RFC. T-cargo Relevant to the Cargo team, which will review and decide on the RFC. to-announce
Projects
No open projects
Status: Implemented
Development

Successfully merging this pull request may close these issues.