gix clean panics if a tracked regular file is replaced with a FIFO

[EliahKagan]

Current behavior 😯

Starting in #1727, if a tracked file is deleted and replaced with a FIFO (named pipe) of the same name, gix clean panics:

$ rm README.md
$ mkfifo README.md
$ cargo run --bin=gix -- clean -n
    Finished `dev` profile [unoptimized + debuginfo] target(s) in 0.16s
     Running `target/debug/gix clean -n`
thread 'main' panicked at gitoxide-core/src/repository/clean.rs:168:49:
present if not pruned
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace

This panic is from the expect() call at:

gitoxide/gitoxide-core/src/repository/clean.rs

Line 168 in 69ee6a3

let mut disk_kind = entry.disk_kind.expect("present if not pruned");

That line is unmodified in #1727, but the logic of this immediately preceding code was somewhat changed:

gitoxide/gitoxide-core/src/repository/clean.rs

Lines 161 to 167 in 69ee6a3

	if entry.disk_kind.is_none() {
	entry.disk_kind = workdir
	.join(gix::path::from_bstr(entry.rela_path.as_bstr()))
	.metadata()
	.ok()
	.and_then(|e| gix::dir::entry::Kind::try_from_file_type(e.file_type()));
	}

Possibly related is the changed behavior of gix status in this situation:

$ cargo run --bin=gix -- status
   Compiling gitoxide v0.39.0 (/home/ek/repos/gitoxide)
    Finished `dev` profile [unoptimized + debuginfo] target(s) in 6.31s
     Running `target/debug/gix status`
 23:42:00 status done 2.3K files in 0.02s (117.9K files/s)
  T README.md
  ? README.md

It lists two status items for README.md: a typechange and an untracked file. This is not necessarily wrong, but it is unusual because a new FIFO (not with a name tracked as something else) does not show in gix status at all since #1727. This suggests that the presence of same-named file in the index causes the FIFO to be treated as an untracked file.

Expected behavior 🤔

gix clean should tolerate untracked typechanges to unsupported kinds of filesystem entries like FIFOs, much as it tolerates untracked typechanges to supported kinds of entries. It should do some predictable thing, and not panic.
 
The goal since #1727 is, whenever possible, to disregard and avoid operating on entries other than regular files, directories, and symlinks. So, more specifically than not panicking, I think gix clean should disregard the untracked typechange. This would also match the Git behavior, as described below.

(As for the gix status behavior of showing two lines, I am not sure what should be done.)

Git behavior

In the state produced by running rm README.md and mkfifo README.md, I also ran git clean -n. This produced no output, fully disregarding the FIFO. This makes sense, since there is no entry at any new (nor ignored) path in the working tree.

The exact way git currently seems to achieve this, though, may not be ideal and should possibly not be seen as a goal. The output of git status --short was:

 M README.md

The output of git status was:

On branch main
Your branch is up to date with 'origin/main'.

Changes not staged for commit:
  (use "git add <file>..." to update what will be committed)
  (use "git restore <file>..." to discard changes in working directory)
        typechange: README.md

no changes added to commit (use "git add" and/or "git commit -a")

Really there has been an untracked typechange to an unsupported kind of filesystem entry. But git has reported it as a modification, including the suggestion to stage the change, which of course cannot succeed:

$ git add README.md
error: README.md: can only add regular files, symbolic links or git-directories
fatal: updating files failed

It seems to me better to report this as just a typechange (as gix status did before #1727, though the rest of the old behavior would not be preferable) or as some new category of untracked modification for an unsupported typechange. But in and of itself, the current behavior of gix status is maybe not so bad, and probably preferable to the behavior of git itself.

Steps to reproduce 🕹

I used my clone of gitoxide on Arch Linux. A simpler repository would be fine and perhaps better, but doing it this way made it easy to switch between the installed gix command by running gix ... and the new gix command (incorporating #1727) by running cargo run --bin=gix -- ....

Although I checked a few other things, as described explicitly above, the important commands, for checking old and new gitoxide behavior here as well as corresponding Git behavior, were as follows (where a symlink is also tested to verify that the bug is specific to typechanges to unsupported kinds of entries):

rm README.md
ln -s nonexistent README.md
git status
gix status
cargo run --bin=gix -- status
git clean -n
gix clean -n
cargo run --bin=gix -- clean -n
rm README.md
mkfifo README.md
git status
gix status
cargo run --bin=gix -- status
git clean -n
gix clean -n
cargo run --bin=gix -- clean -n

The most important output is shown above in the relevant sections, with prompts replaced with $. Full output (and prompts and the commands themselves), including a cargo run --bin=gix -- clean -n run including a stack trace, was:

ek in 🌐 catenary in gitoxide on  main is 📦 v0.39.0 via 🦀 v1.83.0
❯ rm README.md

ek in 🌐 catenary in gitoxide on  main [✘] is 📦 v0.39.0 via 🦀 v1.83.0
❯ ln -s nonexistent README.md

ek in 🌐 catenary in gitoxide on  main is 📦 v0.39.0 via 🦀 v1.83.0
❯ git status
On branch main
Your branch is up to date with 'origin/main'.

Changes not staged for commit:
  (use "git add <file>..." to update what will be committed)
  (use "git restore <file>..." to discard changes in working directory)
        typechange: README.md

no changes added to commit (use "git add" and/or "git commit -a")

ek in 🌐 catenary in gitoxide on  main is 📦 v0.39.0 via 🦀 v1.83.0
❯ gix status
 23:39:47 status done 2.3K files in 0.02s (143.1K files/s)
  T README.md

head -> index isn't implemented yet

ek in 🌐 catenary in gitoxide on  main is 📦 v0.39.0 via 🦀 v1.83.0
❯ cargo run --bin=gix -- status
    Finished `dev` profile [unoptimized + debuginfo] target(s) in 0.19s
     Running `target/debug/gix status`
 23:39:52 status done 2.3K files in 0.02s (118.2K files/s)
  T README.md

head -> index isn't implemented yet

ek in 🌐 catenary in gitoxide on  main is 📦 v0.39.0 via 🦀 v1.83.0
❯ git clean -n

ek in 🌐 catenary in gitoxide on  main is 📦 v0.39.0 via 🦀 v1.83.0
❯ gix clean -n
Nothing to clean (Skipped 1 expendable entry - show with -x)

ek in 🌐 catenary in gitoxide on  main is 📦 v0.39.0 via 🦀 v1.83.0
❯ cargo run --bin=gix -- clean -n
    Finished `dev` profile [unoptimized + debuginfo] target(s) in 0.17s
     Running `target/debug/gix clean -n`
Nothing to clean (Skipped 1 expendable entry - show with -x)

ek in 🌐 catenary in gitoxide on  main is 📦 v0.39.0 via 🦀 v1.83.0
❯ rm README.md

ek in 🌐 catenary in gitoxide on  main [✘] is 📦 v0.39.0 via 🦀 v1.83.0
❯ mkfifo README.md

ek in 🌐 catenary in gitoxide on  main [!] is 📦 v0.39.0 via 🦀 v1.83.0
❯ git status
On branch main
Your branch is up to date with 'origin/main'.

Changes not staged for commit:
  (use "git add <file>..." to update what will be committed)
  (use "git restore <file>..." to discard changes in working directory)
        modified:   README.md

no changes added to commit (use "git add" and/or "git commit -a")

ek in 🌐 catenary in gitoxide on  main [!] is 📦 v0.39.0 via 🦀 v1.83.0
❯ gix status
 23:41:37 status done 2.3K files in 0.02s (151.7K files/s)
  T README.md

head -> index isn't implemented yet

ek in 🌐 catenary in gitoxide on  main [!] is 📦 v0.39.0 via 🦀 v1.83.0
❯ cargo run --bin=gix -- status
   Compiling gitoxide v0.39.0 (/home/ek/repos/gitoxide)
    Finished `dev` profile [unoptimized + debuginfo] target(s) in 6.31s
     Running `target/debug/gix status`
 23:42:00 status done 2.3K files in 0.02s (117.9K files/s)
  T README.md
  ? README.md

head -> index isn't implemented yet

ek in 🌐 catenary in gitoxide on  main [!] is 📦 v0.39.0 via 🦀 v1.83.0 took 6s
❯ git clean -n

ek in 🌐 catenary in gitoxide on  main [!] is 📦 v0.39.0 via 🦀 v1.83.0
❯ gix clean -n
Nothing to clean (Skipped 1 expendable entry - show with -x)

ek in 🌐 catenary in gitoxide on  main [!] is 📦 v0.39.0 via 🦀 v1.83.0
❯ cargo run --bin=gix -- clean -n
    Finished `dev` profile [unoptimized + debuginfo] target(s) in 0.16s
     Running `target/debug/gix clean -n`
thread 'main' panicked at gitoxide-core/src/repository/clean.rs:168:49:
present if not pruned
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace

ek in 🌐 catenary in gitoxide on  main [!] is 📦 v0.39.0 via 🦀 v1.83.0
❯ RUST_BACKTRACE=1 cargo run --bin=gix -- clean -n
    Finished `dev` profile [unoptimized + debuginfo] target(s) in 0.17s
     Running `target/debug/gix clean -n`
thread 'main' panicked at gitoxide-core/src/repository/clean.rs:168:49:
present if not pruned
stack backtrace:
   0: rust_begin_unwind
             at /rustc/90b35a6239c3d8bdabc530a6a0816f7ff89a0aaf/library/std/src/panicking.rs:665:5
   1: core::panicking::panic_fmt
             at /rustc/90b35a6239c3d8bdabc530a6a0816f7ff89a0aaf/library/core/src/panicking.rs:74:14
   2: core::panicking::panic_display
             at /rustc/90b35a6239c3d8bdabc530a6a0816f7ff89a0aaf/library/core/src/panicking.rs:264:5
   3: core::option::expect_failed
             at /rustc/90b35a6239c3d8bdabc530a6a0816f7ff89a0aaf/library/core/src/option.rs:2021:5
   4: core::option::Option<T>::expect
             at /home/ek/.rustup/toolchains/stable-x86_64-unknown-linux-gnu/lib/rustlib/src/rust/library/core/src/option.rs:933:21
   5: gitoxide_core::repository::clean::function::clean
             at ./gitoxide-core/src/repository/clean.rs:168:33
   6: gitoxide::plumbing::main::main::{{closure}}
             at ./src/plumbing/main.rs:322:17
   7: gitoxide::shared::pretty::prepare_and_run
             at ./src/shared.rs:156:17
   8: gitoxide::plumbing::main::main
             at ./src/plumbing/main.rs:314:15
   9: gix::main
             at ./src/gix.rs:5:5
  10: core::ops::function::FnOnce::call_once
             at /home/ek/.rustup/toolchains/stable-x86_64-unknown-linux-gnu/lib/rustlib/src/rust/library/core/src/ops/function.rs:250:5
note: Some details are omitted, run with `RUST_BACKTRACE=full` for a verbose backtrace.

In case it is somehow convenient, that is also shown in this gist.

I have not attempted any testing related to this specific issue on Windows. At least as described, this issue is specific to Unix-like systems, due to the special considerations related to named pipes on Windows described in #1727 (review).

[Byron]

This is a great finding!

I am already working on a fix that should also correct gix status.