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

git status IO error when directory is replaced with non-directory #1735

Closed
EliahKagan opened this issue Dec 22, 2024 · 1 comment · Fixed by #1736
Closed

git status IO error when directory is replaced with non-directory #1735

EliahKagan opened this issue Dec 22, 2024 · 1 comment · Fixed by #1736
Assignees

Comments

@EliahKagan
Copy link
Member

EliahKagan commented Dec 22, 2024

Current behavior 😯

When a directory is removed and replaced with a non-directory, such as a regular file, with the same name, running gix status begins to output information about the changed entries, but then fails with an error:

ek in 🌐 catenary in gitoxide on  main is 📦 v0.39.0 via 🦀 v1.83.0
❯ rm -r examples

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

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/rm <file>..." to update what will be committed)
  (use "git restore <file>..." to discard changes in working directory)
        deleted:    examples/log.rs
        deleted:    examples/ls-tree.rs

Untracked files:
  (use "git add <file>..." to include in what will be committed)
        examples

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
  ? examples
Error: IO error while writing blob or reading file metadata or changing filetype

Caused by:
    Not a directory (os error 20)

This happens with other non-directories, including items that are not tracked, or at least including FIFOs (named pipes). However, the presentation is the same, as shown in this gist, and nothing about this appears to be specific to the handling of "non-files" nor to any changes related to that in #1727 or #1730. Although I discovered this while testing for possible regressions related to #1730, as far as I can tell, this behavior is long-standing and not conceptually related to the changes there.

Expected behavior 🤔

I think this condition should not be treated as an error and probably is not intended to be treated as one. Instead, something reasonable should be reported. As shown above, git status reports the deletion of the files under the directory:

Changes not staged for commit:
  (use "git add/rm <file>..." to update what will be committed)
  (use "git restore <file>..." to discard changes in working directory)
        deleted:    examples/log.rs
        deleted:    examples/ls-tree.rs

That, or a type change, or perhaps both--for nonempty directories where at least one entry somewhere under the directory was a blob or submodule--would make sense.

In the opposite scenario, presented in this second gist where a non-directory is replaced with a directory, gix status shows both changes, which seems like a good approach to me:

 09:13:44 status done 2.3K files in 0.02s (117.5K files/s)
  D general-tasks.md
  ? general-tasks.md/

This makes me think that maybe the best behavior when a directory is replaced with a non-directory is also to show both the deletions and the new file. The other reason I mention this behavior is in the hope that it can be preserved (in case changes to address this bug might threaten it).

A third case, further removed but still conceptually related, is when files' executable permissions are changed. This is shown in this third gist. git status shows these as modifications, while gix status shows X. I mention this to illustrate that that there are several ways, related to changes to entries that are not modifications of file contents, where gix status seems to be a bit nicer than git status. (Of course, I recognize that git status is limited by the need for compatibility, as well as being more complete than gix status is currently; my intent is not to levy an overall judgment.) I mention this mainly as an argument that the Git behavior may not be deciding here, and secondarily because this is another behavior that I hope can be preserved in whatever may change to fix this bug.

Git behavior

In this case, the expected and Git behaviors--even though it seems to me that they are not the same--were conceptually entwined, so I covered both above.

Steps to reproduce 🕹

See the commands and their output in the "Current behavior" section above.

The commands used there, without the output, are, for convenience:

rm -r examples
touch examples
git status
gix status
cargo run --bin=gix -- status

The last two produced the IO error. I ran this in my clone of gitoxide. Running gix ran the installed published version, while cargo run --bin=gix ran something near the tip of main (the last couple of PRs did not, I don't believe, change anything relevant to this issue). I did this on Arch Linux.

I did an analogous sub-experiment with a named pipe. I don't think this is separate but here are the commands for that:

rm -r examples
git status
gix status
cargo run --bin=gix -- status
mkfifo examples
git status
gix status
cargo run --bin=gix -- status

The extra runs of git status and gix status before running mkfifo were to verify that things were working when it is merely deleted. (This covers both sub-experiments since it is before they diverge.)

As noted and linked above, both sub-experiments are covered in this first gist.

@Byron
Copy link
Member

Byron commented Dec 22, 2024

That's a great catch, thanks for reporting and analysis!

And I agree, the fix shouldn't change any other behaviour, but only fix this particular issue. I think this will naturally be the case as it tries to check the entries that are present in the index, but deleted on disk, and fails to handle that a file is in the way.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants