Skip to content

Commit ac9d3ba

Browse files
committed
Auto merge of #12948 - epage:toml-refactor, r=weihanglo
refactor(toml): Simplify code to make schema split easier ### What does this PR try to resolve? This is a follow up to #12911 and is prep for #12801. ### How should we test and review this PR? ### Additional information
2 parents 9b2cad6 + eba0091 commit ac9d3ba

File tree

8 files changed

+274
-272
lines changed

8 files changed

+274
-272
lines changed

src/cargo/ops/cargo_package.rs

Lines changed: 5 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,6 @@ use std::fs::{self, File};
33
use std::io::prelude::*;
44
use std::io::SeekFrom;
55
use std::path::{Path, PathBuf};
6-
use std::rc::Rc;
76
use std::sync::Arc;
87
use std::task::Poll;
98

@@ -412,16 +411,14 @@ fn build_lock(ws: &Workspace<'_>, orig_pkg: &Package) -> CargoResult<String> {
412411
let orig_resolve = ops::load_pkg_lockfile(ws)?;
413412

414413
// Convert Package -> TomlManifest -> Manifest -> Package
415-
let toml_manifest = Rc::new(
416-
orig_pkg
417-
.manifest()
418-
.original()
419-
.prepare_for_publish(ws, orig_pkg.root())?,
420-
);
414+
let toml_manifest = orig_pkg
415+
.manifest()
416+
.original()
417+
.prepare_for_publish(ws, orig_pkg.root())?;
421418
let package_root = orig_pkg.root();
422419
let source_id = orig_pkg.package_id().source_id();
423420
let (manifest, _nested_paths) =
424-
TomlManifest::to_real_manifest(&toml_manifest, false, source_id, package_root, config)?;
421+
TomlManifest::to_real_manifest(toml_manifest, false, source_id, package_root, config)?;
425422
let new_pkg = Package::new(manifest, orig_pkg.manifest_path());
426423

427424
let max_rust_version = new_pkg.rust_version().cloned();

src/cargo/util/command_prelude.rs

Lines changed: 6 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -6,9 +6,8 @@ use crate::ops::{CompileFilter, CompileOptions, NewOptions, Packages, VersionCon
66
use crate::util::important_paths::find_root_manifest_for_wd;
77
use crate::util::interning::InternedString;
88
use crate::util::is_rustup;
9-
use crate::util::restricted_names::is_glob_pattern;
10-
use crate::util::toml::schema::{StringOrVec, TomlProfile};
11-
use crate::util::validate_package_name;
9+
use crate::util::restricted_names;
10+
use crate::util::toml::schema::StringOrVec;
1211
use crate::util::{
1312
print_available_benches, print_available_binaries, print_available_examples,
1413
print_available_packages, print_available_tests,
@@ -607,7 +606,7 @@ Run `{cmd}` to see possible targets."
607606
bail!("profile `doc` is reserved and not allowed to be explicitly specified")
608607
}
609608
(_, _, Some(name)) => {
610-
TomlProfile::validate_name(name)?;
609+
restricted_names::validate_profile_name(name)?;
611610
name
612611
}
613612
};
@@ -801,7 +800,7 @@ Run `{cmd}` to see possible targets."
801800
) -> CargoResult<CompileOptions> {
802801
let mut compile_opts = self.compile_options(config, mode, workspace, profile_checking)?;
803802
let spec = self._values_of("package");
804-
if spec.iter().any(is_glob_pattern) {
803+
if spec.iter().any(restricted_names::is_glob_pattern) {
805804
anyhow::bail!("Glob patterns on package selection are not supported.")
806805
}
807806
compile_opts.spec = Packages::Packages(spec);
@@ -835,7 +834,7 @@ Run `{cmd}` to see possible targets."
835834
(None, None) => config.default_registry()?.map(RegistryOrIndex::Registry),
836835
(None, Some(i)) => Some(RegistryOrIndex::Index(i.into_url()?)),
837836
(Some(r), None) => {
838-
validate_package_name(r, "registry name", "")?;
837+
restricted_names::validate_package_name(r, "registry name", "")?;
839838
Some(RegistryOrIndex::Registry(r.to_string()))
840839
}
841840
(Some(_), Some(_)) => {
@@ -850,7 +849,7 @@ Run `{cmd}` to see possible targets."
850849
match self._value_of("registry").map(|s| s.to_string()) {
851850
None => config.default_registry(),
852851
Some(registry) => {
853-
validate_package_name(&registry, "registry name", "")?;
852+
restricted_names::validate_package_name(&registry, "registry name", "")?;
854853
Ok(Some(registry))
855854
}
856855
}

src/cargo/util/config/mod.rs

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -76,7 +76,6 @@ use crate::sources::CRATES_IO_REGISTRY;
7676
use crate::util::errors::CargoResult;
7777
use crate::util::network::http::configure_http_handle;
7878
use crate::util::network::http::http_handle;
79-
use crate::util::toml as cargo_toml;
8079
use crate::util::{internal, CanonicalUrl};
8180
use crate::util::{try_canonicalize, validate_package_name};
8281
use crate::util::{Filesystem, IntoUrl, IntoUrlWithBase, Rustc};
@@ -1198,7 +1197,7 @@ impl Config {
11981197
}
11991198
let contents = fs::read_to_string(path)
12001199
.with_context(|| format!("failed to read configuration file `{}`", path.display()))?;
1201-
let toml = cargo_toml::parse_document(&contents, path, self).with_context(|| {
1200+
let toml = parse_document(&contents, path, self).with_context(|| {
12021201
format!("could not parse TOML configuration in `{}`", path.display())
12031202
})?;
12041203
let def = match why_load {
@@ -2249,7 +2248,7 @@ pub fn save_credentials(
22492248
)
22502249
})?;
22512250

2252-
let mut toml = cargo_toml::parse_document(&contents, file.path(), cfg)?;
2251+
let mut toml = parse_document(&contents, file.path(), cfg)?;
22532252

22542253
// Move the old token location to the new one.
22552254
if let Some(token) = toml.remove("token") {
@@ -2716,6 +2715,11 @@ impl EnvConfigValue {
27162715

27172716
pub type EnvConfig = HashMap<String, EnvConfigValue>;
27182717

2718+
fn parse_document(toml: &str, _file: &Path, _config: &Config) -> CargoResult<toml::Table> {
2719+
// At the moment, no compatibility checks are needed.
2720+
toml.parse().map_err(Into::into)
2721+
}
2722+
27192723
/// A type to deserialize a list of strings from a toml file.
27202724
///
27212725
/// Supports deserializing either a whitespace-separated list of arguments in a

src/cargo/util/restricted_names.rs

Lines changed: 79 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -120,3 +120,82 @@ pub fn is_windows_reserved_path(path: &Path) -> bool {
120120
pub fn is_glob_pattern<T: AsRef<str>>(name: T) -> bool {
121121
name.as_ref().contains(&['*', '?', '[', ']'][..])
122122
}
123+
124+
/// Validate dir-names and profile names according to RFC 2678.
125+
pub fn validate_profile_name(name: &str) -> CargoResult<()> {
126+
if let Some(ch) = name
127+
.chars()
128+
.find(|ch| !ch.is_alphanumeric() && *ch != '_' && *ch != '-')
129+
{
130+
bail!(
131+
"invalid character `{}` in profile name `{}`\n\
132+
Allowed characters are letters, numbers, underscore, and hyphen.",
133+
ch,
134+
name
135+
);
136+
}
137+
138+
const SEE_DOCS: &str = "See https://doc.rust-lang.org/cargo/reference/profiles.html \
139+
for more on configuring profiles.";
140+
141+
let lower_name = name.to_lowercase();
142+
if lower_name == "debug" {
143+
bail!(
144+
"profile name `{}` is reserved\n\
145+
To configure the default development profile, use the name `dev` \
146+
as in [profile.dev]\n\
147+
{}",
148+
name,
149+
SEE_DOCS
150+
);
151+
}
152+
if lower_name == "build-override" {
153+
bail!(
154+
"profile name `{}` is reserved\n\
155+
To configure build dependency settings, use [profile.dev.build-override] \
156+
and [profile.release.build-override]\n\
157+
{}",
158+
name,
159+
SEE_DOCS
160+
);
161+
}
162+
163+
// These are some arbitrary reservations. We have no plans to use
164+
// these, but it seems safer to reserve a few just in case we want to
165+
// add more built-in profiles in the future. We can also uses special
166+
// syntax like cargo:foo if needed. But it is unlikely these will ever
167+
// be used.
168+
if matches!(
169+
lower_name.as_str(),
170+
"build"
171+
| "check"
172+
| "clean"
173+
| "config"
174+
| "fetch"
175+
| "fix"
176+
| "install"
177+
| "metadata"
178+
| "package"
179+
| "publish"
180+
| "report"
181+
| "root"
182+
| "run"
183+
| "rust"
184+
| "rustc"
185+
| "rustdoc"
186+
| "target"
187+
| "tmp"
188+
| "uninstall"
189+
) || lower_name.starts_with("cargo")
190+
{
191+
bail!(
192+
"profile name `{}` is reserved\n\
193+
Please choose a different name.\n\
194+
{}",
195+
name,
196+
SEE_DOCS
197+
);
198+
}
199+
200+
Ok(())
201+
}

src/cargo/util/toml/embedded.rs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@ const DEFAULT_EDITION: crate::core::features::Edition =
88
crate::core::features::Edition::LATEST_STABLE;
99
const AUTO_FIELDS: &[&str] = &["autobins", "autoexamples", "autotests", "autobenches"];
1010

11-
pub fn expand_manifest(
11+
pub(super) fn expand_manifest(
1212
content: &str,
1313
path: &std::path::Path,
1414
config: &Config,
@@ -329,7 +329,7 @@ impl DocFragment {
329329
}
330330

331331
#[derive(Clone, Copy, PartialEq, Debug)]
332-
pub enum CommentKind {
332+
enum CommentKind {
333333
Line,
334334
Block,
335335
}

0 commit comments

Comments
 (0)