fix: false-positive uncommitted changes for gitignored directories by unclepomedev · Pull Request #16864 · rust-lang/cargo

unclepomedev · 2026-04-09T11:05:23Z

Fixes #16547
(as patch before gix-dir)

What does this PR try to resolve?

Fixes false-positive 'uncommitted changes' errors during cargo publish caused by .gitignored directories (e.g., .venv) in workspace setups.

How to test and review this PR?

Copy of the test described in the issue, with the TODO and a typo corrected.
- (edited) also added the include = ... line that triggers this bug.
A false positive test for LICENSE files under .venv that are gitignored (I encountered and typical in Python+Rust env)

rustbot · 2026-04-09T11:05:32Z

r? @epage

rustbot has assigned @epage.
They will have a look at your PR within the next two weeks and either review your PR or reassign to another reviewer.

Use r? to explicitly pick a reviewer

Why was this reviewer chosen?

The reviewer was selected based on:

Owners of files modified in this PR: @ehuss, @epage, @weihanglo
@ehuss, @epage, @weihanglo expanded to ehuss, epage, weihanglo
Random selection from ehuss, epage, weihanglo

epage · 2026-04-09T13:39:31Z

FYI our contrib process encourages having tests in a commit before the fix, with every commit passing tests, see https://doc.crates.io/contrib/process/working-on-cargo.html#submitting-a-pull-request. I've written up more on this at https://epage.github.io/dev/pr-style/#c-test

epage · 2026-04-09T13:40:56Z

Note that #16547 was never labeled S-accepted which our contrib guide says is a pre-req for opening issues.

unclepomedev · 2026-04-09T15:25:18Z

@epage
My apologies for missing the S-accepted requirement.
I submitted this PR because it seemed like there hadn't been discussion for a while. However, if sufficient design discussion is needed, I may need to close this PR. Please let me know how you'd like to proceed.

unclepomedev · 2026-04-09T15:29:45Z

Also, apologies for overlooking your earlier comment about the contribution process.

This adds tests to reproduce the issue where `cargo publish` falsely flags `.gitignored` directories (like `.venv` or nested repositories) as uncommitted changes when the `include` option is used. The tests currently expect the erroneous behavior to establish a baseline before applying the actual fix.

When the `include` field is specified in `Cargo.toml`, Cargo's file listing logic previously ignored `.gitignore` rules for directories and traversed into nested git repositories. This caused `cargo package` to report uncommitted changes for files that were not intended to be part of the package. This change ensures that: - `.gitignore` is respected for directory traversal even when the `include` option is used. - Nested git repositories are explicitly skipped during file discovery. Fixes rust-lang#16547

unclepomedev · 2026-04-09T16:39:10Z

+                documentation = "foo"
+                homepage = "foo"
+                repository = "foo"
+                include = ["src/**/*", "tests/**/*"]


View changes since the review

Simply copying the test from the issue would result in false-negative (the test would pass even without modifying the code), so added an include statement, which is the trigger for the error.
(Without include, Cargo uses the standard ignore logic and the bug doesn't trigger.)

Doesn't the reproduction is still unclear? Should we discuss in the issue first to be aligned with the issue reporter? The two tests here might be two different issues.

unclepomedev · 2026-04-09T17:02:44Z

Updated the PR to follow the C-TEST guidelines. The changes are split into:

Adding tests that reproduce errors.
The fix and updating the tests to pass.

As commented, I found a flaw in the original reproduction.
Also, since the error output includes Git hashes, I ran:
SNAPSHOTS=overwrite cargo test -p cargo --test testsuite -- package::gitignored_nested_repo_not_treated_as_uncommitted --exact

weihanglo

The two reproducible test seem different than each others. And the fix include also ignoring .git and all files under a gitignore'd directory when package.include is specified.

These cases and behavior changes seem to be slightly different and should be discussed and clarified separately in the original issue or a new issue for new bug report.

View changes since this review

weihanglo · 2026-04-10T13:39:29Z

+    // Create a nested git repository in tests/fixtures/ that is gitignored
+    // This simulates test fixtures that are git repositories (like gix-dir test fixtures)
+    let repo_base = main_repo.root().join(
+        "tests/fixtures/generated-do-not-edit/many/613585535-unix/slash-in-root-and-negated/",


We probably want toavoid creating such a long path. It might exceed path length limit in some operating systems.

weihanglo · 2026-04-10T13:52:59Z

+                documentation = "foo"
+                homepage = "foo"
+                repository = "foo"
+                include = ["src/**/*", "tests/**/*"]


Doesn't the reproduction is still unclear? Should we discuss in the issue first to be aligned with the issue reporter? The two tests here might be two different issues.

weihanglo · 2026-04-10T13:55:15Z

                return false;
            }

+            // Skip nested git repositories (see rust-lang/cargo#16547).


This is also different than ust-lang/cargo#16547 that unclear to me why we want to ignore when not using include (I knew you added an include).

weihanglo · 2026-04-10T14:00:22Z

+                if let Some(ref gitignore) = gitignore_for_include {
+                    if gitignore
+                        .matched_path_or_any_parents(relative_path, is_dir)
+                        .is_ignore()
+                        && !ignore_include
+                            .matched_path_or_any_parents(relative_path, is_dir)
+                            .is_ignore()
+                    {
+                        return false;
+                    }
+                }


This a wrong fix, or at least a significant behavior-breaking change that we should consider more cases. For example, if a directory is gitignore'd but user request to package.include a specific file inside it, after this change the file won't be included.

rustbot · 2026-04-10T14:02:36Z

Reminder, once the PR becomes ready for a review, use @rustbot ready.

unclepomedev · 2026-04-10T15:24:19Z

Thank you for the thorough review.
I'd like to take the following steps before marking this ready again:

Check with the original issue reporter to clarify whether my reproduction covers the same case or a separate one.
If they turn out to be different issues, I'll open a new issue for the bug I encountered, with a proper reproduction.
I'll also re-examine the fix to make sure it doesn't break the case where include is intended to override .gitignore.

unclepomedev · 2026-04-11T06:32:10Z

@weihanglo

I've confirmed with the issue reporter that we're facing the same issue: a false-positive uncommitted error occurs during cargo publish when package.include is specified and the project contains a nested git repository.
I'd like to open a single reproduction issue covering both cases:

False positives for files inside a .git directory
False positives for .gitignored files

Would that be okay, or would you prefer them as separate issues?

weihanglo · 2026-04-11T12:43:39Z

Thanks for the investigation!

They may have different compatibility guarantee, so separate issues sounds better to manage the discussion.

unclepomedev · 2026-04-11T17:19:30Z

Thanks!

Closing in favor of #16872 and #16873 to split the issues as suggested.

rustbot added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Apr 9, 2026

rustbot assigned epage Apr 9, 2026

unclepomedev force-pushed the feat/#16547 branch from 2f9c9d6 to 80f8958 Compare April 9, 2026 11:10

unclepomedev added 2 commits April 10, 2026 01:00

unclepomedev force-pushed the feat/#16547 branch from 80f8958 to 65c8fc4 Compare April 9, 2026 16:20

unclepomedev commented Apr 9, 2026

View reviewed changes

weihanglo requested changes Apr 10, 2026

View reviewed changes

rustbot removed the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Apr 10, 2026

rustbot added the S-waiting-on-author Status: The marked PR is awaiting some action (such as code changes) from the PR author. label Apr 10, 2026

unclepomedev marked this pull request as draft April 10, 2026 15:24

unclepomedev mentioned this pull request Apr 10, 2026

cargo publish produces "X files in the working directory contain changes that were not yet committed into git" for .gitignored files #16547

Closed

This was referenced Apr 11, 2026

cargo publish: false-positive uncommitted error for .gitignored files when package.include is specified #16872

Closed

cargo publish: false-positive uncommitted error for .gitignored nested repositories when package.include is specified #16873

Closed

unclepomedev closed this Apr 11, 2026

unclepomedev deleted the feat/#16547 branch April 11, 2026 17:29

Conversation

unclepomedev commented Apr 9, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

What does this PR try to resolve?

How to test and review this PR?

Uh oh!

rustbot commented Apr 9, 2026

Uh oh!

epage commented Apr 9, 2026

Uh oh!

epage commented Apr 9, 2026

Uh oh!

unclepomedev commented Apr 9, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

unclepomedev commented Apr 9, 2026

Uh oh!

unclepomedev Apr 9, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

weihanglo Apr 10, 2026

Choose a reason for hiding this comment

Uh oh!

unclepomedev commented Apr 9, 2026

Uh oh!

weihanglo left a comment • edited by rustbot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

weihanglo Apr 10, 2026

Choose a reason for hiding this comment

Uh oh!

weihanglo Apr 10, 2026

Choose a reason for hiding this comment

Uh oh!

weihanglo Apr 10, 2026

Choose a reason for hiding this comment

Uh oh!

weihanglo Apr 10, 2026

Choose a reason for hiding this comment

Uh oh!

rustbot commented Apr 10, 2026

Uh oh!

unclepomedev commented Apr 10, 2026

Uh oh!

unclepomedev commented Apr 11, 2026

Uh oh!

weihanglo commented Apr 11, 2026

Uh oh!

unclepomedev commented Apr 11, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

unclepomedev commented Apr 9, 2026 •

edited

Loading

unclepomedev commented Apr 9, 2026 •

edited

Loading

unclepomedev Apr 9, 2026 •

edited

Loading

weihanglo left a comment •

edited by rustbot

Loading

unclepomedev commented Apr 11, 2026 •

edited

Loading