"brian m. carlson" [off-list ref] writes:
Yeah, this is because your operating system returns EINVAL in this case.
POSIX specifies EINVAL when you're trying to make a directory a
subdirectory of itself. Which, I mean, I guess is a valid
interpretation here, but it of course makes renaming the path needlessly
difficult.
...
I suspect part of the problem here is two fold: on macOS we can't
distinguish an attempt to rename the path due to it folding or
canonicalizing to the same thing from a real attempt to move an actual
directory into itself. The latter would be a problem we'd want to
report, and the former is not. Unfortunately, detecting this is
difficult because that means we'd have to implement the macOS
canonicalization algorithm in Git and we don't want to do that.
I agree we'd probably need to resort to macOS specific hack (like we
have NFS or Coda specific hacks), but it may not be too bad.
After seeing EINVAL, we can lstat src 'foo' and dst 'FOO', and
realize that both are directories and have the same st_dev/st_ino,
which should be fairly straightforward, no?
For that, we do not exactly have to depend on any part of macOS-ism;
we do depend on the traditional "within the same device, inum is a
good way to tell if two filesystem entities are the same".
The (mis)design of "git mv a b c d ... DST" that turns the request
into "mv a DST/a && mv b DST/b && ..." too early may make the
fallback behaviour a bit cumbersome to implement (mainly, we need to
strip out the '/foo' part out of the failing dst FOO/foo to get the
real destination directory 'FOO' the user gave us, before checking
with that lstat dance), but since it is an error codepath, we can be
as inefficient as we like, as long as we are correct ;-)
On 2021-05-04 at 03:46:12, Junio C Hamano wrote:
"brian m. carlson" [off-list ref] writes:
quoted
Yeah, this is because your operating system returns EINVAL in this case.
POSIX specifies EINVAL when you're trying to make a directory a
subdirectory of itself. Which, I mean, I guess is a valid
interpretation here, but it of course makes renaming the path needlessly
difficult.
...
I suspect part of the problem here is two fold: on macOS we can't
distinguish an attempt to rename the path due to it folding or
canonicalizing to the same thing from a real attempt to move an actual
directory into itself. The latter would be a problem we'd want to
report, and the former is not. Unfortunately, detecting this is
difficult because that means we'd have to implement the macOS
canonicalization algorithm in Git and we don't want to do that.
I agree we'd probably need to resort to macOS specific hack (like we
have NFS or Coda specific hacks), but it may not be too bad.
After seeing EINVAL, we can lstat src 'foo' and dst 'FOO', and
realize that both are directories and have the same st_dev/st_ino,
which should be fairly straightforward, no?
For that, we do not exactly have to depend on any part of macOS-ism;
we do depend on the traditional "within the same device, inum is a
good way to tell if two filesystem entities are the same".
Yes, although that won't work on Windows, which I don't believe has the
concept of inodes and almost certainly has the same problem. CCing
Dscho in case he has some ideas on how we can make this more resilient
there.
In any event, I'm not planning on writing a patch for this since I have
no way to test it, but I'm sure someone who uses macOS could probably
write one reasonably easily.
--
brian m. carlson (he/him or they/them)
Houston, Texas, US
Hi,
On Tue, 4 May 2021, brian m. carlson wrote:
On 2021-05-04 at 03:46:12, Junio C Hamano wrote:
quoted
"brian m. carlson" [off-list ref] writes:
quoted
Yeah, this is because your operating system returns EINVAL in this case.
POSIX specifies EINVAL when you're trying to make a directory a
subdirectory of itself. Which, I mean, I guess is a valid
interpretation here, but it of course makes renaming the path needlessly
difficult.
...
I suspect part of the problem here is two fold: on macOS we can't
distinguish an attempt to rename the path due to it folding or
canonicalizing to the same thing from a real attempt to move an actual
directory into itself. The latter would be a problem we'd want to
report, and the former is not. Unfortunately, detecting this is
difficult because that means we'd have to implement the macOS
canonicalization algorithm in Git and we don't want to do that.
I agree we'd probably need to resort to macOS specific hack (like we
have NFS or Coda specific hacks), but it may not be too bad.
After seeing EINVAL, we can lstat src 'foo' and dst 'FOO', and
realize that both are directories and have the same st_dev/st_ino,
which should be fairly straightforward, no?
For that, we do not exactly have to depend on any part of macOS-ism;
we do depend on the traditional "within the same device, inum is a
good way to tell if two filesystem entities are the same".
Yes, although that won't work on Windows, which I don't believe has the
concept of inodes and almost certainly has the same problem. CCing
Dscho in case he has some ideas on how we can make this more resilient
there.
Windows does not really have inodes. But it has what it calls "file IDs".
This concept is pretty much what you expect on inodes, except on that
still-used file system called FAT (for full details, see
https://docs.microsoft.com/en-us/windows/win32/api/fileapi/ns-fileapi-by_handle_file_information):
In the FAT file system, the file ID is generated from the first
cluster of the containing directory and the byte offset within the
directory of the entry for the file. Some defragmentation products
change this byte offset. (Windows in-box defragmentation does
not.) Thus, a FAT file ID can change over time. Renaming a file in
the FAT file system can also change the file ID, but only if the
new file name is longer than the old one.
In this instance, because we're not actually renaming (yet), the file ID
would probably be okay. But in general, we should assume that we do not
have inodes on Windows.
There is another complication: it is not actually cheap to get to that
file ID. For performance reasons, we introduced that (optional) FSCache
feature in Git for Windows where cache the `lstat()` results because the
assumption that `lstat()` is fast really only holds true on Linux.
In fact, what we do is to use `FindFirstFile()`/`FindNextFile()` to
enumerate an entire directory's worth of `lstat()` data, because funnily
enough, it is a lot faster when we need an entire directory's worth of
`lstat()` data anyway (calling `GetFileAttributes()` for individual files
is of course faster, but not for a dozen files or so).
But even `GetFileAttributes()` won't get you that "file ID". You have to
create a file handle (via `CreateFile()`, which is *expensive*) and then
call `GetFileInformationByHandle()`.
Now, way too much of Git's source code still pretends that `lstat()` is
just this fast operation and we can do it left and right and not say what
we _actually_ want to know. That function is called when we need only
parts of the `lstat()` data. Sometimes it is even used to determine
whether a file or directory is present. But since Git does not have proper
abstraction, `mingw_lstat()` _still_ has to fill in all the information.
So _if_ we need that file ID information, I would be very much in favor of
introducing a proper abstraction, where differentiate between the
intention (think `get_inode(const char *path)`) from the
platform-dependent implementation detail (think `lstat()`, `CreateFile()`
and `GetFileInformationByHandle()`).
Ciao,
Dscho
In any event, I'm not planning on writing a patch for this since I have
no way to test it, but I'm sure someone who uses macOS could probably
write one reasonably easily.
--
brian m. carlson (he/him or they/them)
Houston, Texas, US