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

Inconsistency with publishing/vendoring listing of files #9555

Open
alexcrichton opened this issue Jun 7, 2021 · 3 comments
Open

Inconsistency with publishing/vendoring listing of files #9555

alexcrichton opened this issue Jun 7, 2021 · 3 comments
Labels
Command-package Command-vendor S-triage Status: This issue is waiting on initial triage.

Comments

@alexcrichton
Copy link
Member

Currently I'm seeing some weird behavior in Cargo with respect to vendoring and publishing. Notably this project will succeed to compile but then fail to compile with a vendored build:

$ cargo new foo
$ cd foo
$ echo 'zstd-sys = "=1.6.0"' >> Cargo.toml
$ cargo build
...
    Finished dev [unoptimized + debuginfo] target(s) in 30.45s
$ mkdir .cargo
$ cargo vendor >> .cargo/config.toml
$ cargo build
error: failed to run custom build command for `zstd-sys v1.6.0+zstd.1.5.0`

Caused by:
  process didn't exit successfully: `/Users/acrichton/code/foo/target/debug/build/zstd-sys-2992c87aa8f96b70/build-script-build` (exit code: 101)
  --- stderr
  thread 'main' panicked at 'Folder 'zstd/lib' does not exists. Maybe you forgot to clone the 'zstd' submodule?', /Users/acrichton/code/foo/vendor/zstd-sys/build.rs:180:13
  note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace

Sure enough the zstd folder is indeed missing from the vendored checkout. Some other tidbits:

I believe the problem here is that cargo vendor uses list_files to list the files that are going to be used for publication/building, and something funny is happening here with the presence of a .git folder. For example checking out zstd-sys and running cargo package -v does indeed vendor the zstd folder. If you delete the zstd/.git folder, however, and then run cargo package -v the zstd folder is not included. This I think best simulates what's happening with the registry because it's presumably published from a source checkout (therefore including zstd), but it's then vendored from a checkout without .git (what was downloaded from crates.io). This means that vendors don't have the zstd folder but downloads from crates.io do.

I haven't dug too much into this but the bug is likely around list_files and how the exclude directory is handled relative to discovered submodules. We should probably warn about this or handle this differently at publication time, ideally publication and vendoring would work with similar sets of files.

@ehuss
Copy link
Contributor

ehuss commented Jun 7, 2021

I believe this is caused by #8095. The filter check for directories doesn't handle the case where there is a ! exclusion. It might be a bit tricky to fix that and still keep #8092 working. I can try to take a look at it, though.

olivierlemasle added a commit to olivierlemasle/zstd-rs that referenced this issue Jun 7, 2021
Using `cargo vendor` with `zstd-sys` does not copy the `zstd` directory,
due to a bug in Cargo: rust-lang/cargo#9555

This commit replaces the "exclude" field with a negation, which
causes an issue with current version of Cargo, by an equivalent
"include" field.
olivierlemasle added a commit to olivierlemasle/zstd-rs that referenced this issue Jun 7, 2021
Using `cargo vendor` with `zstd-sys` does not copy the `zstd` directory,
due to a bug in Cargo: rust-lang/cargo#9555

This commit replaces the "exclude" field with a negation, which
causes an issue with current version of Cargo, by an equivalent
"include" field.
gyscos pushed a commit to gyscos/zstd-rs that referenced this issue Jun 7, 2021
Using `cargo vendor` with `zstd-sys` does not copy the `zstd` directory,
due to a bug in Cargo: rust-lang/cargo#9555

This commit replaces the "exclude" field with a negation, which
causes an issue with current version of Cargo, by an equivalent
"include" field.
@ehuss
Copy link
Contributor

ehuss commented Jun 7, 2021

So thinking about this more, I would almost say that the behavior when under .git is "wrong" in that it is not excluding directories. If you have a git repo with a .gitignore with the entries:

zstd
!zstd/LICENSE
!zstd/COPYING
!zstd/lib/**/**.h
!zstd/lib/**/**.c

Git won't allow you to add anything in the zstd directory without --force, and treats everything in zstd as ignored (the last 4 lines essentially have no meaning). I believe the correct entries to match that would be:

/zstd/**
!/zstd/lib
!/zstd/lib/compress
!/zstd/lib/dictBuilder
!/zstd/lib/decompress
!/zstd/lib/legacy
!/zstd/lib/common
!/zstd/lib/dll
!/zstd/lib/dll/example
!/zstd/lib/deprecated
!/zstd/LICENSE
!/zstd/COPYING
!/zstd/lib/**/**.h
!/zstd/lib/**/**.c

When a directory is ignored, subdirectories need to be negated explicitly. From the git docs:

Git doesn’t list excluded directories for performance reasons, so any patterns on contained files have no effect, no matter where they are defined.

(Or, better, just use include instead.)

WDYT?

@alexcrichton
Copy link
Member Author

Oh I don't have any thoughts here really personally, but matching git makes sense to me

@epage epage added the S-triage Status: This issue is waiting on initial triage. label Nov 20, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Command-package Command-vendor S-triage Status: This issue is waiting on initial triage.
Projects
None yet
Development

No branches or pull requests

3 participants