rust-lang / cargo

The Rust package manager
https://doc.rust-lang.org/cargo
Apache License 2.0
12.75k stars 2.42k forks source link

`cargo vendor`'ing a `git` crate that includes a `workspace` misses important files #8885

Open aidancully opened 3 years ago

aidancully commented 3 years ago

Problem I created a Cargo.toml via cargo new hello, and tried to put a relibc dependency in the Cargo.toml, as below:

...
[dependencies.relibc]
git = "https://gitlab.redox-os.org/redox-os/relibc"
rev = "07ec3b6591878f23f3c4be80c26cbfc584abfe43"

When I run cargo build, the repository gets fetched and built as I hoped, but when I run cargo vendor, the generated vendor/relibc directory is missing important files and directories, such as https://gitlab.redox-os.org/redox-os/relibc/-/blob/master/src/ld_so/access.rs, and cargo build from the vendor/relibc directory fails.

Steps

  1. cargo new hello && cd hello
  2. add the above dependency to hello's Cargo.toml
  3. cargo vendor

At this point, I expect cd vendor/relibc && cargo build to work, but it does not. On the other hand, cargo build will successfully build the binary.

Possible Solution(s) I dug into this, the issue appears due to https://github.com/rust-lang/cargo/blob/64a468261aa05f2cd7a0d65dc1a6b4a576ed1938/src/cargo/sources/path.rs#L286-L297

The workspace module directories contain Cargo.toml files, causing these directories to get added to subpackages_found, and their files to be filtered out of the destination. The issue seems to be that this code wasn't updated to understand the fact that Workspaces allow several Cargo.toml's to participate in building a single package.

Notes

Output of cargo version: cargo 1.49.0

alexcrichton commented 3 years ago

I think that there may be a misunderstanding perhaps? I don't believe that the relibc is intended to be used as a Cargo dependency since I get

warning: The package `relibc` provides no linkable target. The compiler might raise an error while compiling `foo`. Consider adding 'dylib' or 'rlib' to key `crate-type` in `relibc`'s Cargo.toml. This warning might turn into a hard error in the future.

Additionally cargo build inside of the vendor directory builds the outer project, not the vendor directory (Cargo goes up a directory to find Cargo.toml).

Could you describe a bit more what you're trying to do here?

aidancully commented 3 years ago

Sorry, I made some mistakes transcribing what I did from my work environment to here, I've edited the issue text to be more accurate.

relibc provides a staticlib crate type, see https://gitlab.redox-os.org/redox-os/relibc/-/blob/master/Cargo.toml#L7-9

cargo build works for relibc, but cargo vendor a project that uses relibc, and you can't cargo build the vendored directory.

As for what I'm trying to do, I'm basically trying to build a cdylib that doesn't contain any external library references (no libc.so, no libpthread.so) using Bazel, via cargo raze. After getting a proof-of-concept that interacted with Linux via the sc crate, I tried to raise the level of abstraction in my library by embedding relibc, and encountered this issue. I'll try using the musl target, but this still seems like an issue that should be fixed in cargo.

alexcrichton commented 3 years ago

I think this may be a mismatch with Cargo and vendoring in that case? Vendoring is intended for Rust (rlib) dependencies, not for final end-products like executables/dylibs/staticlibs. Cargo doesn't have the concept of building a cdylib or staticlib dependency.

aidancully commented 3 years ago

I'm pretty sure I can construct a failing example that generates an rlib, if that helps. Cargo isn't copying all the files it needs to, because it mistakenly believes that files in git repositories that live in the same directory as a (non-top-level) Cargo.toml are sub-packages, but in this case they are members of the top-level workspace.

alexcrichton commented 3 years ago

Indeed yeah that would be helpful!

aidancully commented 3 years ago

The problem is slightly different than I thought it was. Here's an example of a tree that causes the issue:

.
|-- Cargo.lock
|-- Cargo.toml
`-- src
    |-- lib.rs
    `-- member
        |-- Cargo.toml
        |-- mod.rs
        `-- src
            `-- lib.rs

src/member includes a Cargo.toml for building src/member/src, and a mod.rs that's a module for src/lib.rs.

https://github.com/aidancully/cargo-8885-requirer includes a Cargo.toml that uses a project structured as above. When you cargo build that, you'll see that it builds correctly. But cargo vendor, and follow the directions for building via the vendored repository, and the build will fail: the src/member/mod.rs that's technically part of src/lib.rs does not get copied into the vendor directory.

This matches what I observed with relibc.

aidancully commented 3 years ago

cargo package fails for https://github.com/aidancully/cargo-8885-provider, though cargo build succeeds.

ehuss commented 3 years ago

I think this is generally working as expected. Having overlapping packages like that isn't something Cargo really supports. I think the solution here is to ensure each package exists independently in its own subtree.

aidancully commented 3 years ago

I can believe that it's working as more-or-less intended, but it's still surprising that cargo build can succeed when cargo package or cargo vendor fails, and the failure investigation was fairly painful. Ideally, cargo build, cargo package, cargo vendor etc. would all have the same idea of the files needed... One way might be to interact with rustc, and ask it what files it's compiling... Another might be to take a page from Bazel, and build in a sandbox that only refers to the same rust files that cargo package et al know about.. Or we might make the packaging less conservative, and allow it to include files from sub-packages, even if they don't get used downstream.