-
-
Notifications
You must be signed in to change notification settings - Fork 346
fix!: consider non-files as pruned officially. #1730
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
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,3 +1,4 @@ | ||
status_unchanged.tar | ||
status_changed.tar | ||
symlink_stack.tar | ||
status_nonfile.tar | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Could generated archives be committed for this? I think all tests using fixtures this covers are run only only on Unix-like systems (so the Windows differences discussed in #1727 (review) would not be a problem). With the (A test of the There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Interestingly, I ran into this locally and on MacOS it definitely didn't properly retain the Since the script doesn't do much, I think it's also fine to not have it cached. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. If a commit was pushed, that might still be accessible, that has the |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,19 @@ | ||
#!/usr/bin/env bash | ||
set -eu -o pipefail | ||
|
||
git init -q untracked | ||
(cd untracked | ||
touch file && git add file && git commit -m "just to get an index for the test-suite" | ||
|
||
mkfifo pipe | ||
git status | ||
) | ||
|
||
git init -q tracked-swapped | ||
(cd tracked-swapped | ||
touch file && git add file && git commit -m "it starts out as trackable file" | ||
|
||
rm file && mkfifo file | ||
git status | ||
) | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not sure what the best thing is to call filesystem entries of kinds that are not representable in Git repositories. In my experience, usually "file" is either construed broadly to include such entries well as directories and symbolic links, or construed narrowly to mean "regular file" and thus exclude directories and symbolic links.
I think it is unintuitive for regular files, directories, and symbolic links all not to be
NonFile
s, at the same time as FIFOs, sockets, character and block devices, and other more exotic kinds of filesystem entries areNonFile
s (though at least one such kind of entry, a whiteout, is very intuitively a non-file).I don't know if it's possible to name
NonFile
better--which is why I'm saying this instead of opening a PR to rename it. The documentation comment can probably be expanded to clarify fully whatNonFile
is everything other than, as well as to list the other common examples ofNonFile
s, and I do plan to open a PR for that. It may be that documentation in other places can be usefully expanded too; I'll look into that.This would not be the first technical usage of "non-" with a non-literal meaning. For example, in
git commit -am message
, onlycommit
is said to be a "non-option argument," even though the operandmessage
is unambiguously an argument that is not an option argument. Sometimes this is hard to avoid, or maybe not worth avoiding.However, this does seem to be an area where confusion is possible, or at least possible for me. For example, when I read the description and commit message in #1629, which quoted from a comment in an early version of Git a few weeks before Git began to track symbolic links, I took "non-regular files" to encompass symbolic links and was worried that the code there might wrongly remove support for them.
In fact, the code there deliberately preserved support for them (as did #1727, which also used that phrase). But the same version of the comment that Git developers used early on to describe a situation that encompassed the absence of symlink support was taken to encompass the presence of symlink support when cited.
Another possible source of confusion is that some kinds of
NonFile
s are distinguished from related entities by being files. For example, a FIFO (named pipe) differs from an anonymous pipe by being a file.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for taking the time to make this point of ambiguity so clear.
I'd definitely welcome a PR which changes the variant name and/or improves the documentation - right now it's definitely very brief.
Other
also comes to mind as catch-all for everything that isn't the other variants. Technically that's a good fit as the logic literally does it like this.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I've opened #1734 for this. For now, I have not attempted to rename
NonFile
.