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

GenerateCopyright attempts to vendor sources during installation #136955

Closed
Kangie opened this issue Feb 13, 2025 · 9 comments · Fixed by #137020
Closed

GenerateCopyright attempts to vendor sources during installation #136955

Kangie opened this issue Feb 13, 2025 · 9 comments · Fixed by #137020
Labels
A-licensing Area: Compiler licensing C-bug Category: This is a bug. T-bootstrap Relevant to the bootstrap subteam: Rust's build system (x.py and src/bootstrap)

Comments

@Kangie
Copy link

Kangie commented Feb 13, 2025

Gentoo Linux builds are primarily performed in a sandbox that prevents network activity after sources are fetched.

Unfortunately this leaves us unable to package nightly as GenerateCopyright is invoked by x.py install, which attempts to vendor crates into the build directory.

  1. GenerateCopyright should only be run when generating the source tarball, or should at the very least at least be a separate step from Rustc
  2. src/tools/generate-copyright should respect config.vendor

I tried this:

`x.py install ...

I expected to see this happen:

A successful installation into DESTDIR

Instead, this happened:

running: cd "/var/tmp/portage/dev-lang/rust-9999/work/rust-9999" && CARGO="/opt/rust-bin-9999/bin/cargo" DEST="/var/tmp/portage/dev-lang/rust-9999/work/rust-9999/build/COPYRIGHT.html" DEST_LIBSTD="/var/tmp/portage/dev-lang/rust-9999/work/rust-9999/build/COPYRIGHT-library.html" LD_LIBRARY_PATH="/var/tmp/portage/dev-lang/rust-9999/work/rust-9999/build/x86_64-unknown-linux-gnu/stage0-bootstrap-tools/x86_64-unknown-linux-gnu/release/deps:/opt/rust-bin-9999/lib" LICENSE_METADATA="/var/tmp/portage/dev-lang/rust-9999/work/rust-9999/license-metadata.json" OUT_DIR="/var/tmp/portage/dev-lang/rust-9999/work/rust-9999/build" RUSTC="/opt/rust-bin-9999/bin/rustc" "/var/tmp/portage/dev-lang/rust-9999/work/rust-9999/build/x86_64-unknown-linux-gnu/stage0-tools-bin/generate-copyright" (failure_mode=Exit) (created at src/bootstrap/src/core/build_steps/tool.rs:1211:23, executed at src/bootstrap/src/core/build_steps/run.rs:224:13)
Vendoring deps into /var/tmp/portage/dev-lang/rust-9999/work/rust-9999/build/vendor...
error: failed to sync

Caused by:
  failed to load pkg lockfile

Caused by:
  failed to get `tikv-jemalloc-sys` as a dependency of package `rustc-main v0.0.0 (/var/tmp/portage/dev-lang/rust-9999/work/rust-9999/compiler/rustc)`

Caused by:
  download of config.json failed

Caused by:
  failed to download from `https://index.crates.io/config.json`

Caused by:
  [6] Could not resolve hostname (Could not resolve host: index.crates.io)
Error: Failed to complete cargo vendor

rust-9999:20250213-003653.log.xz.txt

rustc --version --verbose:

rustc 1.86.0-nightly (92bedea1c 2025-02-11)
binary: rustc
commit-hash: 92bedea1c51e3a969d60972be854506ffd8c5cb6
commit-date: 2025-02-11
host: x86_64-unknown-linux-gnu
release: 1.86.0-nightly
LLVM version: 19.1.7

@Kangie Kangie added the C-bug Category: This is a bug. label Feb 13, 2025
@rustbot rustbot added the needs-triage This issue may need triage. Remove it if it has been sufficiently triaged. label Feb 13, 2025
@jyn514
Copy link
Member

jyn514 commented Feb 13, 2025

This happens because x install internally invokes x dist, which includes x dist rustc. dist::Rustc unconditionally calls builder.ensure(GenerateCopyright). I don't think those two components should be coupled together like that, GenerateCopyright should only be run when generating PlainSourceTarball.

@rustbot label T-bootstrap A-licensing

@jyn514
Copy link
Member

jyn514 commented Feb 13, 2025

I think running cargo vendor with the same vendor/ directory as the root actually overwrites any existing vendor sources. so, that's another bug here. all the vendoring needs to be done in a single invocation managed by bootstrap, not piecemeal.

@jyn514
Copy link
Member

jyn514 commented Feb 13, 2025

cc #128353, @jonathanpallant

@rustbot rustbot added A-licensing Area: Compiler licensing T-bootstrap Relevant to the bootstrap subteam: Rust's build system (x.py and src/bootstrap) labels Feb 13, 2025
@jonathanpallant
Copy link
Contributor

The generated HTML needs to be shipped with the binary dist of rustc, not the source tarball, because it contains all things that must be supplied alongside any 'redistribution' of 'the software'.

cc @pietroalbini

@onur-ozkan onur-ozkan removed the needs-triage This issue may need triage. Remove it if it has been sufficiently triaged. label Feb 13, 2025
@jonathanpallant
Copy link
Contributor

Probably shouldn't be vendoring twice though - I can look at using the existing vendor folder if one has already been fetched.

@jyn514
Copy link
Member

jyn514 commented Feb 13, 2025

please do not try to replicate the vendoring logic in a separate tool. instead, do the vendoring all in one place in bootstrap.

i understand that this may run even when config.vendor is disabled, so you could switch on that in bootstrap (ideally it would put sources used only for parsing copyright in a different directory on disk). but it should not live in a separate tool.

@Kangie
Copy link
Author

Kangie commented Feb 13, 2025

The generated HTML needs to be shipped with the binary dist of rustc, not the source tarball, because it contains all things that must be supplied alongside any 'redistribution' of 'the software'.

I can't see any reason that shipping it in a tarball doesn't meet those requirements. We don't even use html files in Gentoo anyway; we have our own mechanism for identifying and managing licensing (and most [all?] distros do the same thing).

If I needed to refer to them I would:

  1. Grab a copy from an appropriate tarball
  2. Build a copy myself from an appropriate git tag

It seems superfluous to run that implicitly as part of the bootstrap x.py install component.

@pietroalbini
Copy link
Member

please do not try to replicate the vendoring logic in a separate tool. instead, do the vendoring all in one place in bootstrap.

Yeah, I'll open a PR soon to have bootstrap retrieve the vendored sources (from config.vendor or from the network if needed) and pass them to generate-copyright.

I can't see any reason that shipping it in a tarball doesn't meet those requirements. We don't even use html files in Gentoo anyway; we have our own mechanism for identifying and managing licensing (and most [all?] distros do the same thing).

[...]

It seems superfluous to run that implicitly as part of the bootstrap x.py install component.

When the Rust project distributes pre-built binaries, we cannot distribute the license file just in the source tarball: if an user just downloads rustc-1.84.0-$target.tar.xz they need to receive the licenses. Those tarballs are produced by x.py dist.

To reduce duplication, x.py install executes x.py dist behind the scenes (with a few variations), so it inherits all of the behavior of x.py dist, including generating the license files. Requiring network access when using vendored sources is a bug that will be fixed, but if you want to disable generating the licensing information altogether that'd be a separate issue.

@pietroalbini
Copy link
Member

Opened #137020 with the fix.

matthiaskrgr added a commit to matthiaskrgr/rust that referenced this issue Feb 17, 2025
Pass vendored sources from bootstrap to generate-copyright

In addition to doing the vendoring in bootstrap, this PR also loads the list of manifests to parse from bootstrap (instead of hardcoding a smaller list in generate-copyright). This is best reviewed commit-by-commit.

Fixes rust-lang#136955
rust-timer added a commit to rust-lang-ci/rust that referenced this issue Feb 18, 2025
Rollup merge of rust-lang#137020 - ferrocene:pa-vendor-sources, r=Kobzol

Pass vendored sources from bootstrap to generate-copyright

In addition to doing the vendoring in bootstrap, this PR also loads the list of manifests to parse from bootstrap (instead of hardcoding a smaller list in generate-copyright). This is best reviewed commit-by-commit.

Fixes rust-lang#136955
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-licensing Area: Compiler licensing C-bug Category: This is a bug. T-bootstrap Relevant to the bootstrap subteam: Rust's build system (x.py and src/bootstrap)
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants