Skip to content

Commit

Permalink
Auto merge of #12971 - epage:schema, r=Muscraft
Browse files Browse the repository at this point in the history
refactor(toml): Clean up workspace inheritance

### What does this PR try to resolve?

The goal is to simplify the code so we have a better boundary between `toml/mod.rs` and `toml/schema.rs` for when we break `toml/schema.rs` into a separate package for #12801.

This let us
- Remove a trait used in some back and forth for error handling
- Move a lot of the inheritable bookkeeping and logic out of `schema.rs`

A lot of these changes were inspired by [`cargo_toml`](https://docs.rs/cargo_toml/latest/cargo_toml/).  This included some renaming that I felt made the code clearer.

I didn't go as far as `cargo_toml`, yet.
- They derive more `Deserializers`, producing worse errors
- Their equivalent of `InheritableField` has an `inherit` function on it.  They eagerly inherit things and then error if anything isn't inherited
  - I'm still toying with something like this but held off for now
  - One idea is `InheritableField` has an `inherit_with` function, like today, that only passes errors up but doesn't generate an error.  We then have a `get` function that errors if it isn't inherited.  We could encode the field names, for error reporting, into a type parameter for `InheritableField`
- They flatten `InheritableDependency` into `TomlDependency`
  - By splitting these up, `workspace.dependencies` and `.cargo/config.toml` code can directly use `TomlDependency` without extra error handling
  -If we went this rout, I think I'd merge`InheritableDependency::Inherit` with `DetailedDependency`, having callers handle the errors (much like `TomlManifest` is both a real and virtual)

Some things I'm trying to balance
- Complexity
- Quality of error messages
- Knowing what code needs touching when changes are made
  - Some more work is needed here

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

### Additional information
  • Loading branch information
bors committed Nov 17, 2023
2 parents f1da36f + 46d9894 commit 9765a44
Show file tree
Hide file tree
Showing 4 changed files with 243 additions and 256 deletions.
1 change: 0 additions & 1 deletion src/cargo/core/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,6 @@ pub use self::workspace::{
find_workspace_root, resolve_relative_path, MaybePackage, Workspace, WorkspaceConfig,
WorkspaceRootConfig,
};
pub use crate::util::toml::schema::InheritableFields;

pub mod compiler;
pub mod dependency;
Expand Down
2 changes: 1 addition & 1 deletion src/cargo/core/workspace.rs
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ use crate::util::edit_distance;
use crate::util::errors::{CargoResult, ManifestError};
use crate::util::interning::InternedString;
use crate::util::toml::{
read_manifest, schema::InheritableFields, schema::TomlDependency, schema::TomlProfiles,
read_manifest, schema::TomlDependency, schema::TomlProfiles, InheritableFields,
};
use crate::util::RustVersion;
use crate::util::{config::ConfigRelativePath, Config, Filesystem, IntoUrl};
Expand Down
Loading

0 comments on commit 9765a44

Please sign in to comment.