Skip to content

Allow setting -Zembed-metadata value from the config - #17266

Merged
weihanglo merged 2 commits into
rust-lang:masterfrom
Kobzol:embed-metadata-config
Jul 27, 2026
Merged

Allow setting -Zembed-metadata value from the config#17266
weihanglo merged 2 commits into
rust-lang:masterfrom
Kobzol:embed-metadata-config

Conversation

@Kobzol

@Kobzol Kobzol commented Jul 27, 2026

Copy link
Copy Markdown
Member

Using a boolean value.

Noted here.

Why not just use bool? The third variant is needed for enabling the flag by default on nightly, to recognize if the flag was set by the user or not.

r? @weihanglo

@rustbot rustbot added A-unstable Area: nightly unstable support S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Jul 27, 2026
Comment thread src/workspace/features.rs
Comment thread src/workspace/features.rs Outdated
Ok(Some(v))
}

fn deserialize_embed_metadata<'de, D>(deserializer: D) -> Result<EmbedMetadata, D::Error>

@weihanglo weihanglo Jul 27, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could we have one simple test verifying it works?

View changes since the review

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Probably in tests/testsuite/config.rs

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I tried that, the problem is that the config tests use .get, and that expects the returned type to implement Deserialize (which this type now doesn't) 🤔

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

😞
Probably we could just use serde(rename) oh those two yes|no variants and serde(skip) unset?

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That is different than this PR's bool config though.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If we are going to have third option, "yes"|"no" string value may be better than bool

@Kobzol Kobzol Jul 27, 2026

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The third option is really just equivalent to None (i.e. the user didn't set any value).

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The what about this?

     pub fn should_embed_metadata(&self) -> bool {
-        match self.cli_unstable().embed_metadata {
-            EmbedMetadata::Embed => true,
-            EmbedMetadata::DoNotEmbed => false,
-            EmbedMetadata::Unset => true,
-        }
+        self.cli_unstable().embed_metadata.unwrap_or(true)
     }
// ...
 unstable_cli_options!(
     // Permanently unstable features:
     allow_features: Option<AllowFeatures> = ("Allow *only* the listed unstable features"),
-    embed_metadata: EmbedMetadata = ("Avoid embedding metadata in library artifacts"),
+    embed_metadata: Option<bool> = ("Avoid embedding metadata in library artifacts"),
     print_im_a_teapot: bool,

It should work under this requirement right?

The third variant is needed for enabling the flag by default on nightly, to recognize if the flag was set by the user or not.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, Option<bool> also works, though I found the enum nicer, because I could document the individual variants :) But I'm also fine with Option<bool>.

@rustbot rustbot added the A-configuration Area: cargo config files and env vars label Jul 27, 2026
@Kobzol

Kobzol commented Jul 27, 2026

Copy link
Copy Markdown
Member Author

Changed to Option<bool> and added a simple test.

@weihanglo weihanglo left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@weihanglo
weihanglo added this pull request to the merge queue Jul 27, 2026
Merged via the queue into rust-lang:master with commit 0158e40 Jul 27, 2026
29 checks passed
@rustbot rustbot removed the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Jul 27, 2026
@Kobzol
Kobzol deleted the embed-metadata-config branch July 27, 2026 22:14
rust-bors Bot pushed a commit to rust-lang/rust that referenced this pull request Jul 29, 2026
Update cargo submodule




## src/tools/cargo

23 commits in 3efb1f477e99b42974b982d939fd100303cdf7db..7c83d4cc0953b81d823e47d640c64da9b8bd4fac
2026-07-17 23:53:19 +0000 to 2026-07-29 21:34:53 +0000
- fix: Pass rustdoc flags to final CCI merge step (rust-lang/cargo#17269)
- Reworked how we enable the new build-dir layout on nightly (rust-lang/cargo#17272)
- Allow setting `-Zembed-metadata` value from the config (rust-lang/cargo#17266)
- fix(cli): don't panic during completions when rustup is unavailable (rust-lang/cargo#17263)
- docs(workspace): add recommended structure to members field (rust-lang/cargo#17166)
- Update cargo-fetch.md to remove cargo-prefetch reference (rust-lang/cargo#16568)
- chore: bump to `libgit2-sys@0.18.7+1.9.6` (rust-lang/cargo#17259)
- Enable build-dir layout v2 on nightly by default (rust-lang/cargo#17258)
- fix(path): clarify error message when path dependency has wrong package (rust-lang/cargo#16927)
- fix(toml): warn on hyphenated lint names and duplicates (rust-lang/cargo#17051)
- fix(test): gate trim-paths tests on split debuginfo support (rust-lang/cargo#17256)
- test(git): Explicitly test for git injection attacks (rust-lang/cargo#17253)
- fix(git): Suggest libgit2 if git-cli fails (rust-lang/cargo#17252)
- fix(diag): bound transitive unused dependency traversal (rust-lang/cargo#17251)
- fix(git): Hide git fetch output without progress  (rust-lang/cargo#17243)
- revert(lint): Remove `new_implicit_minimum_version_req` (rust-lang/cargo#16321) (rust-lang/cargo#17249)
- fix: Add haiku's dylib path (rust-lang/cargo#17248)
- Zsh completion: Add `-p` and `--package` flags for `cargo add` (rust-lang/cargo#17247)
- refactor(source): Clarify the name of the remote git registry (rust-lang/cargo#17240)
- fix(timings): only report units the job queue actually ran (rust-lang/cargo#17238)
- Do not include proc-macro deps in rustc search path args (rust-lang/cargo#17236)
- chore(deps): update cargo-semver-checks to v0.49.0 (rust-lang/cargo#17237)
- rustdoc: rename the doc parts metadata params (rust-lang/cargo#17234)

## src/tools/rustc-perf

12 commits in 0508bdcd37152b28c39b6752828683cdd3f128b5..74ecbcdf88411937a6e39baf2779948565dfd388
2026-07-15 10:20:27 +0000 to 2026-07-27 15:02:48 +0000
- feat: support `@argfile` for rustc-fake (rust-lang/rustc-perf#2509)
- Download Clippy when a Clippy profile is requested (rust-lang/rustc-perf#2508)
- Add early check for missing rustdoc/clippy in a toolchain (rust-lang/rustc-perf#2507)
- Add 2026-07-21 triage (rust-lang/rustc-perf#2506)
- use stable extract_if: since 1.87 (rust-lang/rustc-perf#2191)
- Update GitHub Actions (rust-lang/rustc-perf#2460)
- Update dependency @types/msgpack-lite to v0.1.12 (rust-lang/rustc-perf#2439)
- Add 30 day history link to artifact size tab on the compare page (rust-lang/rustc-perf#2505)
- Fix selecting color for the bootstrap chart on the toolchain page (rust-lang/rustc-perf#2504)
- Add artifact size history chart to toolchain page (rust-lang/rustc-perf#2501)
- Parallel frontend compiler support (perf backend only) (rust-lang/rustc-perf#2491)
- Run benchmark smoke test for all profiles on Windows on CI (rust-lang/rustc-perf#2503)
rust-bors Bot pushed a commit to rust-lang/rust that referenced this pull request Jul 30, 2026
Update cargo submodule




## src/tools/cargo

23 commits in 3efb1f477e99b42974b982d939fd100303cdf7db..7c83d4cc0953b81d823e47d640c64da9b8bd4fac
2026-07-17 23:53:19 +0000 to 2026-07-29 21:34:53 +0000
- fix: Pass rustdoc flags to final CCI merge step (rust-lang/cargo#17269)
- Reworked how we enable the new build-dir layout on nightly (rust-lang/cargo#17272)
- Allow setting `-Zembed-metadata` value from the config (rust-lang/cargo#17266)
- fix(cli): don't panic during completions when rustup is unavailable (rust-lang/cargo#17263)
- docs(workspace): add recommended structure to members field (rust-lang/cargo#17166)
- Update cargo-fetch.md to remove cargo-prefetch reference (rust-lang/cargo#16568)
- chore: bump to `libgit2-sys@0.18.7+1.9.6` (rust-lang/cargo#17259)
- Enable build-dir layout v2 on nightly by default (rust-lang/cargo#17258)
- fix(path): clarify error message when path dependency has wrong package (rust-lang/cargo#16927)
- fix(toml): warn on hyphenated lint names and duplicates (rust-lang/cargo#17051)
- fix(test): gate trim-paths tests on split debuginfo support (rust-lang/cargo#17256)
- test(git): Explicitly test for git injection attacks (rust-lang/cargo#17253)
- fix(git): Suggest libgit2 if git-cli fails (rust-lang/cargo#17252)
- fix(diag): bound transitive unused dependency traversal (rust-lang/cargo#17251)
- fix(git): Hide git fetch output without progress  (rust-lang/cargo#17243)
- revert(lint): Remove `new_implicit_minimum_version_req` (rust-lang/cargo#16321) (rust-lang/cargo#17249)
- fix: Add haiku's dylib path (rust-lang/cargo#17248)
- Zsh completion: Add `-p` and `--package` flags for `cargo add` (rust-lang/cargo#17247)
- refactor(source): Clarify the name of the remote git registry (rust-lang/cargo#17240)
- fix(timings): only report units the job queue actually ran (rust-lang/cargo#17238)
- Do not include proc-macro deps in rustc search path args (rust-lang/cargo#17236)
- chore(deps): update cargo-semver-checks to v0.49.0 (rust-lang/cargo#17237)
- rustdoc: rename the doc parts metadata params (rust-lang/cargo#17234)

## src/tools/rustc-perf

12 commits in 0508bdcd37152b28c39b6752828683cdd3f128b5..74ecbcdf88411937a6e39baf2779948565dfd388
2026-07-15 10:20:27 +0000 to 2026-07-27 15:02:48 +0000
- feat: support `@argfile` for rustc-fake (rust-lang/rustc-perf#2509)
- Download Clippy when a Clippy profile is requested (rust-lang/rustc-perf#2508)
- Add early check for missing rustdoc/clippy in a toolchain (rust-lang/rustc-perf#2507)
- Add 2026-07-21 triage (rust-lang/rustc-perf#2506)
- use stable extract_if: since 1.87 (rust-lang/rustc-perf#2191)
- Update GitHub Actions (rust-lang/rustc-perf#2460)
- Update dependency @types/msgpack-lite to v0.1.12 (rust-lang/rustc-perf#2439)
- Add 30 day history link to artifact size tab on the compare page (rust-lang/rustc-perf#2505)
- Fix selecting color for the bootstrap chart on the toolchain page (rust-lang/rustc-perf#2504)
- Add artifact size history chart to toolchain page (rust-lang/rustc-perf#2501)
- Parallel frontend compiler support (perf backend only) (rust-lang/rustc-perf#2491)
- Run benchmark smoke test for all profiles on Windows on CI (rust-lang/rustc-perf#2503)
@rustbot rustbot added this to the 1.99.0 milestone Jul 30, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

A-configuration Area: cargo config files and env vars A-unstable Area: nightly unstable support

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants