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

feat(toml): Allow adding/removing from cargo scripts #14857

Merged
merged 9 commits into from
Nov 26, 2024
32 changes: 16 additions & 16 deletions src/bin/cargo/commands/remove.rs
Original file line number Diff line number Diff line change
Expand Up @@ -161,8 +161,7 @@ fn parse_section(args: &ArgMatches) -> DepTable {
/// Clean up the workspace.dependencies, profile, patch, and replace sections of the root manifest
/// by removing dependencies which no longer have a reference to them.
fn gc_workspace(workspace: &Workspace<'_>) -> CargoResult<()> {
let mut manifest: toml_edit::DocumentMut =
cargo_util::paths::read(workspace.root_manifest())?.parse()?;
let mut workspace_manifest = LocalManifest::try_new(workspace.root_manifest())?;
let mut is_modified = true;

let members = workspace
Expand All @@ -177,8 +176,8 @@ fn gc_workspace(workspace: &Workspace<'_>) -> CargoResult<()> {

let mut dependencies = members
.into_iter()
.flat_map(|(manifest, unstable_features)| {
manifest
.flat_map(|(member_manifest, unstable_features)| {
member_manifest
.get_sections()
.into_iter()
.flat_map(move |(_, table)| {
Expand All @@ -190,7 +189,7 @@ fn gc_workspace(workspace: &Workspace<'_>) -> CargoResult<()> {
Dependency::from_toml(
workspace.gctx(),
workspace.root(),
&manifest.path,
&member_manifest.path,
&unstable_features,
key,
item,
Expand All @@ -203,7 +202,8 @@ fn gc_workspace(workspace: &Workspace<'_>) -> CargoResult<()> {

// Clean up the workspace.dependencies section and replace instances of
// workspace dependencies with their definitions
if let Some(toml_edit::Item::Table(deps_table)) = manifest
if let Some(toml_edit::Item::Table(deps_table)) = workspace_manifest
.data
.get_mut("workspace")
.and_then(|t| t.get_mut("dependencies"))
{
Expand Down Expand Up @@ -246,7 +246,9 @@ fn gc_workspace(workspace: &Workspace<'_>) -> CargoResult<()> {
// Example tables:
// - profile.dev.package.foo
// - profile.release.package."foo:2.1.0"
if let Some(toml_edit::Item::Table(profile_section_table)) = manifest.get_mut("profile") {
if let Some(toml_edit::Item::Table(profile_section_table)) =
workspace_manifest.data.get_mut("profile")
{
profile_section_table.set_implicit(true);

for (_, item) in profile_section_table.iter_mut() {
Expand Down Expand Up @@ -280,7 +282,7 @@ fn gc_workspace(workspace: &Workspace<'_>) -> CargoResult<()> {
}

// Clean up the replace section
if let Some(toml_edit::Item::Table(table)) = manifest.get_mut("replace") {
if let Some(toml_edit::Item::Table(table)) = workspace_manifest.data.get_mut("replace") {
table.set_implicit(true);

for (key, item) in table.iter_mut() {
Expand All @@ -296,10 +298,7 @@ fn gc_workspace(workspace: &Workspace<'_>) -> CargoResult<()> {
}

if is_modified {
cargo_util::paths::write_atomic(
workspace.root_manifest(),
manifest.to_string().as_bytes(),
)?;
workspace_manifest.write()?;
}

Ok(())
Expand Down Expand Up @@ -340,12 +339,13 @@ fn spec_has_match(

/// Removes unused patches from the manifest
fn gc_unused_patches(workspace: &Workspace<'_>, resolve: &Resolve) -> CargoResult<bool> {
let mut manifest: toml_edit::DocumentMut =
cargo_util::paths::read(workspace.root_manifest())?.parse()?;
let mut workspace_manifest = LocalManifest::try_new(workspace.root_manifest())?;
let mut modified = false;

// Clean up the patch section
if let Some(toml_edit::Item::Table(patch_section_table)) = manifest.get_mut("patch") {
if let Some(toml_edit::Item::Table(patch_section_table)) =
workspace_manifest.data.get_mut("patch")
{
patch_section_table.set_implicit(true);

for (_, item) in patch_section_table.iter_mut() {
Expand Down Expand Up @@ -383,7 +383,7 @@ fn gc_unused_patches(workspace: &Workspace<'_>, resolve: &Resolve) -> CargoResul
}

if modified {
cargo_util::paths::write(workspace.root_manifest(), manifest.to_string().as_bytes())?;
workspace_manifest.write()?;
}

Ok(modified)
Expand Down
182 changes: 100 additions & 82 deletions src/cargo/util/toml/embedded.rs
Original file line number Diff line number Diff line change
Expand Up @@ -21,9 +21,9 @@ pub(super) fn expand_manifest(
path: &std::path::Path,
gctx: &GlobalContext,
) -> CargoResult<String> {
let source = split_source(content)?;
if let Some(frontmatter) = source.frontmatter {
match source.info {
let source = ScriptSource::parse(content)?;
if let Some(frontmatter) = source.frontmatter() {
match source.info() {
Some("cargo") | None => {}
Some(other) => {
if let Some(remainder) = other.strip_prefix("cargo,") {
Expand All @@ -50,15 +50,15 @@ pub(super) fn expand_manifest(
)
.into_path_unlocked();
let mut hacked_source = String::new();
if let Some(shebang) = source.shebang {
if let Some(shebang) = source.shebang() {
writeln!(hacked_source, "{shebang}")?;
}
writeln!(hacked_source)?; // open
for _ in 0..frontmatter.lines().count() {
writeln!(hacked_source)?;
}
writeln!(hacked_source)?; // close
writeln!(hacked_source, "{}", source.content)?;
writeln!(hacked_source, "{}", source.content())?;
if let Some(parent) = hacked_path.parent() {
cargo_util::paths::create_dir_all(parent)?;
}
Expand Down Expand Up @@ -189,94 +189,112 @@ fn sanitize_name(name: &str) -> String {
}

#[derive(Debug)]
struct Source<'s> {
pub struct ScriptSource<'s> {
shebang: Option<&'s str>,
info: Option<&'s str>,
frontmatter: Option<&'s str>,
content: &'s str,
}

fn split_source(input: &str) -> CargoResult<Source<'_>> {
let mut source = Source {
shebang: None,
info: None,
frontmatter: None,
content: input,
};
impl<'s> ScriptSource<'s> {
pub fn parse(input: &'s str) -> CargoResult<Self> {
let mut source = Self {
shebang: None,
info: None,
frontmatter: None,
content: input,
};

// See rust-lang/rust's compiler/rustc_lexer/src/lib.rs's `strip_shebang`
// Shebang must start with `#!` literally, without any preceding whitespace.
// For simplicity we consider any line starting with `#!` a shebang,
// regardless of restrictions put on shebangs by specific platforms.
if let Some(rest) = source.content.strip_prefix("#!") {
// Ok, this is a shebang but if the next non-whitespace token is `[`,
// then it may be valid Rust code, so consider it Rust code.
if rest.trim_start().starts_with('[') {
return Ok(source);
}
Comment on lines +213 to +217
Copy link
Contributor

Choose a reason for hiding this comment

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

Just a comment: This isn't quite the same behavior, as the compiler allows comments before the opening [. I realize parsing that would be difficult, and that particular scenario is a bit silly.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is existing code, just moved. Ok if we put this in the tracking issue or an issue and track separately?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Added this to the checklist: #12207


// See rust-lang/rust's compiler/rustc_lexer/src/lib.rs's `strip_shebang`
// Shebang must start with `#!` literally, without any preceding whitespace.
// For simplicity we consider any line starting with `#!` a shebang,
// regardless of restrictions put on shebangs by specific platforms.
if let Some(rest) = source.content.strip_prefix("#!") {
// Ok, this is a shebang but if the next non-whitespace token is `[`,
// then it may be valid Rust code, so consider it Rust code.
if rest.trim_start().starts_with('[') {
return Ok(source);
// No other choice than to consider this a shebang.
let newline_end = source
.content
.find('\n')
.map(|pos| pos + 1)
.unwrap_or(source.content.len());
let (shebang, content) = source.content.split_at(newline_end);
source.shebang = Some(shebang);
source.content = content;
}

// No other choice than to consider this a shebang.
let newline_end = source
.content
.find('\n')
.map(|pos| pos + 1)
const FENCE_CHAR: char = '-';

let mut trimmed_content = source.content;
while !trimmed_content.is_empty() {
let c = trimmed_content;
let c = c.trim_start_matches([' ', '\t']);
let c = c.trim_start_matches(['\r', '\n']);
if c == trimmed_content {
break;
}
trimmed_content = c;
}
let fence_end = trimmed_content
.char_indices()
.find_map(|(i, c)| (c != FENCE_CHAR).then_some(i))
.unwrap_or(source.content.len());
let (shebang, content) = source.content.split_at(newline_end);
source.shebang = Some(shebang);
let (fence_pattern, rest) = match fence_end {
0 => {
return Ok(source);
}
1 | 2 => {
anyhow::bail!(
"found {fence_end} `{FENCE_CHAR}` in rust frontmatter, expected at least 3"
)
}
_ => trimmed_content.split_at(fence_end),
};
let (info, content) = rest.split_once("\n").unwrap_or((rest, ""));
let info = info.trim();
if !info.is_empty() {
source.info = Some(info);
}
source.content = content;
}

const FENCE_CHAR: char = '-';
let Some((frontmatter, content)) = source.content.split_once(fence_pattern) else {
anyhow::bail!("no closing `{fence_pattern}` found for frontmatter");
};
source.frontmatter = Some(frontmatter);
source.content = content;

let mut trimmed_content = source.content;
while !trimmed_content.is_empty() {
let c = trimmed_content;
let c = c.trim_start_matches([' ', '\t']);
let c = c.trim_start_matches(['\r', '\n']);
if c == trimmed_content {
break;
let (line, content) = source
.content
.split_once("\n")
.unwrap_or((source.content, ""));
let line = line.trim();
if !line.is_empty() {
anyhow::bail!("unexpected trailing content on closing fence: `{line}`");
}
trimmed_content = c;
source.content = content;

Ok(source)
}
let fence_end = trimmed_content
.char_indices()
.find_map(|(i, c)| (c != FENCE_CHAR).then_some(i))
.unwrap_or(source.content.len());
let (fence_pattern, rest) = match fence_end {
0 => {
return Ok(source);
}
1 | 2 => {
anyhow::bail!(
"found {fence_end} `{FENCE_CHAR}` in rust frontmatter, expected at least 3"
)
}
_ => trimmed_content.split_at(fence_end),
};
let (info, content) = rest.split_once("\n").unwrap_or((rest, ""));
let info = info.trim();
if !info.is_empty() {
source.info = Some(info);

pub fn shebang(&self) -> Option<&'s str> {
self.shebang
}
source.content = content;

let Some((frontmatter, content)) = source.content.split_once(fence_pattern) else {
anyhow::bail!("no closing `{fence_pattern}` found for frontmatter");
};
source.frontmatter = Some(frontmatter);
source.content = content;

let (line, content) = source
.content
.split_once("\n")
.unwrap_or((source.content, ""));
let line = line.trim();
if !line.is_empty() {
anyhow::bail!("unexpected trailing content on closing fence: `{line}`");
pub fn info(&self) -> Option<&'s str> {
self.info
}
source.content = content;

Ok(source)
pub fn frontmatter(&self) -> Option<&'s str> {
self.frontmatter
}

pub fn content(&self) -> &'s str {
self.content
}
}

#[cfg(test)]
Expand All @@ -291,16 +309,16 @@ mod test_expand {
fn assert_source(source: &str, expected: impl IntoData) {
use std::fmt::Write as _;

let actual = match split_source(source) {
let actual = match ScriptSource::parse(source) {
Ok(actual) => actual,
Err(err) => panic!("unexpected err: {err}"),
};

let mut rendered = String::new();
write_optional_field(&mut rendered, "shebang", actual.shebang);
write_optional_field(&mut rendered, "info", actual.info);
write_optional_field(&mut rendered, "frontmatter", actual.frontmatter);
writeln!(&mut rendered, "content: {:?}", actual.content).unwrap();
write_optional_field(&mut rendered, "shebang", actual.shebang());
write_optional_field(&mut rendered, "info", actual.info());
write_optional_field(&mut rendered, "frontmatter", actual.frontmatter());
writeln!(&mut rendered, "content: {:?}", actual.content()).unwrap();
assert_data_eq!(rendered, expected.raw());
}

Expand Down Expand Up @@ -497,7 +515,7 @@ content: "\nfn main() {}"
#[test]
fn split_too_few_dashes() {
assert_err(
split_source(
ScriptSource::parse(
r#"#!/usr/bin/env cargo
--
[dependencies]
Expand All @@ -513,7 +531,7 @@ fn main() {}
#[test]
fn split_mismatched_dashes() {
assert_err(
split_source(
ScriptSource::parse(
r#"#!/usr/bin/env cargo
---
[dependencies]
Expand All @@ -529,7 +547,7 @@ fn main() {}
#[test]
fn split_missing_close() {
assert_err(
split_source(
ScriptSource::parse(
r#"#!/usr/bin/env cargo
---
[dependencies]
Expand Down
2 changes: 2 additions & 0 deletions src/cargo/util/toml/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,8 @@ mod targets;

use self::targets::to_targets;

pub use embedded::ScriptSource;

/// See also `bin/cargo/commands/run.rs`s `is_manifest_command`
pub fn is_embedded(path: &Path) -> bool {
let ext = path.extension();
Expand Down
2 changes: 2 additions & 0 deletions src/cargo/util/toml_mut/dependency.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1252,6 +1252,8 @@ mod tests {
let mut local = LocalManifest {
path: crate_root.clone(),
manifest,
embedded: None,
raw: toml.to_owned(),
};
assert_eq!(local.manifest.to_string(), toml);
let gctx = GlobalContext::default().unwrap();
Expand Down
Loading