feat: in-memory only `Manifest` #16409

secona · 2025-12-17T06:44:10Z

What does this PR try to resolve?

This PR supports creating an in-memory only Manifest.

Part of #16290

How to test and review this PR?

No tests should change as this is for cargo-as-a-library.

src/cargo/core/manifest.rs

secona · 2025-12-17T06:47:50Z

src/cargo/lints/rules/unknown_lints.rs

    }

-    let document = manifest.document();
+    let document = manifest.document().unwrap();


Should lints assume that this is a Some? I'm not familiar on how lints are used in cargo-as-a-library. Other options would probably be bail!ing out or silently stopping.

We will need a way to run lint passes with plumbing commands, so we'll need to have a way to resolve this

We will need a way to run lint passes with plumbing commands

By this, do you mean lints being run as part of an existing plumbing command, like read-manifest, or having a separate plumbing command to run lints?

Unsure where we will run it atm

Right. I think we can safely assume that the fields are Some. Within cargo itself, the calls to constructors use Some.

No, we should be handling the Nones

epage · 2025-12-17T14:06:15Z

src/cargo/core/manifest.rs

        contents: Rc<String>,
-        document: Rc<toml::Spanned<toml::de::DeTable<'static>>>,
-        original_toml: Rc<TomlManifest>,
+        document: Option<Rc<toml::Spanned<toml::de::DeTable<'static>>>>,


I feel like removing the .unwrap()s would need to happen before this

rustbot · 2025-12-17T17:07:01Z

r? @epage

rustbot has assigned @epage.
They will have a look at your PR within the next two weeks and either review your PR or reassign to another reviewer.

Use r? to explicitly pick a reviewer

epage · 2025-12-17T19:13:48Z

src/cargo/ops/vendor.rs

 ) -> CargoResult<Package> {
-    let contents = me.manifest().contents();
-    let document = me.manifest().document();
+    let contents = me.manifest().contents().unwrap();


My default assumption is that this PR is not ready to merge until all .unwrap()s are resolved. Any remaining .unwrap() should have a justification, likely including switching it to an .expect() explaining what invariants are keeping it from panicking.

epage · 2025-12-17T19:14:10Z

Because of #16409 (comment), I'm moving this back to a Draft

secona · 2025-12-19T00:07:20Z

src/cargo/core/compiler/mod.rs

+            spans: unit.pkg.manifest().document().unwrap().clone(),
+            contents: unit.pkg.manifest().contents().unwrap().to_owned(),


Looks like these are for public dependency diagnostics. Its either requiring compilation to have a complete Workspace or disabling diagnostics for public dependency.

This is no different than any other diagnostics: we should work to allow contents to be optional.

epage · 2025-12-19T15:51:40Z

src/cargo/core/manifest.rs

-    pub fn document(&self) -> &toml::Spanned<toml::de::DeTable<'static>> {
-        self.document.as_ref().unwrap()
+    pub fn document(&self) -> Option<&toml::Spanned<toml::de::DeTable<'static>>> {
+        self.document.as_ref().map(|d| d.as_ref())


Why are these maps needed?

epage · 2025-12-19T15:52:38Z

src/cargo/lints/mod.rs

-    let contents = manifest.contents();
+    let contents = manifest
+        .contents()
+        .with_context(|| "incomplete Workspace information")?;


Note that these with_context in this commit are not sufficient either. The error message is not meaningful to the end user and most or all of the functionality that they guard should be runnable

Right. So functionality should still be present but without the snippets?

Wherever possible, yes

Make `document`, `original_toml` and `contents` optional to allow creating Manifests from in-memory data.

Make `contents` and `spans` optional in ManifestErrorContext to allow disabling diagnostics.

Remove calls to `.unwrap` to make lints work with in-memory `Manifest`.

Change the signature for `to_real_manifest`, `to_virtual_manifest` and `missing_dep_diagnostic` to support in-memory `Manifest`.

rustbot added A-build-execution Area: anything dealing with executing the compiler A-manifest Area: Cargo.toml issues A-workspaces Area: workspaces Command-fix Command-vendor labels Dec 17, 2025

secona commented Dec 17, 2025

View reviewed changes

src/cargo/core/manifest.rs Outdated Show resolved Hide resolved

secona commented Dec 17, 2025

View reviewed changes

secona mentioned this pull request Dec 17, 2025

Add a way to construct a Workspace without reading manifests from disk #16290

Open

epage reviewed Dec 17, 2025

View reviewed changes

secona force-pushed the in-memory-manifest branch from f857ec0 to 8d25903 Compare December 17, 2025 16:21

secona marked this pull request as ready for review December 17, 2025 17:06

rustbot added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Dec 17, 2025

rustbot assigned epage Dec 17, 2025

secona force-pushed the in-memory-manifest branch from 8d25903 to c9fa22d Compare December 17, 2025 17:08

epage reviewed Dec 17, 2025

View reviewed changes

epage marked this pull request as draft December 17, 2025 19:13

rustbot removed the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Dec 17, 2025

secona force-pushed the in-memory-manifest branch from c9fa22d to fae3b40 Compare December 18, 2025 23:21

secona commented Dec 19, 2025

View reviewed changes

epage reviewed Dec 19, 2025

View reviewed changes

secona force-pushed the in-memory-manifest branch from fae3b40 to 1dfa6b3 Compare December 19, 2025 20:52

secona added 2 commits December 20, 2025 04:18

refactor: optional fields in Manifest

ffa6d53

Make `document`, `original_toml` and `contents` optional to allow creating Manifests from in-memory data.

refactor: optional fields in ManifestErrorContext

b0ec233

Make `contents` and `spans` optional in ManifestErrorContext to allow disabling diagnostics.

secona force-pushed the in-memory-manifest branch from 1dfa6b3 to 4573605 Compare December 19, 2025 21:25

secona added 2 commits December 20, 2025 10:18

refactor: introduce Option in getters

9124ff9

refactor: introduce Option in constructors

fc138a6

secona force-pushed the in-memory-manifest branch from 4573605 to 9c99c39 Compare December 20, 2025 03:18

secona added 3 commits December 20, 2025 19:45

refactor(lints): properly handle in-memory Manifest

87ae739

Remove calls to `.unwrap` to make lints work with in-memory `Manifest`.

refactor(toml): prepare for in-memory only Manifest

fe61fe3

Change the signature for `to_real_manifest`, `to_virtual_manifest` and `missing_dep_diagnostic` to support in-memory `Manifest`.

refactor(vendor): properly handle in-memory Manifest

3f8a05f

secona force-pushed the in-memory-manifest branch from 9c99c39 to 3f8a05f Compare December 20, 2025 12:59

		spans: unit.pkg.manifest().document().unwrap().clone(),
		contents: unit.pkg.manifest().contents().unwrap().to_owned(),

feat: in-memory only Manifest #16409

Are you sure you want to change the base?

feat: in-memory only Manifest #16409

Conversation

secona commented Dec 17, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

What does this PR try to resolve?

How to test and review this PR?

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

rustbot commented Dec 17, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

epage commented Dec 17, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

feat: in-memory only `Manifest` #16409

feat: in-memory only `Manifest` #16409

secona commented Dec 17, 2025 •

edited

Loading