Improve performance of git status check in cargo package.

[ehuss]

The check for a dirty repository during packaging/publishing is quite slow. It was calling status_file for every packaged file, which is very expensive. I have a directory that had about 10,000 untracked files. Previously, cargo would hang for over 2 minutes without any output. With this PR, it finishes in 0.3 seconds.

The solution here is to collect the status information once, and then compare the package list against it.

One subtle point is that it does not use recurse_untracked_dirs, and instead relies on a primitive starts_with comparison, which I believe should be equivalent.

This still includes an inefficient n^2 algorithm, but I am too lazy to make a better approach.

I'm moderately confident this is pretty much the same as before (at least, all the scenarios I could think of).

[rust-highfive]

r? @Eh2406

(rust-highfive has picked a reviewer for you, use r? to override)

[alexcrichton]

@bors: r+

Seems like a nice win to me!

[bors]

📌 Commit a200640 has been approved by alexcrichton

[bors]

⌛ Testing commit a200640 with merge f99f965...

[bors]

☀️ Test successful - checks-actions
Approved by: alexcrichton
Pushing f99f965 to master...

[Byron]

Thanks for the performance improvement, I wasn't even aware :).

I'm moderately confident this is pretty much the same as before (at least, all the scenarios I could think of).

@ehuss, I think I have found a probably not too uncommon case where this isn't necessarily the case. The culprit, I believe, is this line…:

        status_opts
            .exclude_submodules(true)
                                    .include_ignored(true)  ←
            .include_untracked(true);

…which can cause the publish to fail because it's alarmed by untracked files which are ignored and wouldn't be published anyway.

As a concrete example, here is the publish attempt of a gitoxide crate:

[TRACE] Will run "cargo" "publish" "--manifest-path" "/Users/byron/dev/github.com/Byron/gitoxide/git-worktree/Cargo.toml" "--package" "git-worktree"
    Updating crates.io index
error: 2 files in the working directory contain changes that were not yet committed into git:

tests/fixtures/generated-do-not-edit/make_ignorecase_collisions/444887939-unix/link-to-X
tests/fixtures/generated-do-not-edit/make_ignorecase_collisions/444887939-unix/x

to proceed despite this and include the uncommitted changes, pass the `--allow-dirty` flag
Error: Could not successfully execute 'cargo publish'.

(note that tests/* is in git-worktree)

The top-level .gitignore file ignores the entire directory of generated files with **/generated-do-not-edit/ and the include directory in git-worktree/Cargo.toml does not include the tests either with include = ["src/**/*", "CHANGELOG.md"].

Now I wonder if a solution would be to pass all supposedly dirty files through the include/exclude filter coming from Cargo.toml?

[ehuss]

I can't reproduce the problem. If you'd like, I recommend opening an issue with a reproduction (minimal if possible). Ignored files that aren't published should be excluded from the intersection here. Perhaps it is possible there is some kind of path normalization issue or something.

[Byron]

Thanks for the swift reply!

If I read the exclusion check correctly, then src_files does indeed include the files in the test directory, like they are pre-include/exclude, which is why they match with what's considered dirty/untracked.

There is something peculiar about these paths though:

lrwxr-xr-x 1 byron staff 13 Jun 11:42 tests/fixtures/generated-do-not-edit/make_ignorecase_collisions/444887939-unix/link-to-X -> X
➜  git-worktree git:(main) l tests/fixtures/generated-do-not-edit/make_ignorecase_collisions/444887939-unix/x
lrwxr-xr-x 1 byron staff 13 Jun 11:42 tests/fixtures/generated-do-not-edit/make_ignorecase_collisions/444887939-unix/x -> X

So it refers to itself and does so in a case-insensitive manner. Maybe this is why these files have not been excluded in the first place. In any case, it's probably enough for me to cook up a test that reproduces the issue. Thanks for your help thus far!