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

Cargo package says the repo is dirty, but the files are gitignored #12294

Open
scullionw opened this issue Jun 21, 2023 · 20 comments
Open

Cargo package says the repo is dirty, but the files are gitignored #12294

scullionw opened this issue Jun 21, 2023 · 20 comments
Labels
A-git Area: anything dealing with git C-bug Category: bug Command-package S-needs-design Status: Needs someone to work further on the design for the feature or fix. NOT YET accepted.

Comments

@scullionw
Copy link

scullionw commented Jun 21, 2023

Problem

I have a folder called "extra" that is in my .gitignore. git status confirms it

Screenshot 2023-06-21 at 8 24 38 AM

Cargo package says the repo is dirty, but those files are gitignored.

Screenshot 2023-06-21 at 8 25 20 AM

Note: In my Cargo.toml I am adding this extra folder to the include field, because i want this folder present in the package (but not in git).

Steps

  1. Add a folder with some data to the .gitignore
  2. Include that folder in the cargo package using the include field in the Cargo.toml

Possible Solution(s)

No response

Notes

Normally I worked around the bug with "--allow-dirty", but now that i've switched to cargo release to publish my workspace, I cannot (it doesn't support that flag)

Version

cargo version --verbose
cargo 1.70.0 (ec8a8a0ca 2023-04-25)
release: 1.70.0
commit-hash: ec8a8a0cabb0e0cadef58902470f6c7ee7868bdc
commit-date: 2023-04-25
host: aarch64-apple-darwin
libgit2: 1.6.3 (sys:0.17.0 vendored)
libcurl: 7.87.0 (sys:0.4.61+curl-8.0.1 system ssl:(SecureTransport) LibreSSL/3.3.6)
os: Mac OS 13.3.0 [64-bit]
@scullionw scullionw added C-bug Category: bug S-triage Status: This issue is waiting on initial triage. labels Jun 21, 2023
@weihanglo
Copy link
Member

Thanks for filing this report, and sorry for the inconvenience.

The function check_repo_state() is how Cargo checks VCS (Git) status to determine whether the repo is dirty. Roughly it compares src files, which are what you list in include directive, to git status/untracked/ignored files. The implementation was done in #9478. I wasn't there but so far as I can tell, it is intentional to report “dirty” when a file is gitignore'd but still included in include. Here is a test verifying this behavior:

#[cargo_test]
fn dirty_ignored() {
// Cargo warns about an ignored file that will be published.
let (p, repo) = git::new_repo("foo", |p| {
p.file(
"Cargo.toml",
r#"
[package]
name = "foo"
version = "0.1.0"
description = "foo"
license = "foo"
documentation = "foo"
include = ["src", "build"]
"#,
)
.file("src/lib.rs", "")
.file(".gitignore", "build")
});
// Example of adding a file that is confusingly ignored by an overzealous
// gitignore rule.

I guess the intention was making files being packaged and file tracked by Git in sync. Despite that, this “smart” behaivor does break tools like cargo-release and maybe cargo-smart-release as well.

@epage, @Byron, as you're the maintainers of those tools. Do you have any thought on this behavior? How to make cargo and releases tools inline with each others?

@weihanglo weihanglo added A-git Area: anything dealing with git Command-package S-needs-design Status: Needs someone to work further on the design for the feature or fix. NOT YET accepted. and removed S-triage Status: This issue is waiting on initial triage. labels Jun 22, 2023
@Byron
Copy link
Member

Byron commented Jun 22, 2023

The behaviour of cargo publish here is desired, even though it might lack refinement for this case: a file is ignored by git and not tracked by git yet - in this case it could probably just be added to the package anyway. Maybe there is a test for that as well and I didn't see it. Maybe this is something that could be improved when porting this area over to gitoxide, then I should have a better understanding of the tests as well and how it should work in detail.

Speaking only for cargo-smart-release, it doesn't try to re-implement any ignore logic but relies completely on cargo package - doing a git status check is merely for convenience, and for that it only relies on git itself without extra logic so I think it's fine in this case.

@scullionw
Copy link
Author

scullionw commented Jun 22, 2023

I see!

I guess the intention was making files being packaged and file tracked by Git in sync

I was under the impression (can't find the docs where I read this) that there was no guarantee that a cargo package matches a git repo 1-1. I feel that is also implied by the include/exclude?

For reference I am including some build artifacts for a couple targets that speed up compilation (the crate implements cxx bindings to a c++ library). The crate is not usable otherwise (10sec vs 20min compilation).. This is in a private registry.

I understand this is an edge case though.

In case someone else runs into this: another workaround I am using now is when publishing in CI, I make a dummy commit that I don't push just before the publish.

Anyways thanks for looking into this!

@weihanglo
Copy link
Member

#13592 is now merged. We have file listing support in gitoxide. I wonder if that is a step toward fixing this with gitoxide
cc @Byron

@Byron
Copy link
Member

Byron commented Mar 24, 2024

Would you have a rustup-based cargo invocation handy that invokes a dry-run publish with gitoxide for file listing? I would have left it here myself, but seem to have lost my old invocations. Thanks in advance.

@gaykitty
Copy link

I am experiencing this problem too, but the files are not in Cargo.toml's include. No matter what I do cargo thinks the files in .direnv are not commit despite that directory being ignored.

@Byron
Copy link
Member

Byron commented May 24, 2024

My guess would be that the .gitignore file contains something like this:

.direnv/

And it might work if it looks like this:

.direnv/
.direnv

Does that work for you?

@gaykitty
Copy link

No, cargo package --list still complains with either variation.

@korrat
Copy link

korrat commented Jun 12, 2024

I've just run into the same issue: cargo package complains about dirty files despite these being gitignored. As far as I can tell, all of these are READMEs for various Rust projects.

@Byron
Copy link
Member

Byron commented Jun 13, 2024

Would you be able to try a nightly cargo, for instance with cargo +nightly package -Z gitoxide? If I am not mistaken, this should already be triggered for listing files within a directory, and it would be expected it handles this case correctly.

@korrat
Copy link

korrat commented Jun 13, 2024

One more aspect, which may relate to the problem: My project is using a slightly particular setup based on nix, so toolchains and other Rust artifacts are installed in a .data directory inside the project tree.

With that said, here is the output of listing files with nightly cargo and -Z gitoxide:

$ rustup run nightly -- cargo package -Z gitoxide --list
error: 12 files in the working directory contain changes that were not yet committed into git:

.data/rustup/toolchains/nightly-x86_64-unknown-linux-gnu/share/doc/cargo/README.md
.data/rustup/toolchains/nightly-x86_64-unknown-linux-gnu/share/doc/clippy/README.md
.data/rustup/toolchains/nightly-x86_64-unknown-linux-gnu/share/doc/rust/README.md
.data/rustup/toolchains/nightly-x86_64-unknown-linux-gnu/share/doc/rustfmt/README.md
.data/cargo/lib/rustlib/src/rust/library/stdarch/README.md
.data/cargo/share/doc/cargo/README.md
.data/cargo/share/doc/clippy/README.md
.data/cargo/share/doc/rls/README.md
.data/cargo/share/doc/rust/README.md
.data/cargo/share/doc/rust-analyzer/README.md
.data/cargo/share/doc/rust-demangler/README.md
.data/cargo/share/doc/rustfmt/README.md

to proceed despite this and include the uncommitted changes, pass the `--allow-dirty` flag

The top 4 READMEs are new, after installing the nightly toolchain.

@korrat
Copy link

korrat commented Jun 13, 2024

Project link, if you want to check it out for yourself: https://gitlab.com/korrat/nullable-utils/

@weihanglo
Copy link
Member

Project link, if you want to check it out for yourself: gitlab.com/korrat/nullable-utils

The link is broken btw

@korrat
Copy link

korrat commented Jun 13, 2024

Thanks for the heads-up, repo was still private. Should be fixed now.

@Byron
Copy link
Member

Byron commented Jun 15, 2024

@korrat Can you show how the .gitignore file is ignoring .data? I assume that this .gitignore file might even be brought in by core.excludesFile. And maybe that key, core.excludesFile, is it mentioned in a conditional git-config file by any chance (using include.<condition>.path)?

I suspect that the configuration or .gitignore file isn't seen which is why it's not active. If there is a chance that this is the case, one could put .data/ in the .gitignore file directly at the root of the repository as a workaround.

@gaykitty
Copy link

I figured out the issue. My Cargo.toml had the line include = ["src/**/*", "LICENSE", "README.md"].
The .direnv directory contains files called LICENSE and README.md, which are the uncommitted files cargo was complaining about. Prefixing those file names with a / in Cargo.toml makes cargo happy.

@Byron
Copy link
Member

Byron commented Jun 16, 2024

That is interesting! I wasn't aware that writing (e.g.) README.md as include directive is equivalent to **/README.md. It's how .gitignore globs are working effectively, but for some reason in Cargo I expected standard globbing behavior. Maybe there is (even) more to this.

@korrat
Copy link

korrat commented Jun 16, 2024

Can you show how the .gitignore file is ignoring .data?

.gitignore simply contains a line .data. core.excludesFile is not set. After reading up on Cargo's manifest documentation, it seems that I misunderstood the include key as well. Not only is specifying include = ["README.md"] equal to include = ["**/README.md"], it also overrides the .gitignore exclusion behaviour.

@Byron
Copy link
Member

Byron commented Jun 16, 2024

I also got includes pretty wrong it seems :).

What would interest me is, without the /README.md modification that fixed it for you, is if ignoring .data/ (note the trailing slash) will fix the issue. The idea is that directory excludes are special, and they might prevent included files inside of the directory from ever being seen. It's just a guess, and I don't remember the exact rules anymore.

@korrat
Copy link

korrat commented Jun 16, 2024

I tried that out, but it didn't fix the problem. From reading the Cargo book, it seems that once include is specified, .gitignore is no longer relevant.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-git Area: anything dealing with git C-bug Category: bug Command-package S-needs-design Status: Needs someone to work further on the design for the feature or fix. NOT YET accepted.
Projects
None yet
Development

No branches or pull requests

5 participants