Skip to content

Commit

Permalink
Auto merge of #13693 - epage:resolve-toml, r=Muscraft
Browse files Browse the repository at this point in the history
refactor(toml): Split out an explicit step to resolve `Cargo.toml`

### What does this PR try to resolve?

This builds on #13664 and #13666.   Currently, once we have deserialized `Cargo.toml`, we pass it to a large machinery (`to_real_manifest`, `to_virtual_manifest`) so that
- `Cargo.toml` is resolved
- `Summary` is created
- `Manifest` is created

This splits out the resolving of `Cargo.toml` which is mostly workspace inheritance today.

While splitting logic conjoined like this can be a bit messy in the short term, the hope is that overall this makes the logic easier to follow (more condensed, focused sections to view; more explicit inputs/outputs).

In particular, I hope that this will make it clearer and easier to shift more logic into the resolving step, specifically the inferring of build targets for #13456.

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

This is broken up into very small steps in the hope that it makes it easier to analyze a step.

### Additional information
  • Loading branch information
bors committed Apr 3, 2024
2 parents f4ea1d1 + 58b6501 commit 09d5e96
Show file tree
Hide file tree
Showing 4 changed files with 1,905 additions and 1,630 deletions.
114 changes: 114 additions & 0 deletions crates/cargo-util-schemas/src/manifest/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -105,6 +105,16 @@ impl TomlManifest {
pub fn features(&self) -> Option<&BTreeMap<FeatureName, Vec<String>>> {
self.features.as_ref()
}

pub fn resolved_badges(
&self,
) -> Result<Option<&BTreeMap<String, BTreeMap<String, String>>>, UnresolvedError> {
self.badges.as_ref().map(|l| l.resolved()).transpose()
}

pub fn resolved_lints(&self) -> Result<Option<&TomlLints>, UnresolvedError> {
self.lints.as_ref().map(|l| l.resolved()).transpose()
}
}

#[derive(Debug, Deserialize, Serialize, Clone)]
Expand Down Expand Up @@ -194,6 +204,83 @@ pub struct TomlPackage {
pub _invalid_cargo_features: Option<InvalidCargoFeatures>,
}

impl TomlPackage {
pub fn resolved_edition(&self) -> Result<Option<&String>, UnresolvedError> {
self.edition.as_ref().map(|v| v.resolved()).transpose()
}

pub fn resolved_rust_version(&self) -> Result<Option<&RustVersion>, UnresolvedError> {
self.rust_version.as_ref().map(|v| v.resolved()).transpose()
}

pub fn resolved_version(&self) -> Result<Option<&semver::Version>, UnresolvedError> {
self.version.as_ref().map(|v| v.resolved()).transpose()
}

pub fn resolved_authors(&self) -> Result<Option<&Vec<String>>, UnresolvedError> {
self.authors.as_ref().map(|v| v.resolved()).transpose()
}

pub fn resolved_exclude(&self) -> Result<Option<&Vec<String>>, UnresolvedError> {
self.exclude.as_ref().map(|v| v.resolved()).transpose()
}

pub fn resolved_include(&self) -> Result<Option<&Vec<String>>, UnresolvedError> {
self.include.as_ref().map(|v| v.resolved()).transpose()
}

pub fn resolved_publish(&self) -> Result<Option<&VecStringOrBool>, UnresolvedError> {
self.publish.as_ref().map(|v| v.resolved()).transpose()
}

pub fn resolved_description(&self) -> Result<Option<&String>, UnresolvedError> {
self.description.as_ref().map(|v| v.resolved()).transpose()
}

pub fn resolved_homepage(&self) -> Result<Option<&String>, UnresolvedError> {
self.homepage.as_ref().map(|v| v.resolved()).transpose()
}

pub fn resolved_documentation(&self) -> Result<Option<&String>, UnresolvedError> {
self.documentation
.as_ref()
.map(|v| v.resolved())
.transpose()
}

pub fn resolved_readme(&self) -> Result<Option<&String>, UnresolvedError> {
self.readme
.as_ref()
.map(|v| {
v.resolved().and_then(|sb| match sb {
StringOrBool::Bool(_) => Err(UnresolvedError),
StringOrBool::String(value) => Ok(value),
})
})
.transpose()
}

pub fn resolved_keywords(&self) -> Result<Option<&Vec<String>>, UnresolvedError> {
self.keywords.as_ref().map(|v| v.resolved()).transpose()
}

pub fn resolved_categories(&self) -> Result<Option<&Vec<String>>, UnresolvedError> {
self.categories.as_ref().map(|v| v.resolved()).transpose()
}

pub fn resolved_license(&self) -> Result<Option<&String>, UnresolvedError> {
self.license.as_ref().map(|v| v.resolved()).transpose()
}

pub fn resolved_license_file(&self) -> Result<Option<&String>, UnresolvedError> {
self.license_file.as_ref().map(|v| v.resolved()).transpose()
}

pub fn resolved_repository(&self) -> Result<Option<&String>, UnresolvedError> {
self.repository.as_ref().map(|v| v.resolved()).transpose()
}
}

/// An enum that allows for inheriting keys from a workspace in a Cargo.toml.
#[derive(Serialize, Copy, Clone, Debug)]
#[serde(untagged)]
Expand All @@ -205,6 +292,10 @@ pub enum InheritableField<T> {
}

impl<T> InheritableField<T> {
pub fn resolved(&self) -> Result<&T, UnresolvedError> {
self.as_value().ok_or(UnresolvedError)
}

pub fn as_value(&self) -> Option<&T> {
match self {
InheritableField::Inherit(_) => None,
Expand Down Expand Up @@ -504,6 +595,13 @@ impl InheritableDependency {
InheritableDependency::Inherit(w) => w._unused_keys.keys().cloned().collect(),
}
}

pub fn resolved(&self) -> Result<&TomlDependency, UnresolvedError> {
match self {
InheritableDependency::Value(d) => Ok(d),
InheritableDependency::Inherit(_) => Err(UnresolvedError),
}
}
}

impl<'de> de::Deserialize<'de> for InheritableDependency {
Expand Down Expand Up @@ -1297,6 +1395,16 @@ pub struct InheritableLints {
pub lints: TomlLints,
}

impl InheritableLints {
pub fn resolved(&self) -> Result<&TomlLints, UnresolvedError> {
if self.workspace {
Err(UnresolvedError)
} else {
Ok(&self.lints)
}
}
}

fn is_false(b: &bool) -> bool {
!b
}
Expand Down Expand Up @@ -1512,3 +1620,9 @@ impl<'de> de::Deserialize<'de> for PathValue {
Ok(PathValue(String::deserialize(deserializer)?.into()))
}
}

/// Error validating names in Cargo.
#[derive(Debug, thiserror::Error)]
#[error("manifest field was not resolved")]
#[non_exhaustive]
pub struct UnresolvedError;
4 changes: 2 additions & 2 deletions src/cargo/core/features.rs
Original file line number Diff line number Diff line change
Expand Up @@ -151,7 +151,7 @@ pub type AllowFeatures = BTreeSet<String>;
/// - Update [`CLI_VALUES`] to include the new edition.
/// - Set [`LATEST_UNSTABLE`] to Some with the new edition.
/// - Add an unstable feature to the [`features!`] macro invocation below for the new edition.
/// - Gate on that new feature in [`toml::to_real_manifest`].
/// - Gate on that new feature in [`toml`].
/// - Update the shell completion files.
/// - Update any failing tests (hopefully there are very few).
/// - Update unstable.md to add a new section for this new edition (see [this example]).
Expand All @@ -178,7 +178,7 @@ pub type AllowFeatures = BTreeSet<String>;
/// [`LATEST_STABLE`]: Edition::LATEST_STABLE
/// [this example]: https://github.com/rust-lang/cargo/blob/3ebb5f15a940810f250b68821149387af583a79e/src/doc/src/reference/unstable.md?plain=1#L1238-L1264
/// [`is_stable`]: Edition::is_stable
/// [`toml::to_real_manifest`]: crate::util::toml::to_real_manifest
/// [`toml`]: crate::util::toml
/// [`features!`]: macro.features.html
#[derive(
Default, Clone, Copy, Debug, Hash, PartialOrd, Ord, Eq, PartialEq, Serialize, Deserialize,
Expand Down
1 change: 0 additions & 1 deletion src/cargo/core/workspace.rs
Original file line number Diff line number Diff line change
Expand Up @@ -449,7 +449,6 @@ impl<'gctx> Workspace<'gctx> {
// NOTE: Since we use ConfigRelativePath, this root isn't used as
// any relative paths are resolved before they'd be joined with root.
Path::new("unused-relative-path"),
self.unstable_features(),
/* kind */ None,
)
})
Expand Down
Loading

0 comments on commit 09d5e96

Please sign in to comment.