bindeps failing build fix by enthropy7 · Pull Request #3111 · rust-lang/docs.rs

enthropy7 · 2026-01-01T20:09:21Z

fixed #2710 by ensuring unstable cargo flags (e.g., -Zbindeps) from [package.metadata.docs.rs] cargo-args are passed to all cargo commands, before it was only rustdoc. Added Metadata::unstable_cargo_flags() to extract -Z* flags and updated load_metadata_from_rustwide() and all cargo invocations (metadata, fetch, generate-lockfile, rustdoc) to accept and forward these flags. This lets crates using unstable features build on docs.rs. change is minimal, backward compatible, and follows existing patterns, covering all cargo commands to prevent failures at any stage. all tests added in 2 commits. 1-st demonstrate how it was before, 2-nd - how it works now (well).

enthropy7 · 2026-01-03T13:36:58Z

r? @syphar take a look please if you have time :>

syphar · 2026-01-04T10:55:23Z

r? @syphar take a look please if you have time :>

Already saw it, needed to finish a bigger refactor (#3113).

Also need to check the implications of adding this argument to all commands. Give me a couple of days.

Until then:
the linters fail, you can already fix that. (running just lint locally should suffice)

enthropy7 · 2026-01-04T12:05:11Z

r? @syphar take a look please if you have time :>

Already saw it, needed to finish a bigger refactor (#3113).

Also need to check the implications of adding this argument to all commands. Give me a couple of days.

Until then: the linters fail, you can already fix that. (running just lint locally should suffice)

thanks! about link - it’s false detection as i remember, but i will check it again

syphar · 2026-01-04T12:06:20Z

thanks! about link - it’s false detection as i remember, but i will check it again

in what way? It's rustfmt failing, so you have to run it locally.

Also check if you're using the latest rust stable.

enthropy7 · 2026-01-04T12:21:47Z

thanks! about link - it’s false detection as i remember, but i will check it again

in what way? It's rustfmt failing, so you have to run it locally.

Also check if you're using the latest rust stable.

oh, sorry then, i’m currently working on many projects, something mixed up in my brain. will fix this if a new few hours

syphar · 2026-01-09T15:59:12Z

starting to look into this

syphar

I need to do more research here before we can merge this.

To explain:
we have two ways of running cargo commands here.

"safe" commands are run directly on the docs.rs server.
"unsafe" commands are run inside a docker container

in the code:

Command::new is on the host,
stuff behind prepare_sandbox or in methods accepting a &Build is sandboxed

All commands where a crate author could run their own code (like in a build script) in any way have to be sandboxed, otherwise it would be possible for one crate author to affect the build output of other crates, so also what's hosted / distributed via docs.rs

For the current commands that are run on the host (generate-lockfile, fetch, etc) we know that there is no way you can affect the host in any way.

What I don't know: Does bindeps or any other unstable cargo feature open up these possibilities? If yes, we need to also sandbox these commands.

I'll try to get someone from the cargo team to provide more insight.

crates/bin/docs_rs_builder/tests/crates/bindeps-test/Cargo.toml

syphar · 2026-01-09T19:32:59Z

just a note: please don't start sandboxing for now, generally sandboxed commands run slower, and we would have to keep the sandbox limits in mind. :)

syphar · 2026-01-10T03:15:51Z

@enthropy7 generally:
when we do a review, it's better when you just add commits in the branch.

Like this I can directly see what exactly changed after the last review.

( for long-running reviews I even recommend not rebasing on main until before the merge).

enthropy7 · 2026-01-10T19:54:21Z

@enthropy7 generally: when we do a review, it's better when you just add commits in the branch.

Like this I can directly see what exactly changed after the last review.

( for long-running reviews I even recommend not rebasing on main until before the merge).

ok, thanks, that's just practice i learnt from cargo rules of commit :)))

syphar · 2026-01-27T14:55:28Z

short update: I'm still on this, and trying to get the info I need

enthropy7 · 2026-01-27T14:58:19Z

thanks for the information! i don’t forget about it either

syphar · 2026-02-11T04:42:02Z

@enthropy7
#t-cargo > docs.rs / cargo bindeps feature?

sorry for the delay.

syphar

I got a response from the cargo team.

Also, we should update our metadata help-page with the new info?

crates/lib/metadata/lib.rs

enthropy7 · 2026-03-06T09:24:33Z

@syphar hi, finally found time to finish it properly, new approach implemented and your suggestions too

syphar

Super nice work!

I'm very happy to see this progress.

Two small things, and one question remaining.

View changes since this review

tests/crates

test_unstable_flags

syphar · 2026-03-10T06:31:43Z

crates/bin/docs_rs_builder/tests/crates/bindeps-test/Cargo.toml

+edition = "2021"
+
+[package.metadata.docs.rs]
+cargo-args = ["-Zbindeps"]


More a question, because I don't know:

Would this bindeps-test crate fail the docs-build without the changes in this PR? Or would the arg just be added to the main build, and the other calls (cargo metadata) would fail?

hi again! thanks for your kind words :)

i rebased the branch and removed artifacts from my builds and tests, so the PR now only contains the intended files.

about your question on bindeps-test: without this PR, docs.rs would fail before the main rustdoc build, during host-side cargo commands (cargo metadata / lockfile/fetch path), because the manifest uses artifact dependencies and Cargo requires -Z bindeps for parsing/resolution there. normal docs build command already received cargo-args. the missing part was forwarding the required flag to those host-side commands too.

i also fixed the new audit failure by updating quinn-proto in Cargo.lock

I think this is the last piece missing for me :) Thank you for working on this!

One thing I'm still not sure about (might be lack of knowledge) is if this test crate would really fail to build without this PR?

Wouldn't a cleaner example be one where we actually artifact dependencies? like the one mentioned in #2710?

done, thanks for the suggestion. changed bindeps-test to use a real artifact dependency, now it fails on cargo metadata without -Z bindeps and succeeds with it. and it demonstrates exactly why forwarding -Zbindeps to host-side cargo commands is needed.

This commit adds tests that demonstrate the problem: crates using unstable cargo features like `-Zbindeps` fail to build because the unstable flags from `cargo-args` are not passed to `cargo metadata` and `cargo fetch` commands. - Add test_bindeps_cargo_args_parsing() to verify cargo-args parsing - Add test_bindeps_separate_args_parsing() for -Z bindeps style - Add test_bindeps_crate_fails_without_unstable_flags() integration test that EXPECTS FAILURE (demonstrates the issue) - Add test_load_metadata_signature_doesnt_accept_unstable_flags() to document the issue - Add test crate tests/crates/bindeps-test/ with bindeps configuration These tests demonstrate the issue: 1. load_metadata_from_rustwide() doesn't accept unstable flags parameter 2. cargo metadata/fetch are called without -Z flags from cargo-args 3. This causes builds to fail for crates using bindeps See: rust-lang#2710

This commit fixes issue rust-lang#2710 by ensuring that unstable cargo flags (e.g., `-Zbindeps`) from `cargo-args` are passed to ALL cargo commands, not just `cargo rustdoc`. Changes: - Add `Metadata::unstable_cargo_flags()` method to extract `-Z*` flags from `cargo-args` - Update `load_metadata_from_rustwide()` to accept `unstable_flags` parameter and pass them to `cargo metadata` - Update `build_local_package()` to read metadata and pass unstable flags - Update `execute_build()` to pass unstable flags to `cargo metadata` - Update fallback path (`cargo generate-lockfile`, `cargo fetch`) to pass unstable flags - Remove unused `.peekable()` in `unstable_cargo_flags()` method - Apply rustfmt formatting - Update test to expect success (with fix)

rustbot · 2026-03-10T11:40:25Z

This PR was rebased onto a different main commit. Here's a range-diff highlighting what actually changed.

Rebasing is a normal part of keeping PRs up to date, so no action is needed—this note is just to help reviewers.

enthropy7 · 2026-03-11T01:02:12Z

crates/bin/docs_rs_builder/src/docbuilder/rustwide_builder.rs

CI failed cause new artifact-based test hit rustwide local manifest validation before our build path.
i changed the test to check cargo metadata flag forwarding directly - fails without -Zbindeps, passes with it.

enthropy7 requested a review from a team as a code owner January 1, 2026 20:09

github-actions bot added the S-waiting-on-review Status: This pull request has been implemented and needs to be reviewed label Jan 1, 2026

rustbot assigned syphar Jan 3, 2026

enthropy7 force-pushed the main branch 3 times, most recently from 962dca4 to f2f2850 Compare January 4, 2026 14:03

syphar reviewed Jan 9, 2026

View reviewed changes

crates/bin/docs_rs_builder/tests/crates/bindeps-test/Cargo.toml Show resolved Hide resolved

syphar removed their assignment Jan 9, 2026

enthropy7 force-pushed the main branch 5 times, most recently from be755ba to dc3f5f1 Compare January 9, 2026 23:45

syphar requested changes Feb 11, 2026

View reviewed changes

crates/lib/metadata/lib.rs Outdated Show resolved Hide resolved

syphar added S-waiting-on-author Status: This PR is incomplete or needs to address review comments and removed S-waiting-on-review Status: This pull request has been implemented and needs to be reviewed labels Feb 24, 2026

This comment has been minimized.

Sign in to view

syphar requested changes Mar 10, 2026

View reviewed changes

syphar removed the S-waiting-on-author Status: This PR is incomplete or needs to address review comments label Mar 10, 2026

enthropy7 added 4 commits March 10, 2026 14:37

Whitelist host-side unstable cargo flags to bindeps and document it.

71bc093

Remove accidental local artifacts from the PR.

b67da7f

enthropy7 force-pushed the main branch from 82d7e53 to b67da7f Compare March 10, 2026 11:40

enthropy7 added 3 commits March 10, 2026 15:08

Update quinn-proto to 0.11.14 for RUSTSEC-2026-0037.

8154a5e

Use real artifact dependency in bindeps test crate.

124aaba

Make bindeps test validate metadata flag forwarding directly.

87bd166

enthropy7 commented Mar 11, 2026

View reviewed changes

Conversation

enthropy7 commented Jan 1, 2026

Uh oh!

enthropy7 commented Jan 3, 2026

Uh oh!

syphar commented Jan 4, 2026

Uh oh!

enthropy7 commented Jan 4, 2026

Uh oh!

syphar commented Jan 4, 2026

Uh oh!

enthropy7 commented Jan 4, 2026

Uh oh!

syphar commented Jan 9, 2026

Uh oh!

syphar left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

syphar commented Jan 9, 2026

Uh oh!

syphar commented Jan 10, 2026

Uh oh!

enthropy7 commented Jan 10, 2026

Uh oh!

syphar commented Jan 27, 2026

Uh oh!

enthropy7 commented Jan 27, 2026 via email • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

syphar commented Feb 11, 2026

Uh oh!

syphar left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

This comment has been minimized.

enthropy7 commented Mar 6, 2026

Uh oh!

syphar left a comment • edited by rustbot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

syphar Mar 10, 2026

Choose a reason for hiding this comment

Uh oh!

enthropy7 Mar 10, 2026

Choose a reason for hiding this comment

Uh oh!

syphar Mar 10, 2026

Choose a reason for hiding this comment

Uh oh!

enthropy7 Mar 10, 2026

Choose a reason for hiding this comment

Uh oh!

rustbot commented Mar 10, 2026

Uh oh!

enthropy7 Mar 11, 2026

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

enthropy7 commented Jan 27, 2026 via email •

edited

Loading

syphar left a comment •

edited by rustbot

Loading