Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

x.py dist shouldn’t require network access #79218

Closed
Keruspe opened this issue Nov 19, 2020 · 10 comments · Fixed by #80082
Closed

x.py dist shouldn’t require network access #79218

Keruspe opened this issue Nov 19, 2020 · 10 comments · Fixed by #80082
Assignees
Labels
P-critical Critical priority regression-from-stable-to-nightly Performance or correctness regression from stable to nightly. T-infra Relevant to the infrastructure team, which will review and decide on the PR/issue. T-release Relevant to the release subteam, which will review and decide on the PR/issue.
Milestone

Comments

@Keruspe
Copy link
Contributor

Keruspe commented Nov 19, 2020

Since #78790 we run cargo vendor unconditionally to vendor libtest in libstd.
We should either pass it '--offline' or add a way to conditionally do so to avoid using network when unnecessary

@Mark-Simulacrum Mark-Simulacrum added P-critical Critical priority T-release Relevant to the release subteam, which will review and decide on the PR/issue. regression-from-stable-to-beta Performance or correctness regression from stable to beta. labels Nov 19, 2020
@Mark-Simulacrum Mark-Simulacrum added this to the 1.49.0 milestone Nov 19, 2020
@Mark-Simulacrum Mark-Simulacrum added the T-infra Relevant to the infrastructure team, which will review and decide on the PR/issue. label Nov 19, 2020
@Mark-Simulacrum
Copy link
Member

I am happy to accept a patch for this; if it is not small (e.g., just adding offline) we will likely end up wanting to revert #78790 on beta.

@camelid
Copy link
Member

camelid commented Nov 20, 2020

Is this really P-critical? I would think this is P-high since P-critical is usually reserved for severe user-facing bugs.

@Mark-Simulacrum
Copy link
Member

I consider inability for distros to build a particular version of Rust to be a release blocker.

@Keruspe
Copy link
Contributor Author

Keruspe commented Nov 20, 2020

Here what i've tried so far:

  • cargo vendor --offline
  • cargo vendor --locked
  • cargo generate-lockfile --offline && cargo vendor --locked
  • cargo generate-lockfile --offline && cargo vendor --locked --offline

None of those worked out, last failure:

running: "/usr/x86_64-pc-linux-gnu/bin/cargo-beta" "generate-lockfile" "--offline"
running: "/usr/x86_64-pc-linux-gnu/bin/cargo-beta" "vendor" "--offline" "--locked" "/var/tmp/paludis/build/dev-lang-rust-1.49.0-scm/work/rust-1.49.0-scm/build/tmp/dist/rust-src-beta-image/lib/rustlib/src/rust/vend
or"
                                                                                                 
command did not execute successfully: "/usr/x86_64-pc-linux-gnu/bin/cargo-beta" "vendor" "--offline" "--locked" "/var/tmp/paludis/build/dev-lang-rust-1.49.0-scm/work/rust-1.49.0-scm/build/tmp/dist/rust-src-beta-im
age/lib/rustlib/src/rust/vendor"
expected success, got: exit code: 101                                                                              
error: failed to sync            
Caused by:                   
  failed to download packages                                                               
Caused by:               
  failed to download `rustc-std-workspace-alloc v1.0.0`
Caused by:                                                                                                                                                                                                           
  can't make HTTP request in the offline mode   

@Keruspe
Copy link
Contributor Author

Keruspe commented Nov 20, 2020

With just the cargo vendor --locked --offline (without the generate-lockfile):

Caused by:
  failed to load pkg lockfile                                                                                                                                   
Caused by:                                                                                                
  can't update in the offline mode

@camelid
Copy link
Member

camelid commented Nov 20, 2020

I consider inability for distros to build a particular version of Rust to be a release blocker.

Ah, I didn't realize it was that bad! Sorry about that.

@Mark-Simulacrum
Copy link
Member

cc @rust-lang/cargo -- do we expect cargo vendor to work without network when vendoring a subset of a workspace? It looks like we're running into some trouble here to get it working, though I've not personally had time yet to test.

cc @Gankra as well

@alexcrichton
Copy link
Member

I believe Cargo is working as intended here, when you run cargo vendor in the workspace root it doesn't actually vendor the rustc-std-workspace-* crates from crates.io because they're patched locally to the in-tree versions. Then when Cargo later vendors for libtest, it doesn't see the [patch] annotations so it tries to download them from the network. If you've passed --offline and you're using a vendor directory that means Cargo can't download any more packages, so it provides this error.

I think the fix here is to either get Cargo to see the [patch] for these crates during the libtest vendoring process or somehow get those crates vendored in whatever does the original vendor.

@Keruspe
Copy link
Contributor Author

Keruspe commented Nov 20, 2020

Will give the [patch] approach a try, thanks a lot for the explanation

edit: assuming adding the patch section to e.g. libtest Cargo.toml can work

Keruspe added a commit to Keruspe/rust that referenced this issue Nov 22, 2020
Sadly, we need to copy the [patch] section from the toplevel Cargo.toml
for the --offline mode to properly work

Fixes rust-lang#79218

Signed-off-by: Marc-Antoine Perennou <[email protected]>
@Mark-Simulacrum Mark-Simulacrum modified the milestones: 1.49.0, 1.50.0 Dec 10, 2020
@Mark-Simulacrum Mark-Simulacrum added regression-from-stable-to-nightly Performance or correctness regression from stable to nightly. and removed regression-from-stable-to-beta Performance or correctness regression from stable to beta. labels Dec 10, 2020
@ehuss
Copy link
Contributor

ehuss commented Dec 16, 2020

I have posted a revert for the current release (#80082) since there were several issues on the Cargo side, too. Hopefully we'll try to avoid these issues when we try again.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
P-critical Critical priority regression-from-stable-to-nightly Performance or correctness regression from stable to nightly. T-infra Relevant to the infrastructure team, which will review and decide on the PR/issue. T-release Relevant to the release subteam, which will review and decide on the PR/issue.
Projects
None yet
5 participants