r? @ehuss

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

The Windows failure here is surprising. It suggests that on Windows, Cargo considers the symlinks files, not directories, and thus they don't match /symlink/. Probably around here

Even though the docs for Path::is_dir pretty clearly state

This function will traverse symbolic links to query information about the destination file.

🤔

Given that I don't have a Windows computer handy to debug with, I think this has me stomped for now.

I'll just add that the fix I had originally intended for #10032 is

        let is_dir = path.is_dir();

        // Symlinks that point to directories are a little odd because they are both directories
        // _and_ files for the purposes of matching. At least as far as Cargo is concerned, since
        // we never include symlinks directly in the pacakge. The actual rules are:
        //
        //  1. A pattern of `/symlink` should match the symlink itself.
        //  2. A pattern of `/symlink/` should match the symlink's target's contents.
        //
        // But for us, this means that if a directory symlink is excluded as either a directory
        // _or_ a file, it should be skipped.
        if !is_root && is_dir {
            let is_symlink = path.symlink_metadata()?.file_type().is_symlink();
            // See if the symlink is filtered out when viewed as a file -- if so, skip it.
            // The directory case is handled by the regular logic below.
            if is_symlink && !(*filter)(path, false)? {
                return Ok(());
            }
        }

        if !is_root && !(*filter)(path, is_dir)? {

which may come in handy for whoever decides to try tackling this.

I believe the issue here is with libgit2. When creating the repository, the add_all function ends up adding the symbolic links to the repo. There is something about libgit2's gitignore implementation that is not honoring symlinks as directories. I have opened libgit2/libgit2#6250 with a reproduction.

If you want to just disable these tests on Windows, I think it would be fine.

Pushed a change that ignores the test on Windows 👍

Thanks!

@bors r+

📌 Commit 79cc65f has been approved by ehuss

⌛ Testing commit 79cc65f with merge e458f6d...

☀️ Test successful - checks-actions
Approved by: ehuss
Pushing e458f6d to master...