Skip to content

Commit

Permalink
Auto merge of #12245 - epage:script, r=weihanglo
Browse files Browse the repository at this point in the history
feat: Initial support for single-file packages

### What does this PR try to resolve?

This is the first step towards #12207.  In particular, this focuses on pulling in the [demo](https://github.com/epage/cargo-script-mvs) roughly as-is to serve as a baseline for further PRs.  I have a couple months of runtime (multiple times a week) using the version of the demo included here.

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

Commit-by-commit.  Most likely, the last (docs) commit should be done first to provide context for the others.

Naming is hard.  I came up with these terms just so we can have ways to refer to them.  Feedback is welcome.
- `-Zscript`  for this general feature (not great but didn't want to spend too long coming up with a throwaway name)
- "single-file package": Rust code and a cargo manifest in a single file
- "embedded manifest": the explicit manifest inside of a single-file package
- "manifest command": when we interpret `cargo <name>` as referring to a single-file package, and similar to "built-in commands" and "external commands".

Keep in mind that this is a very hacky solution with many deficiencies and is mostly starting as a baseline for implementing and reviewing those improvements, including
- Switching from `regex` to `syn` for extracting manifests for greater resilience
- Matching `cargo new`s logic when sanitizing the inferred package name
- Allowing `cargo <name>` to also be a `Cargo.toml` file (for consistency in where manifests are accepted)
- Allowing single-file packages to be used wherever a manifest is accepted

To minimize conflict pain, I would ask that we consider what feedback can be handled in a follow up (though still mention it!).  It'll be much easier creating multiple, independent PRs once this baseline is merged to address concerns.

### Additional information

The only affect for people on stable is that they may get a warning if they have an external subcommand that will be shadowed when this feature is implemented.  This will allow us to collect feedback, without blocking people, so we can have an idea of how "safe" our precedence scheme is for interpreting `cargo <name>`.

As of right now, aliases with a `.` in them will be ignored (well, technically their suffix will be exposed as an alias).    We directly inject the name into a lookup string into the config  that uses `.` as the separator, so we drill down until we get to the leaf element.  Ideally, we would be generating / parsing the lookup key using TOML key syntax so we can better report that this won't be supported after this change :)
  • Loading branch information
bors committed Jun 12, 2023
2 parents 18a28a1 + 6b0b5a8 commit f8fef7a
Show file tree
Hide file tree
Showing 12 changed files with 1,681 additions and 44 deletions.
65 changes: 58 additions & 7 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

7 changes: 7 additions & 0 deletions Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ exclude = [
[workspace.dependencies]
anyhow = "1.0.47"
base64 = "0.21.0"
blake3 = "1.3.3"
bytesize = "1.0"
cargo = { path = "" }
cargo-credential = { version = "0.2.0", path = "credential/cargo-credential" }
Expand Down Expand Up @@ -66,6 +67,7 @@ pretty_env_logger = "0.4"
proptest = "1.1.0"
pulldown-cmark = { version = "0.9.2", default-features = false }
rand = "0.8.5"
regex = "1.8.3"
rustfix = "0.6.0"
same-file = "1.0.6"
security-framework = "2.0.0"
Expand All @@ -79,6 +81,7 @@ sha2 = "0.10.6"
shell-escape = "0.1.4"
snapbox = { version = "0.4.0", features = ["diff", "path"] }
strip-ansi-escapes = "0.1.0"
syn = { version = "2.0.14", features = ["extra-traits", "full"] }
tar = { version = "0.4.38", default-features = false }
tempfile = "3.1.0"
termcolor = "1.1.2"
Expand Down Expand Up @@ -112,6 +115,7 @@ path = "src/cargo/lib.rs"
[dependencies]
anyhow.workspace = true
base64.workspace = true
blake3.workspace = true
bytesize.workspace = true
cargo-platform.workspace = true
cargo-util.workspace = true
Expand Down Expand Up @@ -147,7 +151,9 @@ os_info.workspace = true
pasetors.workspace = true
pathdiff.workspace = true
pretty_env_logger = { workspace = true, optional = true }
pulldown-cmark.workspace = true
rand.workspace = true
regex.workspace = true
rustfix.workspace = true
semver.workspace = true
serde = { workspace = true, features = ["derive"] }
Expand All @@ -157,6 +163,7 @@ serde_json = { workspace = true, features = ["raw_value"] }
sha1.workspace = true
shell-escape.workspace = true
strip-ansi-escapes.workspace = true
syn.workspace = true
tar.workspace = true
tempfile.workspace = true
termcolor.workspace = true
Expand Down
4 changes: 3 additions & 1 deletion crates/cargo-test-support/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -110,7 +110,9 @@ impl FileBuilder {

fn mk(&mut self) {
if self.executable {
self.path.set_extension(env::consts::EXE_EXTENSION);
let mut path = self.path.clone().into_os_string();
write!(path, "{}", env::consts::EXE_SUFFIX).unwrap();
self.path = path.into();
}

self.dirname().mkdir_p();
Expand Down
1 change: 1 addition & 0 deletions deny.toml
Original file line number Diff line number Diff line change
Expand Up @@ -106,6 +106,7 @@ allow = [
"MIT-0",
"Apache-2.0",
"BSD-3-Clause",
"BSD-2-Clause",
"MPL-2.0",
"Unicode-DFS-2016",
"CC0-1.0",
Expand Down
91 changes: 77 additions & 14 deletions src/bin/cargo/cli.rs
Original file line number Diff line number Diff line change
Expand Up @@ -258,7 +258,7 @@ fn expand_aliases(
args: ArgMatches,
mut already_expanded: Vec<String>,
) -> Result<(ArgMatches, GlobalArgs), CliError> {
if let Some((cmd, args)) = args.subcommand() {
if let Some((cmd, sub_args)) = args.subcommand() {
let exec = commands::builtin_exec(cmd);
let aliased_cmd = super::aliased_command(config, cmd);

Expand All @@ -274,7 +274,7 @@ fn expand_aliases(
// Here we ignore errors from aliasing as we already favor built-in command,
// and alias doesn't involve in this context.

if let Some(values) = args.get_many::<OsString>("") {
if let Some(values) = sub_args.get_many::<OsString>("") {
// Command is built-in and is not conflicting with alias, but contains ignored values.
return Err(anyhow::format_err!(
"\
Expand Down Expand Up @@ -305,17 +305,34 @@ For more information, see issue #10049 <https://github.com/rust-lang/cargo/issue
))?;
}
}
if commands::run::is_manifest_command(cmd) {
if config.cli_unstable().script {
return Ok((args, GlobalArgs::default()));
} else {
config.shell().warn(format_args!(
"\
user-defined alias `{cmd}` has the appearance of a manfiest-command
This was previously accepted but will be phased out when `-Zscript` is stabilized.
For more information, see issue #12207 <https://github.com/rust-lang/cargo/issues/12207>."
))?;
}
}

let mut alias = alias
.into_iter()
.map(|s| OsString::from(s))
.collect::<Vec<_>>();
alias.extend(args.get_many::<OsString>("").unwrap_or_default().cloned());
alias.extend(
sub_args
.get_many::<OsString>("")
.unwrap_or_default()
.cloned(),
);
// new_args strips out everything before the subcommand, so
// capture those global options now.
// Note that an alias to an external command will not receive
// these arguments. That may be confusing, but such is life.
let global_args = GlobalArgs::new(args);
let global_args = GlobalArgs::new(sub_args);
let new_args = cli().no_binary_name(true).try_get_matches_from(alias)?;

let new_cmd = new_args.subcommand_name().expect("subcommand is required");
Expand Down Expand Up @@ -382,19 +399,54 @@ fn config_configure(
Ok(())
}

/// Precedence isn't the most obvious from this function because
/// - Some is determined by `expand_aliases`
/// - Some is enforced by `avoid_ambiguity_between_builtins_and_manifest_commands`
///
/// In actuality, it is:
/// 1. built-ins xor manifest-command
/// 2. aliases
/// 3. external subcommands
fn execute_subcommand(config: &mut Config, cmd: &str, subcommand_args: &ArgMatches) -> CliResult {
if let Some(exec) = commands::builtin_exec(cmd) {
return exec(config, subcommand_args);
}

let mut ext_args: Vec<&OsStr> = vec![OsStr::new(cmd)];
ext_args.extend(
subcommand_args
.get_many::<OsString>("")
.unwrap_or_default()
.map(OsString::as_os_str),
);
super::execute_external_subcommand(config, cmd, &ext_args)
if commands::run::is_manifest_command(cmd) {
let ext_path = super::find_external_subcommand(config, cmd);
if !config.cli_unstable().script && ext_path.is_some() {
config.shell().warn(format_args!(
"\
external subcommand `{cmd}` has the appearance of a manfiest-command
This was previously accepted but will be phased out when `-Zscript` is stabilized.
For more information, see issue #12207 <https://github.com/rust-lang/cargo/issues/12207>.",
))?;
let mut ext_args = vec![OsStr::new(cmd)];
ext_args.extend(
subcommand_args
.get_many::<OsString>("")
.unwrap_or_default()
.map(OsString::as_os_str),
);
super::execute_external_subcommand(config, cmd, &ext_args)
} else {
let ext_args: Vec<OsString> = subcommand_args
.get_many::<OsString>("")
.unwrap_or_default()
.cloned()
.collect();
commands::run::exec_manifest_command(config, cmd, &ext_args)
}
} else {
let mut ext_args = vec![OsStr::new(cmd)];
ext_args.extend(
subcommand_args
.get_many::<OsString>("")
.unwrap_or_default()
.map(OsString::as_os_str),
);
super::execute_external_subcommand(config, cmd, &ext_args)
}
}

#[derive(Default)]
Expand Down Expand Up @@ -438,9 +490,9 @@ pub fn cli() -> Command {
#[allow(clippy::disallowed_methods)]
let is_rustup = std::env::var_os("RUSTUP_HOME").is_some();
let usage = if is_rustup {
"cargo [+toolchain] [OPTIONS] [COMMAND]"
"cargo [+toolchain] [OPTIONS] [COMMAND]\n cargo [+toolchain] [OPTIONS] -Zscript <MANIFEST_RS> [ARGS]..."
} else {
"cargo [OPTIONS] [COMMAND]"
"cargo [OPTIONS] [COMMAND]\n cargo [OPTIONS] -Zscript <MANIFEST_RS> [ARGS]..."
};
Command::new("cargo")
// Subcommands all count their args' display order independently (from 0),
Expand Down Expand Up @@ -570,3 +622,14 @@ impl LazyConfig {
fn verify_cli() {
cli().debug_assert();
}

#[test]
fn avoid_ambiguity_between_builtins_and_manifest_commands() {
for cmd in commands::builtin() {
let name = cmd.get_name();
assert!(
!commands::run::is_manifest_command(&name),
"built-in command {name} is ambiguous with manifest-commands"
)
}
}
Loading

0 comments on commit f8fef7a

Please sign in to comment.