Hi, On Tue, 4 May 2021, brian m. carlson wrote: > On 2021-05-04 at 03:46:12, Junio C Hamano wrote: > > "brian m. carlson" <sandals@xxxxxxxxxxxxxxxxxxxx> 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". > > 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 >
