{"thread":{"id":"31211","subject":"Git does not handle changing inode numbers well","startedAt":"2012-08-08T15:22:30Z","lastAt":"2012-08-08T18:34:11Z","messageCount":4,"participants":["Matthijs Kooijman","Junio C Hamano"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"196670","messageId":"20120808152230.GQ21274@login.drsnuggles.stderr.nl","threadId":"31211","inReplyTo":null,"subject":"Git does not handle changing inode numbers well","fromName":"Matthijs Kooijman","fromEmail":"matthijs@stdin.nl","sentAt":"2012-08-08T15:22:30Z","receivedAt":"2012-08-08T15:22:30Z","isPatch":false,"sender":{"key":"matthijs@stdin.nl","avatar":"https://avatars.githubusercontent.com/u/194491?v=4"},"body":"(Please CC me, I'm not on the list)\n\nHi folks,\n\nI've spent some time debugging an issue and I'd like to share the\nresults. The conclusion of my debugging is that git does not currently\nhandle changing inode numbers on files well.\n\nI have a custom Fuse filesystem, and fuse dynamically allocates inode\nnumbers to paths, but keeps a limited cache of inode -> name mappings,\ncausing the inodes to change over time.\n\nNow of course, you'll probably say, \"it's the filesystem's fault, git\ncan't be expected to cope with that\". You'll be right of course, but\nsince I already spent the time digging into this and figuring out what\ngoes on inside git in this case, I thought I might as well share the\nanalysis, just in case someone sees an easy fix in here, or in case\nsomeone else stumbles upon this problem as well.\n\nSo, the actual problem I was seeing is that running \"git status\" showed\nall symlinks as \"modified\", even though they really were identical\nbetween the working copy, index and HEAD. Interestingly enough this only\nhappened when running \"git status\" without further arguments, when\nrunning on a subdirectory, it would show no changes as expected.\n\nI compared the output of stat to a hexdump of the index file and found\nthat everything matched, except for the inode numbers. I originally\nthought I was misinterpreting what I saw, but gdb confirmed that it were\nindeed the inode numbers that git observed as different.\n\nNow, I could have stopped here and started trying to fix my filesystem\ninstead. But it was still weird that this problem only existed for\nsymlinks and that normal files acted as expected. So I dug in a bit\ndeeper, hoping to find some way to make this work for symlinks as well.\n\nSo, here's what happens (IIUC):\n - cmd_status calls refresh_index, which calls refresh_cache_ent for\n   every entry in the index.\n - refresh_cache_ent notices that the inode number has changed (for both\n   symlinks and regular files) and compares the file / symlink contents.\n - refresh_cache_ent sees the content hasn't changed, so it calls\n   fill_stat_cache_info to update the stat info.\n - fill_stat_cache_info sets the EC_UPTODATE flag on the entry, but only\n   if it is a regular file.\n - cmd_status calls wt_status_collect which calls\n   wt_status_collect_changes_worktree which calls run_diff_files.\n - run_diff_files skips regular files, because of the EC_UPTODATE flag.\n   For symlinks, however, it checks the stat info and notices that the\n   inode number has changed (again). It does not do a content check at\n   this point, but instead just outputs the file as \"modified\".\n\n\nIt turned out that the reason running \"git status\" on a subdirectory did\nappear to work, was that the number of files in the subdir wasn't big\nenough to overflow the inode number cache fuse keeps, so that numbers\ndidn't change in this case (the problem _did_ occur when trying a bigger\nsubdirectory).\n\nSo, it seems that git just doesn't cope well with changing inode numbers\nbecause it checks the content in a first pass in refresh_index, but only\nchecks the stat info in the second pass in run_diff_files. The reason it\ndoes work for regular files is EC_UPTODATE optimization introduced in\neadb5831: Avoid running lstat(2) on the same cache entry.\n\nSo, let's see if I can fix my filesystem now ;-)\n\nGr.\n\nMatthijs\n"},{"id":"196679","messageId":"7vboiltglr.fsf@alter.siamese.dyndns.org","threadId":"31211","inReplyTo":"20120808152230.GQ21274@login.drsnuggles.stderr.nl","subject":"Re: Git does not handle changing inode numbers well","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2012-08-08T17:53:52Z","receivedAt":"2012-08-08T17:53:52Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Matthijs Kooijman <matthijs@stdin.nl> writes:\n\n> So, it seems that git just doesn't cope well with changing inode numbers\n> because it checks the content in a first pass in refresh_index, but only\n> checks the stat info in the second pass in run_diff_files. The reason it\n> does work for regular files is EC_UPTODATE optimization introduced in\n> eadb5831: Avoid running lstat(2) on the same cache entry.\n>\n> So, let's see if I can fix my filesystem now ;-)\n\nTrue.  We have knobs to cope with filesystems whose st_dev or\nst_ctime are not stable, but there is no such knob to tweak for\nst_ino.  Shouldn't be too hard to add such, though.  One approach is\nto do something like the attached patch, and declare, define,\ninitialize, and set trust_inum in a way similar to how we handle\ntrust_ctime in the existing code.\n\n read-cache.c | 2 +-\n 1 file changed, 1 insertion(+), 1 deletion(-)\n\ndiff --git a/read-cache.c b/read-cache.c\nindex 2f8159f..6da99af 100644\n--- a/read-cache.c\n+++ b/read-cache.c\n@@ -210,7 +210,7 @@ static int ce_match_stat_basic(struct cache_entry *ce, struct stat *st)\n \tif (ce->ce_uid != (unsigned int) st->st_uid ||\n \t    ce->ce_gid != (unsigned int) st->st_gid)\n \t\tchanged |= OWNER_CHANGED;\n-\tif (ce->ce_ino != (unsigned int) st->st_ino)\n+\tif (trust_inum && ce->ce_ino != (unsigned int) st->st_ino)\n \t\tchanged |= INODE_CHANGED;\n \n #ifdef USE_STDEV\n"},{"id":"196681","messageId":"20120808180748.GS21274@login.drsnuggles.stderr.nl","threadId":"31211","inReplyTo":"20120808152230.GQ21274@login.drsnuggles.stderr.nl","subject":"Re: Git does not handle changing inode numbers well","fromName":"Matthijs Kooijman","fromEmail":"matthijs@stdin.nl","sentAt":"2012-08-08T18:07:48Z","receivedAt":"2012-08-08T18:07:48Z","isPatch":false,"sender":{"key":"matthijs@stdin.nl","avatar":"https://avatars.githubusercontent.com/u/194491?v=4"},"body":"> So, let's see if I can fix my filesystem now ;-)\nFor anyone interested: turns out passing -o noforget makes fuse keep a\npersistent path -> inode mapping (at the cost of memory usage, of\ncourse).\n\nHowever, it also turns out that fuse wasn't my problem: It was the aufs\nmount that was overlayed over my fuse mount (this was on a Debian live\nsystem), which sets the noxino option that prevents aufs from keeping\npersistent inode numbers.\n\nTo get git status working as expected, I had to both remove noxino from\nthe aufs mount and add noforget to the underlying fuse mount.\n\nGr.\n\nMatthijs\n"},{"id":"196685","messageId":"20120808183411.GT21274@login.drsnuggles.stderr.nl","threadId":"31211","inReplyTo":"7vboiltglr.fsf@alter.siamese.dyndns.org","subject":"Re: Git does not handle changing inode numbers well","fromName":"Matthijs Kooijman","fromEmail":"matthijs@stdin.nl","sentAt":"2012-08-08T18:34:11Z","receivedAt":"2012-08-08T18:34:11Z","isPatch":false,"sender":{"key":"matthijs@stdin.nl","avatar":"https://avatars.githubusercontent.com/u/194491?v=4"},"body":"Hi Junio,\n\n> -\tif (ce->ce_ino != (unsigned int) st->st_ino)\n> +\tif (trust_inum && ce->ce_ino != (unsigned int) st->st_ino)\n>  \t\tchanged |= INODE_CHANGED;\n\nI just tried this with 1.7.10 (that is, I deleted these two lines to\nmimic trust_inum being false) and it indeed fixes my problem.\n\n(I'll probably won't be implementing the full patch, though, I've\nalready figured out how to fix my filesystem instead)\n\nGr.\n\nMatthijs\n"}]}