{"thread":{"id":"47394","subject":"Unfortunate interaction between git diff-index and fakeroot","startedAt":"2017-12-07T09:04:12Z","lastAt":"2017-12-08T15:39:01Z","messageCount":4,"participants":["Elazar Leibovich","Junio C Hamano"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"334311","messageId":"0190ae30-b6c8-2a8b-b1fb-fd9d84e6dfdf@oracle.com","threadId":"47394","inReplyTo":null,"subject":"Unfortunate interaction between git diff-index and fakeroot","fromName":"Elazar Leibovich","fromEmail":"elazar.leibovich@oracle.com","sentAt":"2017-12-07T09:03:57Z","receivedAt":"2017-12-07T09:04:12Z","isPatch":false,"sender":{"key":"elazar.leibovich@oracle.com","avatar":null},"body":"Hi,\n\nWe noticed some unexpected behavior of git, when running git commands under\nfakeroot, and then running another command without fakeroot.\n\nWhen running, e.g., git status, or git describe --dirty, git can\nupdate the index file.\n\nThis can happen inadvertently by, e.g., running the make install process of\na package under fakeroot, and running \"git describe --dirty\", or \"git\nstatus\" to generate package name.\n\nThe unexpected result is:\n\n\"fakeroot git status\" updates the index, and the index now says all files\nare owned by uid:0.\n\n\"git diff-index --name-only HEAD\" is used to test if the git tree is dirty\nwithout fakeroot, concluding all files have changed, since their owner UID\nis changed.\n\nYou can easily recreate it:\n\n     $ mkdir gitexample;cd gitexample;git init\n     Initialized empty Git repository in /home/elazar/gitexample/.git/\n     $ touch x; git add x; git commit -m init\n      1 file changed, 0 insertions(+), 0 deletions(-)\n      create mode 100644 x\n     $ git diff-index HEAD # nothing is printed, since working dir clean\n     $ fakeroot git diff # index is updated to contain uid:0\n     $ git ls-files -s --debug\n     100644 e69de29bb2d1d6434b8b29ae775ad8c2e48c5391 0       x\n       ctime: 1512636533:915178157\n       mtime: 1512636533:915178157\n       dev: 64515    ino: 8523907\n       uid: 0        gid: 0\n       size: 0       flags: 0\n     $ git diff-index --name-only HEAD # x is considered different now\n     x\n\nMake sure you do not have a shell prompt that calls git status in\nbetween, like oh-my-zsh, or bash-it.\n\nThis is problematic because:\n\n1. File owner is not really tracked by git porcelain, and I'm not sure\n    what's the benefit of tracking ownership of files in git diff-index. \nUsers\n    who expect git diff to return \"no changes\" after ownership changed, \nexpect\n    git diff-index to return \"no changes\" as well.\n\n   This is supported by the manual page who say \"Compares the content \nand mode\n   of the blobs found in a tree object with the corresponding tracked \nfiles in\n   the working tree\", and doesn't mention file ownership.\n\n2. Linux tree uses git diff-index to test if the working tree is dirty. It\n    can, and has cause subtle issues when building and packaging different\n    parts of the tree to different directories concurrently.\n\nOur proposed solutions:\n\nMake match_stat_data ignore ownership changes.\n\nI searched git source for usage of OWNER_CHANGED bit and found that no one\ntests for it. I can't think of a scenario where the fact that the owner have\nchanged would matter.\n\nI searched for usage of git-diff-index in git tree, and found that\ngit-stash uses it. This is not relevant since stash would ignore ownership\nchanges. I saw that merge-octupus and filter-branch uses git-diff-index\nbut they also don't seem to care if ownership has changed.\n\nI searched for the first commit who tracked file ownership in the index,\nit was there from the first commit e83c5163316f89bfbde7d9ab23ca2e25604af290\nI found no specific reason given for tracking the owner in related commits.\n\nYou can see a change in a similar spirit to what I propose in commit\n2cb45e95438c113871fbbea5b4f629f9463034e7 where st_dev is ignored by default\ndue to similar problem in NFS.\n\nWhat do you think?\n\nShould the behavior remain, and diff-index should say it tracks \"mode and\nowner changes\"?\nWould it be OK to change the actual behavior of diff-index to ignore owner\nchanges?\n\n"},{"id":"334324","messageId":"xmqqzi6ueern.fsf@gitster.mtv.corp.google.com","threadId":"47394","inReplyTo":"0190ae30-b6c8-2a8b-b1fb-fd9d84e6dfdf@oracle.com","subject":"Re: Unfortunate interaction between git diff-index and fakeroot","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2017-12-07T15:31:08Z","receivedAt":"2017-12-07T15:31:44Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Elazar Leibovich <elazar.leibovich@oracle.com> writes:\n\n> We noticed some unexpected behavior of git, when running git commands under\n> fakeroot, and then running another command without fakeroot.\n>\n> When running, e.g., git status, or git describe --dirty, git can\n> update the index file.\n\nCorrect.  fakeroot would report that the files that are actually\nowned by the user who is running fakeroot are owned by root; the\ncached stat information in the index would be \"corrected\" to say\nthat they are owned by root.  So once the index is refreshed like\nso, things will become consistent.\n\n> The unexpected result is:\n>\n> \"fakeroot git status\" updates the index, and the index now says all files\n> are owned by uid:0.\n\nYou should learn to expect it; that is how fakeroot works.\n\n> \"git diff-index --name-only HEAD\" is used to test if the git tree is dirty\n> without fakeroot, concluding all files have changed, since their owner UID\n> is changed.\n\nThe lower-level plumbing diff-* commands are meant to be used by\nscripts that refresh the index once upfront.\n\nIf you are using \"diff-index\" for the \"is the tree dirty?\" check\nwithout running \"update-index --refresh\", then you are not using\nthe command correctly.\n"},{"id":"334403","messageId":"a770d158-c472-206f-dcdc-55cea0cf308f@oracle.com","threadId":"47394","inReplyTo":"xmqqzi6ueern.fsf@gitster.mtv.corp.google.com","subject":"Re: Unfortunate interaction between git diff-index and fakeroot","fromName":"Elazar Leibovich","fromEmail":"elazar.leibovich@oracle.com","sentAt":"2017-12-08T09:48:40Z","receivedAt":"2017-12-08T09:56:28Z","isPatch":false,"sender":{"key":"elazar.leibovich@oracle.com","avatar":null},"body":"\nOn 12/07/2017 05:31 PM, Junio C Hamano wrote:\n> Correct.  fakeroot would report that the files that are actually\n> owned by the user who is running fakeroot are owned by root; the\n> cached stat information in the index would be \"corrected\" to say\n> that they are owned by root.  So once the index is refreshed like\n> so, things will become consistent.\nI still fail to understand, what's the benefit of storing the owner ID in\nthe index, where no one seems to use this information.\nI mean, one could store many other meta information in the index, such as\nACL lists,  number of blocks allocated, etc, but the code seems to\nignore unused information, such as commit \n2cb45e95438c113871fbbea5b4f629f9463034e7\nwhich ignores st_dev, because it doesn't actually matter, or\ncommit e44794706eeb57f2ee38ed1604821aa38b8ad9d2 which ignores\nmode changes not relevant to the owner.\n\nIn which situation does anyone care about the owner UID?\nWhat's the difference between that, and, e.g., st_dev?\n> If you are using \"diff-index\" for the \"is the tree dirty?\" check\n> without running \"update-index --refresh\", then you are not using\n> the command correctly.\nJust want to clarify. It seems like the linux kernel is using diff-index\nto do just that, in scripts/setlocalversion\n\nhttps://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/scripts/setlocalversion#n77\n\nDo I understand correctly that linux should update the index\nfirst, or better, use porcelain \"git diff --name-only\" instead?\n\nIt seems to be the case previously, but in commit \ncdf2bc632ebc9ef512345fe8e6015edfd367e256\ngit update-index was removed, to prevent error messages on\nread-only linux tree.\n\nDo I understand correctly that the more correct solution\nfor modern git, is to use git diff --name-only instead? It should\nwork on read-only as well as on read-write trees, and\nwould not break if anyone uses git under fakeroot.\n\nDid I understand you correctly?\n"},{"id":"334432","messageId":"xmqqwp1x9qm0.fsf@gitster.mtv.corp.google.com","threadId":"47394","inReplyTo":"a770d158-c472-206f-dcdc-55cea0cf308f@oracle.com","subject":"Re: Unfortunate interaction between git diff-index and fakeroot","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2017-12-08T15:38:47Z","receivedAt":"2017-12-08T15:39:01Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Elazar Leibovich <elazar.leibovich@oracle.com> writes:\n\n> ignore unused information, such as commit\n> 2cb45e95438c113871fbbea5b4f629f9463034e7\n> which ignores st_dev, because it doesn't actually matter, or\n\nI do not think it ignores because \"it doesn't matter\".  st_dev is\nknown not to be stable (e.g. across reboots and reconnects nfs), so\nit is a poor indicator to use as a quick measure to see if the file\ncontents may have changed since we last checked.  On the other hand,\nin a sane (it is of course open to debate \"by whose definition\")\nenvironment, the fact that the owner of the file has changed _is_ a\nsignificant enough sign to suspect that the file contents have\nchanged (i.e. the file is suspect to be dirty; you can actually\ncheck and refresh, of course, though).\n\n> commit e44794706eeb57f2ee38ed1604821aa38b8ad9d2 which ignores\n> mode changes not relevant to the owner.\n\nAnd that one is not even about the cached stat information used for\nthe quick \"dirty-ness\" check.  The change is about the mode bits in\nrecorded history.\n\nFile's mtime is not recorded in the history, either, but we do care\nand record it in the index, because it can be used as a good (albeit\na bit too coarse grained) indicator that the contents of a file may\nhave changed.  \n\nThe owner and group fall into the same category.\n\n"}]}