Skip to content

Commit

Permalink
Auto merge of #12948 - epage:toml-refactor, r=weihanglo
Browse files Browse the repository at this point in the history
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
  • Loading branch information
bors committed Nov 9, 2023
2 parents 9b2cad6 + eba0091 commit ac9d3ba
Show file tree
Hide file tree
Showing 8 changed files with 274 additions and 272 deletions.
13 changes: 5 additions & 8 deletions src/cargo/ops/cargo_package.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,6 @@ use std::fs::{self, File};
use std::io::prelude::*;
use std::io::SeekFrom;
use std::path::{Path, PathBuf};
use std::rc::Rc;
use std::sync::Arc;
use std::task::Poll;

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

// Convert Package -> TomlManifest -> Manifest -> Package
let toml_manifest = Rc::new(
orig_pkg
.manifest()
.original()
.prepare_for_publish(ws, orig_pkg.root())?,
);
let toml_manifest = orig_pkg
.manifest()
.original()
.prepare_for_publish(ws, orig_pkg.root())?;
let package_root = orig_pkg.root();
let source_id = orig_pkg.package_id().source_id();
let (manifest, _nested_paths) =
TomlManifest::to_real_manifest(&toml_manifest, false, source_id, package_root, config)?;
TomlManifest::to_real_manifest(toml_manifest, false, source_id, package_root, config)?;
let new_pkg = Package::new(manifest, orig_pkg.manifest_path());

let max_rust_version = new_pkg.rust_version().cloned();
Expand Down
13 changes: 6 additions & 7 deletions src/cargo/util/command_prelude.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,9 +6,8 @@ use crate::ops::{CompileFilter, CompileOptions, NewOptions, Packages, VersionCon
use crate::util::important_paths::find_root_manifest_for_wd;
use crate::util::interning::InternedString;
use crate::util::is_rustup;
use crate::util::restricted_names::is_glob_pattern;
use crate::util::toml::schema::{StringOrVec, TomlProfile};
use crate::util::validate_package_name;
use crate::util::restricted_names;
use crate::util::toml::schema::StringOrVec;
use crate::util::{
print_available_benches, print_available_binaries, print_available_examples,
print_available_packages, print_available_tests,
Expand Down Expand Up @@ -607,7 +606,7 @@ Run `{cmd}` to see possible targets."
bail!("profile `doc` is reserved and not allowed to be explicitly specified")
}
(_, _, Some(name)) => {
TomlProfile::validate_name(name)?;
restricted_names::validate_profile_name(name)?;
name
}
};
Expand Down Expand Up @@ -801,7 +800,7 @@ Run `{cmd}` to see possible targets."
) -> CargoResult<CompileOptions> {
let mut compile_opts = self.compile_options(config, mode, workspace, profile_checking)?;
let spec = self._values_of("package");
if spec.iter().any(is_glob_pattern) {
if spec.iter().any(restricted_names::is_glob_pattern) {
anyhow::bail!("Glob patterns on package selection are not supported.")
}
compile_opts.spec = Packages::Packages(spec);
Expand Down Expand Up @@ -835,7 +834,7 @@ Run `{cmd}` to see possible targets."
(None, None) => config.default_registry()?.map(RegistryOrIndex::Registry),
(None, Some(i)) => Some(RegistryOrIndex::Index(i.into_url()?)),
(Some(r), None) => {
validate_package_name(r, "registry name", "")?;
restricted_names::validate_package_name(r, "registry name", "")?;
Some(RegistryOrIndex::Registry(r.to_string()))
}
(Some(_), Some(_)) => {
Expand All @@ -850,7 +849,7 @@ Run `{cmd}` to see possible targets."
match self._value_of("registry").map(|s| s.to_string()) {
None => config.default_registry(),
Some(registry) => {
validate_package_name(&registry, "registry name", "")?;
restricted_names::validate_package_name(&registry, "registry name", "")?;
Ok(Some(registry))
}
}
Expand Down
10 changes: 7 additions & 3 deletions src/cargo/util/config/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -76,7 +76,6 @@ use crate::sources::CRATES_IO_REGISTRY;
use crate::util::errors::CargoResult;
use crate::util::network::http::configure_http_handle;
use crate::util::network::http::http_handle;
use crate::util::toml as cargo_toml;
use crate::util::{internal, CanonicalUrl};
use crate::util::{try_canonicalize, validate_package_name};
use crate::util::{Filesystem, IntoUrl, IntoUrlWithBase, Rustc};
Expand Down Expand Up @@ -1198,7 +1197,7 @@ impl Config {
}
let contents = fs::read_to_string(path)
.with_context(|| format!("failed to read configuration file `{}`", path.display()))?;
let toml = cargo_toml::parse_document(&contents, path, self).with_context(|| {
let toml = parse_document(&contents, path, self).with_context(|| {
format!("could not parse TOML configuration in `{}`", path.display())
})?;
let def = match why_load {
Expand Down Expand Up @@ -2249,7 +2248,7 @@ pub fn save_credentials(
)
})?;

let mut toml = cargo_toml::parse_document(&contents, file.path(), cfg)?;
let mut toml = parse_document(&contents, file.path(), cfg)?;

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

pub type EnvConfig = HashMap<String, EnvConfigValue>;

fn parse_document(toml: &str, _file: &Path, _config: &Config) -> CargoResult<toml::Table> {
// At the moment, no compatibility checks are needed.
toml.parse().map_err(Into::into)
}

/// A type to deserialize a list of strings from a toml file.
///
/// Supports deserializing either a whitespace-separated list of arguments in a
Expand Down
79 changes: 79 additions & 0 deletions src/cargo/util/restricted_names.rs
Original file line number Diff line number Diff line change
Expand Up @@ -120,3 +120,82 @@ pub fn is_windows_reserved_path(path: &Path) -> bool {
pub fn is_glob_pattern<T: AsRef<str>>(name: T) -> bool {
name.as_ref().contains(&['*', '?', '[', ']'][..])
}

/// Validate dir-names and profile names according to RFC 2678.
pub fn validate_profile_name(name: &str) -> CargoResult<()> {
if let Some(ch) = name
.chars()
.find(|ch| !ch.is_alphanumeric() && *ch != '_' && *ch != '-')
{
bail!(
"invalid character `{}` in profile name `{}`\n\
Allowed characters are letters, numbers, underscore, and hyphen.",
ch,
name
);
}

const SEE_DOCS: &str = "See https://doc.rust-lang.org/cargo/reference/profiles.html \
for more on configuring profiles.";

let lower_name = name.to_lowercase();
if lower_name == "debug" {
bail!(
"profile name `{}` is reserved\n\
To configure the default development profile, use the name `dev` \
as in [profile.dev]\n\
{}",
name,
SEE_DOCS
);
}
if lower_name == "build-override" {
bail!(
"profile name `{}` is reserved\n\
To configure build dependency settings, use [profile.dev.build-override] \
and [profile.release.build-override]\n\
{}",
name,
SEE_DOCS
);
}

// These are some arbitrary reservations. We have no plans to use
// these, but it seems safer to reserve a few just in case we want to
// add more built-in profiles in the future. We can also uses special
// syntax like cargo:foo if needed. But it is unlikely these will ever
// be used.
if matches!(
lower_name.as_str(),
"build"
| "check"
| "clean"
| "config"
| "fetch"
| "fix"
| "install"
| "metadata"
| "package"
| "publish"
| "report"
| "root"
| "run"
| "rust"
| "rustc"
| "rustdoc"
| "target"
| "tmp"
| "uninstall"
) || lower_name.starts_with("cargo")
{
bail!(
"profile name `{}` is reserved\n\
Please choose a different name.\n\
{}",
name,
SEE_DOCS
);
}

Ok(())
}
4 changes: 2 additions & 2 deletions src/cargo/util/toml/embedded.rs
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ const DEFAULT_EDITION: crate::core::features::Edition =
crate::core::features::Edition::LATEST_STABLE;
const AUTO_FIELDS: &[&str] = &["autobins", "autoexamples", "autotests", "autobenches"];

pub fn expand_manifest(
pub(super) fn expand_manifest(
content: &str,
path: &std::path::Path,
config: &Config,
Expand Down Expand Up @@ -329,7 +329,7 @@ impl DocFragment {
}

#[derive(Clone, Copy, PartialEq, Debug)]
pub enum CommentKind {
enum CommentKind {
Line,
Block,
}
Expand Down
Loading

0 comments on commit ac9d3ba

Please sign in to comment.