Skip to content

Commit

Permalink
Auto merge of #10086 - epage:toml, r=ehuss
Browse files Browse the repository at this point in the history
Port cargo from toml-rs to toml_edit

Benefits:
- A TOML 1.0 compliant parser
- Unblock future work
  - Have `cargo init` add the current crate to the workspace, rather
    than error
  - #5586: Upstream `cargo-add`

TODO
- [x] Analyze performance and address regressions
- [x] Identify and resolve incompatibiies
- [x] Resolve remaining test failures, see
      https://github.com/ordian/toml_edit/labels/cargo
- [x] ~~Switch the code from #10176 to only parse once~~ (this PR is being merged first)
  • Loading branch information
bors committed Jan 20, 2022
2 parents 7e89966 + 320c279 commit bb96b3a
Show file tree
Hide file tree
Showing 34 changed files with 176 additions and 38 deletions.
2 changes: 1 addition & 1 deletion Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,7 @@ strip-ansi-escapes = "0.1.0"
tar = { version = "0.4.36", default-features = false }
tempfile = "3.0"
termcolor = "1.1"
toml = "0.5.7"
toml_edit = { version = "0.13", features = ["serde", "easy"] }
unicode-xid = "0.2.0"
url = "2.2.2"
walkdir = "2.2"
Expand Down
2 changes: 1 addition & 1 deletion benches/capture/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -9,4 +9,4 @@ description = "Tool for capturing a real-world workspace for benchmarking."
cargo_metadata = "0.14.0"
flate2 = { version = "1.0.3", default-features = false, features = ["zlib"] }
tar = { version = "0.4.35", default-features = false }
toml = "0.5.8"
toml_edit = { version = "0.9.1", features = ["easy"] }
1 change: 1 addition & 0 deletions benches/capture/src/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ use flate2::{Compression, GzBuilder};
use std::fs;
use std::path::{Path, PathBuf};
use std::process::Command;
use toml_edit::easy as toml;

fn main() {
let force = std::env::args().any(|arg| arg == "-f");
Expand Down
2 changes: 1 addition & 1 deletion crates/cargo-test-support/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ remove_dir_all = "0.5"
serde_json = "1.0"
tar = { version = "0.4.18", default-features = false }
termcolor = "1.1.2"
toml = "0.5.7"
toml_edit = { version = "0.9.1", features = ["serde", "easy"] }
url = "2.2.2"

[features]
Expand Down
2 changes: 1 addition & 1 deletion crates/cargo-test-support/src/registry.rs
Original file line number Diff line number Diff line change
Expand Up @@ -709,7 +709,7 @@ impl Package {
if !self.cargo_features.is_empty() {
manifest.push_str(&format!(
"cargo-features = {}\n\n",
toml::to_string(&self.cargo_features).unwrap()
toml_edit::ser::to_item(&self.cargo_features).unwrap()
));
}

Expand Down
1 change: 1 addition & 0 deletions src/cargo/core/manifest.rs
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ use anyhow::Context as _;
use semver::Version;
use serde::ser;
use serde::Serialize;
use toml_edit::easy as toml;
use url::Url;

use crate::core::compiler::{CompileKind, CrateType};
Expand Down
3 changes: 2 additions & 1 deletion src/cargo/core/package.rs
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ use lazycell::LazyCell;
use log::{debug, warn};
use semver::Version;
use serde::Serialize;
use toml_edit::easy as toml;

use crate::core::compiler::{CompileKind, RustcTargetData};
use crate::core::dependency::DepKind;
Expand Down Expand Up @@ -199,7 +200,7 @@ impl Package {
.manifest()
.original()
.prepare_for_publish(ws, self.root())?;
let toml = toml::to_string(&manifest)?;
let toml = toml::to_string_pretty(&manifest)?;
Ok(format!("{}\n{}", MANIFEST_PREAMBLE, toml))
}

Expand Down
1 change: 1 addition & 0 deletions src/cargo/core/workspace.rs
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ use anyhow::{bail, Context as _};
use glob::glob;
use itertools::Itertools;
use log::debug;
use toml_edit::easy as toml;
use url::Url;

use crate::core::features::Features;
Expand Down
13 changes: 9 additions & 4 deletions src/cargo/ops/cargo_config.rs
Original file line number Diff line number Diff line change
Expand Up @@ -127,19 +127,24 @@ fn print_toml(config: &Config, opts: &GetOptions<'_>, key: &ConfigKey, cv: &CV)
config,
"{} = {}{}",
key,
toml::to_string(&val).unwrap(),
toml_edit::Value::from(val),
origin(def)
),
CV::List(vals, _def) => {
if opts.show_origin {
drop_println!(config, "{} = [", key);
for (val, def) in vals {
drop_println!(config, " {}, # {}", toml::to_string(&val).unwrap(), def);
drop_println!(
config,
" {}, # {}",
toml_edit::ser::to_item(&val).unwrap(),
def
);
}
drop_println!(config, "]");
} else {
let vals: Vec<&String> = vals.iter().map(|x| &x.0).collect();
drop_println!(config, "{} = {}", key, toml::to_string(&vals).unwrap());
let vals: toml_edit::Array = vals.iter().map(|x| &x.0).collect();
drop_println!(config, "{} = {}", key, vals);
}
}
CV::Table(table, _def) => {
Expand Down
1 change: 1 addition & 0 deletions src/cargo/ops/cargo_new.rs
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ use std::io::{BufRead, BufReader, ErrorKind};
use std::path::{Path, PathBuf};
use std::process::Command;
use std::str::{from_utf8, FromStr};
use toml_edit::easy as toml;

#[derive(Clone, Copy, Debug, PartialEq)]
pub enum VersionControl {
Expand Down
1 change: 1 addition & 0 deletions src/cargo/ops/cargo_output_metadata.rs
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ use cargo_platform::Platform;
use serde::Serialize;
use std::collections::BTreeMap;
use std::path::PathBuf;
use toml_edit::easy as toml;

const VERSION: u32 = 1;

Expand Down
1 change: 1 addition & 0 deletions src/cargo/ops/common_for_install_and_uninstall.rs
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ use std::rc::Rc;

use anyhow::{bail, format_err, Context as _};
use serde::{Deserialize, Serialize};
use toml_edit::easy as toml;

use crate::core::compiler::Freshness;
use crate::core::{Dependency, FeatureValue, Package, PackageId, Source, SourceId};
Expand Down
22 changes: 16 additions & 6 deletions src/cargo/ops/lockfile.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ use crate::util::toml as cargo_toml;
use crate::util::Filesystem;

use anyhow::Context as _;
use toml_edit::easy as toml;

pub fn load_pkg_lockfile(ws: &Workspace<'_>) -> CargoResult<Option<Resolve>> {
if !ws.root().join("Cargo.lock").exists() {
Expand Down Expand Up @@ -100,7 +101,7 @@ fn resolve_to_string_orig(
}

fn serialize_resolve(resolve: &Resolve, orig: Option<&str>) -> String {
let toml = toml::Value::try_from(resolve).unwrap();
let toml = toml_edit::ser::to_item(resolve).unwrap();

let mut out = String::new();

Expand Down Expand Up @@ -139,7 +140,7 @@ fn serialize_resolve(resolve: &Resolve, orig: Option<&str>) -> String {

let deps = toml["package"].as_array().unwrap();
for dep in deps {
let dep = dep.as_table().unwrap();
let dep = dep.as_inline_table().unwrap();

out.push_str("[[package]]\n");
emit_package(dep, &mut out);
Expand All @@ -149,14 +150,23 @@ fn serialize_resolve(resolve: &Resolve, orig: Option<&str>) -> String {
let list = patch["unused"].as_array().unwrap();
for entry in list {
out.push_str("[[patch.unused]]\n");
emit_package(entry.as_table().unwrap(), &mut out);
emit_package(entry.as_inline_table().unwrap(), &mut out);
out.push('\n');
}
}

if let Some(meta) = toml.get("metadata") {
out.push_str("[metadata]\n");
out.push_str(&meta.to_string());
// 1. We need to ensure we print the entire tree, not just the direct members of `metadata`
// (which `toml_edit::Table::to_string` only shows)
// 2. We need to ensure all children tables have `metadata.` prefix
let meta_table = meta
.clone()
.into_table()
.expect("validation ensures this is a table");
let mut meta_doc = toml_edit::Document::new();
meta_doc["metadata"] = toml_edit::Item::Table(meta_table);

out.push_str(&meta_doc.to_string());
}

// Historical versions of Cargo in the old format accidentally left trailing
Expand Down Expand Up @@ -190,7 +200,7 @@ fn are_equal_lockfiles(orig: &str, current: &str, ws: &Workspace<'_>) -> bool {
orig.lines().eq(current.lines())
}

fn emit_package(dep: &toml::value::Table, out: &mut String) {
fn emit_package(dep: &toml_edit::InlineTable, out: &mut String) {
out.push_str(&format!("name = {}\n", &dep["name"]));
out.push_str(&format!("version = {}\n", &dep["version"]));

Expand Down
1 change: 1 addition & 0 deletions src/cargo/ops/vendor.rs
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ use std::collections::{BTreeMap, BTreeSet, HashMap};
use std::fs::{self, File, OpenOptions};
use std::io::{Read, Write};
use std::path::{Path, PathBuf};
use toml_edit::easy as toml;

pub struct VendorOptions<'a> {
pub no_delete: bool,
Expand Down
2 changes: 1 addition & 1 deletion src/cargo/util/config/key.rs
Original file line number Diff line number Diff line change
Expand Up @@ -111,6 +111,6 @@ fn escape_key_part<'a>(part: &'a str) -> Cow<'a, str> {
Cow::Borrowed(part)
} else {
// This is a bit messy, but toml doesn't expose a function to do this.
Cow::Owned(toml::to_string(&toml::Value::String(part.to_string())).unwrap())
Cow::Owned(toml_edit::Value::from(part).to_string())
}
}
1 change: 1 addition & 0 deletions src/cargo/util/config/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -79,6 +79,7 @@ use cargo_util::paths;
use curl::easy::Easy;
use lazycell::LazyCell;
use serde::Deserialize;
use toml_edit::easy as toml;
use url::Url;

mod de;
Expand Down
1 change: 1 addition & 0 deletions src/cargo/util/config/target.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ use crate::util::CargoResult;
use serde::Deserialize;
use std::collections::{BTreeMap, HashMap};
use std::path::PathBuf;
use toml_edit::easy as toml;

/// Config definition of a `[target.'cfg(…)']` table.
///
Expand Down
20 changes: 18 additions & 2 deletions src/cargo/util/toml/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ use semver::{self, VersionReq};
use serde::de;
use serde::ser;
use serde::{Deserialize, Serialize};
use toml_edit::easy as toml;
use url::Url;

use crate::core::compiler::{CompileKind, CompileTarget};
Expand Down Expand Up @@ -77,16 +78,21 @@ pub fn read_manifest_from_str(
let pretty_filename = manifest_file
.strip_prefix(config.cwd())
.unwrap_or(manifest_file);
parse(contents, pretty_filename, config)?
parse_document(contents, pretty_filename, config)?
};

// Provide a helpful error message for a common user error.
if let Some(package) = toml.get("package").or_else(|| toml.get("project")) {
if let Some(feats) = package.get("cargo-features") {
let mut feats = feats.clone();
if let Some(value) = feats.as_value_mut() {
// Only keep formatting inside of the `[]` and not formatting around it
value.decor_mut().clear();
}
bail!(
"cargo-features = {} was found in the wrong location: it \
should be set at the top of Cargo.toml before any tables",
toml::to_string(feats).unwrap()
feats.to_string()
);
}
}
Expand Down Expand Up @@ -164,6 +170,16 @@ pub fn parse(toml: &str, _file: &Path, _config: &Config) -> CargoResult<toml::Va
.map_err(|e| anyhow::Error::from(e).context("could not parse input as TOML"))
}

pub fn parse_document(
toml: &str,
_file: &Path,
_config: &Config,
) -> CargoResult<toml_edit::Document> {
// At the moment, no compatibility checks are needed.
toml.parse()
.map_err(|e| anyhow::Error::from(e).context("could not parse input as TOML"))
}

type TomlLibTarget = TomlTarget;
type TomlBinTarget = TomlTarget;
type TomlExampleTarget = TomlTarget;
Expand Down
36 changes: 33 additions & 3 deletions tests/testsuite/bad_config.rs
Original file line number Diff line number Diff line change
Expand Up @@ -196,7 +196,12 @@ Caused by:
could not parse input as TOML
Caused by:
expected an equals, found eof at line 1 column 2
TOML parse error at line 1, column 2
|
1 | 4
| ^
Unexpected end of input
Expected `.` or `=`
",
)
.run();
Expand Down Expand Up @@ -442,7 +447,13 @@ Caused by:
could not parse input as TOML
Caused by:
expected a table key, found a newline at line 8 column 27
TOML parse error at line 8, column 27
|
8 | native = {
| ^
Unexpected `
`
Expected key
",
)
.run();
Expand Down Expand Up @@ -781,7 +792,26 @@ fn invalid_toml_historically_allowed_fails() {

p.cargo("build")
.with_status(101)
.with_stderr_contains(" expected newline, found an identifier at line 1 column 7")
.with_stderr(
"\
error: could not load Cargo configuration
Caused by:
could not parse TOML configuration in `[..]`
Caused by:
could not parse input as TOML
Caused by:
TOML parse error at line 1, column 7
|
1 | [bar] baz = 2
| ^
Unexpected `b`
Expected newline or end of input
While parsing a Table Header
",
)
.run();
}

Expand Down
21 changes: 18 additions & 3 deletions tests/testsuite/build.rs
Original file line number Diff line number Diff line change
Expand Up @@ -198,7 +198,12 @@ Caused by:
could not parse input as TOML
Caused by:
invalid TOML value, did you mean to use a quoted string? at line 3 column 23
TOML parse error at line 3, column 23
|
3 | foo = bar
| ^
Unexpected `b`
Expected quoted string
",
)
.run();
Expand All @@ -218,7 +223,12 @@ Caused by:
could not parse input as TOML
Caused by:
invalid TOML value, did you mean to use a quoted string? at line 1 column 5
TOML parse error at line 1, column 5
|
1 | a = bar
| ^
Unexpected `b`
Expected quoted string
",
)
.run();
Expand Down Expand Up @@ -2773,7 +2783,12 @@ Caused by:
could not parse input as TOML
Caused by:
expected an equals, found an identifier at line 1 column 6
TOML parse error at line 1, column 6
|
1 | this is not valid toml
| ^
Unexpected `i`
Expected `.` or `=`
",
)
.run();
Expand Down
Loading

0 comments on commit bb96b3a

Please sign in to comment.