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.

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?

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

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

+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.

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

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.

I thing I would like to keep a full build test to be sure the whole feature keeps working.

CI failed cause new artifact-based test hit rustwide local manifest validation before our build path.

Which part fails?

It fails in rustwide’s local crate manifest validation step (before the docs.rs build path): rustwide::prepare::validate_manifest runs cargo metadata --manifest-path Cargo.toml --no-deps without -Zbindeps, so Cargo rejects artifact = "...".

I don't fully follow.

I can only see that build_local_package calls load_metadata_from_rustwide, which you both adapt in this PR?

So from what I see, any local crate manifest validation path would also be called in the "normal" build path?

It's totally possible I'm just missing context or details, but since I'll need to maintain it, I want to fully understand :)

Can you push a commit where I can see the failing test how you tried it? That would help .

generally I'm super happy this can make progress

I just tried to build the crate from the issue using your branch, using the "normal" docs.rs build, not test.

2026-03-14T12:35:01.734600Z INFO build_package{name=KrateName("protoc-plugin-by-closure") version=Version(Version { major: 0, minor: 1, patch: 6 }) kind=CratesIo collect_metrics=true}:build_package_inner{name=KrateName("prot oc-plugin-by-closure") version=Version(Version { major: 0, minor: 1, patch: 6 }) kind=CratesIo crate_id=CrateId(1) release_id=ReleaseId(1) build_id=BuildId(1) collect_metrics=true}: rustwide::cmd: running `Command { std: CARG O_HOME="/opt/docsrs/rustwide/cargo-home" RUSTUP_HOME="/opt/docsrs/rustwide/rustup-home" "/opt/docsrs/rustwide/cargo-home/bin/cargo" "+nightly" "metadata" "--manifest-path" "Cargo.toml" "--no-deps", kill_on_drop: false }` 2026-03-14T12:35:02.031737Z DEBUG build_package{name=KrateName("protoc-plugin-by-closure") version=Version(Version { major: 0, minor: 1, patch: 6 }) kind=CratesIo collect_metrics=true}:update_build_with_error{build_id=BuildId (1) build_error=Some(Other(invalid Cargo.toml syntax Stack backtrace: 0: anyhow::error::<impl core::convert::From<E> for anyhow::Error>::from 1: <T as core::convert::Into<U>>::into 2: rustwide::prepare::Prepare::validate_manifest 3: rustwide::prepare::Prepare::prepare 4: rustwide::build::BuildDirectory::run at ./usr/local/cargo/registry/src/index.crates.io-1949cf8c6b5b557f/rustwide-0.22.1/src/build.rs:197:17

This looks like the failing test that you saw was a sign that this feature wouldn't have worked at all for builds.

Did you manually run a build to test it?

In any case: I believe you should re-add the test, and then we can figure out what is necessary to fix it.

Perhaps even a change to rustwide?

enthropy7 · 2026-04-03T20:25:02Z

re-added the full build path test (test_bindeps_crate_full_build) that goes through build_local_package. as you spotted, it fails at rustwide::Prepare::validate_manifest - rustwide runs cargo metadata --no-deps without -Zbindeps before our build closure gets a chance to pass the flag. Same issue applies to capture_lockfile and fetch_deps inside Prepare::prepare().

i've been looking into rustwide and there's no API to pass extra cargo args to the prepare phase. i was working on a small PR in rustwide to, that adds extra_cargo_args support to BuildBuilder → Prepare, so we can forward the whitelisted flags there too.

syphar · 2026-04-07T06:03:12Z

thanks for the continued work!

( I'm sick at the moment, will probably be able to look through both PRs at some point this week or next week)

enthropy7 · 2026-04-07T10:33:20Z

hi, thank you, i always get things done:) get well! looking forward to finish work on this together

syphar · 2026-05-22T10:51:54Z

We're happy to pick this topic back up, just wanted to clean up open PRs

syphar · 2026-07-02T01:38:58Z

From this comment:

The user has been banned from the rust-lang org and will not be providing more updates here

syphar · 2026-08-17T09:09:29Z

I assume this has to be rebased on main,

also please remove the "merge main" commit

rustbot · 2026-08-17T16:17:40Z

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.

`-Z bindeps` from `cargo-args` only reached `cargo rustdoc`, so `cargo metadata`, `generate-lockfile` and `fetch` still choked on manifests containing `artifact = "bin"` and the build died before rustdoc ran. rustwide 0.26 lets us cover its prepare phase too, which fails earliest of all — at `validate_manifest`. Its `fetch_build_std_dependencies` drops the extra args, so that one fetch we run ourselves. Host cargo runs unsandboxed, so this forwards a whitelist rather than the whole `cargo-args` — `bindeps` alone for now.

syphar · 2026-08-18T07:43:38Z

+                        build.fetch_build_std_dependencies(&targets)?;
+                    } else {
+                        // `Build` doesn't carry the `extra_cargo_args` given to `BuildBuilder`,
+                        // so rustwide would run this fetch without them and fail to parse the


I have the impression this should be fixed in rustwide? So fetch_build_std_dependencies will work with extra args directly?

View changes since the review

syphar · 2026-08-18T07:44:43Z


            krate.copy_source_to(&self.workspace, temp_dir.path())?;

+            // Read from this copy: rustwide's prepare phase needs the flags before the build


Shouldn't we fix rustwide to have the flags available when the build needs them?

Or am I missing something?

View changes since the review

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

syphar reviewed Jan 9, 2026

View reviewed changes

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

syphar removed their assignment Jan 9, 2026

syphar requested changes Feb 11, 2026

View reviewed changes

Comment thread crates/lib/metadata/lib.rs Outdated

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

This comment has been minimized.

Sign in to view

enthropy7 commented Mar 11, 2026

View reviewed changes

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

This comment has been minimized.

Sign in to view

GuillaumeGomez closed this May 22, 2026

syphar reopened this May 22, 2026

github-actions Bot added S-waiting-on-review Status: This pull request has been implemented and needs to be reviewed and removed S-waiting-on-author Status: This PR is incomplete or needs to address review comments labels May 22, 2026

syphar marked this pull request as draft July 2, 2026 01:38

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 Aug 11, 2026

github-actions Bot added S-waiting-on-review Status: This pull request has been implemented and needs to be reviewed and removed S-waiting-on-author Status: This PR is incomplete or needs to address review comments labels Aug 12, 2026

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 Aug 17, 2026

syphar removed their request for review August 17, 2026 09:09

syphar requested changes Aug 18, 2026

View reviewed changes


		krate.copy_source_to(&self.workspace, temp_dir.path())?;

		// Read from this copy: rustwide's prepare phase needs the flags before the build

Uh oh!

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!

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!

This comment has been minimized.

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!

enthropy7 commented Apr 3, 2026

Uh oh!

syphar commented Apr 7, 2026

Uh oh!

enthropy7 commented Apr 7, 2026

Uh oh!

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

syphar commented May 22, 2026

Uh oh!

syphar commented Jul 2, 2026

Uh oh!

enthropy7 commented Jan 27, 2026 via email •

edited

Loading

syphar left a comment •

edited by rustbot

Loading

syphar Aug 18, 2026 •

edited by rustbot

Loading

syphar Aug 18, 2026 •

edited by rustbot

Loading