{"thread":{"id":"42152","subject":"Strangeness with git-add and nested repositories","startedAt":"2016-04-27T08:31:11Z","lastAt":"2016-04-29T02:54:55Z","messageCount":10,"participants":["Andrew J","Stefan Beller","Junio C Hamano"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"284637","messageId":"CAH6n4TdG9LQOPaaw_H6vuCgia0-4JXhPsSiAJPa5GtjfduQoSw@mail.gmail.com","threadId":"42152","inReplyTo":null,"subject":"Strangeness with git-add and nested repositories","fromName":"Andrew J","fromEmail":"andj2223@gmail.com","sentAt":"2016-04-27T08:31:11Z","receivedAt":"2016-04-27T08:31:11Z","isPatch":false,"sender":{"key":"andj2223@gmail.com","avatar":null},"body":"I've noticed some strangeness with git-add and nested repos.\n\nThis tar.gz has a reproduction script, parent repo, and nested repo\nthat exhibits the issue:\nhttps://drive.google.com/uc?export=download&id=0Bwjufq6oAZMfcGVxZ2dlMElEVlE\n\nIf you extract the archive and do the following (on linux please, I\nhaven't even tried this on Windows):\ncd git-add-bug\n./bad-git-add.sh\n(Examine the script before running, please)\n\nIt will echo what I'm encountering to the screen, so it should be\nstraightforward to follow, but I will summarize here:\nIf I run a git-add command where one or more of the files live in\nnested repositories, like so (command is shortened here, includes more\nfiles in the script):\ngit add -v -f --\nsrc/chromium/src/third_party/libFuzzer/src/FuzzerInterface.h testfile\nAs confirmed by git-status, this results in testfile being added, but\nFuzzerInterface.h not being added.\nFuzzerInterface.h is inside of a nested repository\n(src/chromium/src/third_party/libFuzzer/src/), while testfile is in\nthe current main repository.\n\nOn the other hand, the following command:\ngit add -v -f -- src/chromium/src/third_party/libFuzzer/src/FuzzerInterface.h\nResults in FuzzerInterface.h being added, as confirmed by git-status.\nExcluding testfile from the git-add command seems to do the trick.\n\nMy expectation:\nBoth testfile and FuzzerInterface.h should be added if they are\nspecified in the git-add command, regardless of whether they are\nspecified along with another file that doesn't happen to live in a\nnested repository (\"testfile\" in this example).\n\nIf someone could help me understand what's going on here, I'd appreciate it.\n\nThanks,\n\nAndrew\n"},{"id":"284660","messageId":"CAGZ79kZhATfP1FpXnhivCa_Az-3KADSCReOo68E2Q3s29x5HNw@mail.gmail.com","threadId":"42152","inReplyTo":"CAH6n4TdG9LQOPaaw_H6vuCgia0-4JXhPsSiAJPa5GtjfduQoSw@mail.gmail.com","subject":"Re: Strangeness with git-add and nested repositories","fromName":"Stefan Beller","fromEmail":"sbeller@google.com","sentAt":"2016-04-27T16:08:22Z","receivedAt":"2016-04-27T16:08:22Z","isPatch":false,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":"On Wed, Apr 27, 2016 at 1:31 AM, Andrew J wrote:\n> I've noticed some strangeness with git-add and nested repos.\n>\n> This tar.gz has a reproduction script, parent repo, and nested repo\n> that exhibits the issue:\n> https://drive.google.com/uc?export=download&id=0Bwjufq6oAZMfcGVxZ2dlMElEVlE\n>\n> If you extract the archive and do the following (on linux please, I\n> haven't even tried this on Windows):\n> cd git-add-bug\n> ./bad-git-add.sh\n> (Examine the script before running, please)\n>\n> It will echo what I'm encountering to the screen, so it should be\n> straightforward to follow, but I will summarize here:\n> If I run a git-add command where one or more of the files live in\n> nested repositories, like so (command is shortened here, includes more\n> files in the script):\n> git add -v -f --\n> src/chromium/src/third_party/libFuzzer/src/FuzzerInterface.h testfile\n> As confirmed by git-status, this results in testfile being added, but\n> FuzzerInterface.h not being added.\n> FuzzerInterface.h is inside of a nested repository\n> (src/chromium/src/third_party/libFuzzer/src/), while testfile is in\n> the current main repository.\n>\n> On the other hand, the following command:\n> git add -v -f -- src/chromium/src/third_party/libFuzzer/src/FuzzerInterface.h\n> Results in FuzzerInterface.h being added, as confirmed by git-status.\n> Excluding testfile from the git-add command seems to do the trick.\n>\n> My expectation:\n> Both testfile and FuzzerInterface.h should be added if they are\n> specified in the git-add command, regardless of whether they are\n> specified along with another file that doesn't happen to live in a\n> nested repository (\"testfile\" in this example).\n\nI would call it a bug. [This use case is interesting for working with submodules\n(though your nested repositories do not seem to be submodules), so worth\nlooking at for me.]\n\n>\n> If someone could help me understand what's going on here, I'd appreciate it.\n\nI think (pure speculation), that it the error is in the context\n(repository) switching logic.\nWhat happens if you alter the order, i.e. give testfile first and then\nthe files in the nested\nrepos?\n\n git add -- file path/to/subdir/file\n\nshould do internally IMHO:\n\n git add file\n git -C path-to-subdir add file\n\nThanks,\nStefan\n\n>\n> Thanks,\n>\n> Andrew\n> --\n> To unsubscribe from this list: send the line \"unsubscribe git\" in\n> the body of a message to majordomo@vger.kernel.org\n> More majordomo info at http://vger.kernel.org/majordomo-info.html\n"},{"id":"284767","messageId":"CAH6n4TfvybZyAYLkzyjfaP=2ZkirpacTgVQApF3bZT-j8=_Qng@mail.gmail.com","threadId":"42152","inReplyTo":"CAGZ79kZhATfP1FpXnhivCa_Az-3KADSCReOo68E2Q3s29x5HNw@mail.gmail.com","subject":"Re: Strangeness with git-add and nested repositories","fromName":"Andrew J","fromEmail":"andj2223@gmail.com","sentAt":"2016-04-28T06:10:24Z","receivedAt":"2016-04-28T06:10:24Z","isPatch":false,"sender":{"key":"andj2223@gmail.com","avatar":null},"body":"Hi Stefan,\n\nOn Wed, Apr 27, 2016 at 9:08 AM, Stefan Beller wrote:\n> I think (pure speculation), that it the error is in the context\n> (repository) switching logic.\n> What happens if you alter the order, i.e. give testfile first and then\n> the files in the nested\n> repos?\n\nInterestingly, reversing the order produces the same result (the\ntestfile is added, the nested files are not).\n\nI've also noticed that running something like 'git status testfile\nnestedfiles' results in the nested files being omitted from the git\nstatus output; I'd expect them to be printed by git-status as\nuntracked files. So it appears the problem is not isolated to git-add.\n\nTo give some context, my use case is that I have a parent project that\nlinks to numerous chromium libraries, thus my parent project needs\naccess to many of chromium's headers at build time. I wanted to make\nthese headers available to other developers without them needing to\ncheck out all of chromium.\nSo I add all the chromium headers to the parent project with something like:\nfind deps/chromium/src -name \"*.h\" | xargs git add --\nI was weirded out to find that many of the header files weren't being\nadded, as I've already described.\n\nI ultimately worked around this by doing:\nfind chromium/src -name \"*.h\" | xargs -n 1 git add\nSince each file gets added separately, this is quite slow. So it'd be\nnice if this little bug was fixed someday :)\n\nAs you probably know, Chromium is comprised of many hundreds of nested\nrepos, so that aided in manifesting this issue.\n\nThanks,\nAndrew\n"},{"id":"284838","messageId":"CAGZ79ka9v0v4gwtbiojR5o89JWLDZYkDM2CmhySeXXKYaiLQig@mail.gmail.com","threadId":"42152","inReplyTo":"CAH6n4TfvybZyAYLkzyjfaP=2ZkirpacTgVQApF3bZT-j8=_Qng@mail.gmail.com","subject":"Re: Strangeness with git-add and nested repositories","fromName":"Stefan Beller","fromEmail":"sbeller@google.com","sentAt":"2016-04-28T16:35:26Z","receivedAt":"2016-04-28T16:35:26Z","isPatch":false,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":"On Wed, Apr 27, 2016 at 11:10 PM, Andrew J wrote:\n> Hi Stefan,\n>\n> On Wed, Apr 27, 2016 at 9:08 AM, Stefan Beller wrote:\n>> I think (pure speculation), that it the error is in the context\n>> (repository) switching logic.\n>> What happens if you alter the order, i.e. give testfile first and then\n>> the files in the nested\n>> repos?\n>\n> Interestingly, reversing the order produces the same result (the\n> testfile is added, the nested files are not).\n>\n> I've also noticed that running something like 'git status testfile\n> nestedfiles' results in the nested files being omitted from the git\n> status output; I'd expect them to be printed by git-status as\n> untracked files. So it appears the problem is not isolated to git-add.\n\nLooking at the code, git-add uses parse_pathspec, which is used my\nmost commands[https://github.com/git/git/blob/master/builtin/add.c#L364]\nand I think that function needs to learn to cover paths for different\nrepositories.\nMaybe we can pass in a flag, which allows parse_pathspec to get files from\nthe different repos and then it's up to each command how to deal with that\nlist of files.\n\n\n\n>\n> To give some context, my use case is that I have a parent project that\n> links to numerous chromium libraries, thus my parent project needs\n> access to many of chromium's headers at build time. I wanted to make\n> these headers available to other developers without them needing to\n> check out all of chromium.\n> So I add all the chromium headers to the parent project with something like:\n> find deps/chromium/src -name \"*.h\" | xargs git add --\n> I was weirded out to find that many of the header files weren't being\n> added, as I've already described.\n>\n> I ultimately worked around this by doing:\n> find chromium/src -name \"*.h\" | xargs -n 1 git add\n> Since each file gets added separately, this is quite slow. So it'd be\n> nice if this little bug was fixed someday :)\n>\n> As you probably know, Chromium is comprised of many hundreds of nested\n> repos, so that aided in manifesting this issue.\n>\n> Thanks,\n> Andrew\n"},{"id":"284839","messageId":"xmqq1t5p7kmp.fsf@gitster.mtv.corp.google.com","threadId":"42152","inReplyTo":"CAGZ79kZhATfP1FpXnhivCa_Az-3KADSCReOo68E2Q3s29x5HNw@mail.gmail.com","subject":"Re: Strangeness with git-add and nested repositories","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2016-04-28T16:39:42Z","receivedAt":"2016-04-28T16:39:42Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Stefan Beller writes:\n\n> I think (pure speculation), that it the error is in the context\n> (repository) switching logic.\n> What happens if you alter the order, i.e. give testfile first and then\n> the files in the nested\n> repos?\n>\n> git add -- file path/to/subdir/file\n>\n> should do internally IMHO:\n>\n> git add file\n> git -C path-to-subdir add file\n\nMy undertanding of what _should_ happen in the world order as\ncurrently defined (not necessarily implemented) is:\n\n * \"git add -- A B\" must work the same way as \"git add -- B A\" and\n \"git add -- A; git add -- B\"\n\n * \"git add -- path/to/subdir/file\", when any of path/, path/to/,\n path/to/subdir/ is a Git repository that is different from the\n current Git repository, must fail.\n\nIOW, if 'path' is a repository (whether it is known as a submodule\nto the repository whose working tree contains it, or it is an\nuntracked directory from the containing repository's point of view),\nthe index of the containing repository cannot get path/$anything in\nit. If you managed to do so, you found a bug [*1*].\n\npath/.git/index can of course have \"to/subdir/file\" in it, and from\nthat point of view, \"git -C path/to/subdir add file\" may one day\nbecome an improved world order. It is just we haven't discussed\nthat possibility or reached concensus that it is a good idea.\n\n\n[Footnote]\n\n*1* Of course, some of the bugs in this class may fundamentally be\n unfixable and would fall into the same category as \"doctor, it\n hurts when I do this--don't do it then\". For example, you may\n treat path/ as the top of the working tree of another repository\n whose git-dir is not at path/.git by arranging GIT_WORK_TREE and\n GIT_DIR environment variables, but you may do so only when you\n actually are accessing the contents of path/ as its own project.\n And when you are using the enclosing project (whose .git/ would\n sit next to path/), there is no way for \"git add path/to/file\"\n to know that everything under \"path/\" does not belong to the\n current repository and instead it is part of the project rooted\n at path/, which is an obvious example of \"fundamentally\n unfixable\" case.\n"},{"id":"284840","messageId":"xmqqtwil65nx.fsf@gitster.mtv.corp.google.com","threadId":"42152","inReplyTo":"xmqq1t5p7kmp.fsf@gitster.mtv.corp.google.com","subject":"Re: Strangeness with git-add and nested repositories","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2016-04-28T16:48:18Z","receivedAt":"2016-04-28T16:48:18Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Junio C Hamano writes:\n\n> * \"git add -- path/to/subdir/file\", when any of path/, path/to/,\n> path/to/subdir/ is a Git repository that is different from the\n> current Git repository, must fail.\n\nIf any of the leading directories in that long path is actually a\ngit repository that is different from the current one, it shouldn't\nhave added it. IOW, adding the path is a bug.\n\n\"git add A/B/C\" usually enforces this rule by\n\n - check A; if A is a separate repository (A/.git exists, etc.),\n do not add this path;\n\n - check A/B; if A/B is a separate repository, ignore;\n\n - check A/B/C; if A/B/C is a separate repository, ignore.\n otherwise add it.\n\nWhen given multiple paths, e.g. \"git add A/B/C A/D\", it tries to\n\"optimize\" things by first finding common leading directory (in this\ncase \"A/\") and doing something slightly different, and I think the\nbug Andrew saw lies in that codepath. It is likely that the code is\nforgetting to make sure that there is no top of enclosed working\ntree in the common leading directory part of the path.\n"},{"id":"284843","messageId":"CAGZ79ka0WB58HyHYXAkaMYKccWD-NwSxhRUGHvq1TrA_ix+Hxw@mail.gmail.com","threadId":"42152","inReplyTo":"xmqq1t5p7kmp.fsf@gitster.mtv.corp.google.com","subject":"Re: Strangeness with git-add and nested repositories","fromName":"Stefan Beller","fromEmail":"sbeller@google.com","sentAt":"2016-04-28T16:54:23Z","receivedAt":"2016-04-28T16:54:23Z","isPatch":false,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":"On Thu, Apr 28, 2016 at 9:39 AM, Junio C Hamano wrote:\n> Stefan Beller writes:\n>\n>> I think (pure speculation), that it the error is in the context\n>> (repository) switching logic.\n>> What happens if you alter the order, i.e. give testfile first and then\n>> the files in the nested\n>> repos?\n>>\n>> git add -- file path/to/subdir/file\n>>\n>> should do internally IMHO:\n>>\n>> git add file\n>> git -C path-to-subdir add file\n>\n> My undertanding of what _should_ happen in the world order as\n> currently defined (not necessarily implemented) is:\n>\n> * \"git add -- A B\" must work the same way as \"git add -- B A\" and\n> \"git add -- A; git add -- B\"\n\nI agree.\n\n>\n> * \"git add -- path/to/subdir/file\", when any of path/, path/to/,\n> path/to/subdir/ is a Git repository that is different from the\n> current Git repository, must fail.\n\nI agree that this is the current expectation for the world order.\nHowever I would like to propose to change that eventually.\n(Once the submodule groups are there and we can treat\nsubmodules as a special form of narrow checkout, we want to\nhave the feature of adding across submodules and even committing\nacross submodules/repos, I would think)\n\n>\n> IOW, if 'path' is a repository (whether it is known as a submodule\n> to the repository whose working tree contains it, or it is an\n> untracked directory from the containing repository's point of view),\n> the index of the containing repository cannot get path/$anything in\n> it. If you managed to do so, you found a bug [*1*].\n>\n> path/.git/index can of course have \"to/subdir/file\" in it, and from\n> that point of view, \"git -C path/to/subdir add file\" may one day\n> become an improved world order. It is just we haven't discussed\n> that possibility or reached concensus that it is a good idea.\n>\n>\n> [Footnote]\n>\n> *1* Of course, some of the bugs in this class may fundamentally be\n> unfixable and would fall into the same category as \"doctor, it\n> hurts when I do this--don't do it then\". For example, you may\n> treat path/ as the top of the working tree of another repository\n> whose git-dir is not at path/.git by arranging GIT_WORK_TREE and\n> GIT_DIR environment variables, but you may do so only when you\n> actually are accessing the contents of path/ as its own project.\n> And when you are using the enclosing project (whose .git/ would\n> sit next to path/), there is no way for \"git add path/to/file\"\n> to know that everything under \"path/\" does not belong to the\n> current repository and instead it is part of the project rooted\n> at path/, which is an obvious example of \"fundamentally\n> unfixable\" case.\n>\n"},{"id":"284888","messageId":"CAH6n4TdYOKT_e4MziZ1qwAX+xZ_3NJEHcKgTRK1BqAUd_mvYOQ@mail.gmail.com","threadId":"42152","inReplyTo":"xmqqtwil65nx.fsf@gitster.mtv.corp.google.com","subject":"Re: Strangeness with git-add and nested repositories","fromName":"Andrew J","fromEmail":"andj2223@gmail.com","sentAt":"2016-04-29T02:11:51Z","receivedAt":"2016-04-29T02:11:51Z","isPatch":false,"sender":{"key":"andj2223@gmail.com","avatar":null},"body":"On Thu, Apr 28, 2016 at 9:48 AM, Junio C Hamano wrote:\n> Junio C Hamano writes:\n> When given multiple paths, e.g. \"git add A/B/C A/D\", it tries to\n> \"optimize\" things by first finding common leading directory (in this\n> case \"A/\") and doing something slightly different, and I think the\n> bug Andrew saw lies in that codepath. It is likely that the code is\n> forgetting to make sure that there is no top of enclosed working\n> tree in the common leading directory part of the path.\n\nActually, the very simplest case succeeds in adding a file from a nested repo:\n==\n$ mkdir parent\n$ cd parent\n$ git init\nInitialized empty Git repository in /home/ajohnson/parent/.git/\n$ mkdir subrepo\n$ cd subrepo\n$ git init\nInitialized empty Git repository in /home/ajohnson/parent/subrepo/.git/\n$ touch subfile\n$ git add subfile\n$ git commit -m test\n[master (root-commit) 7480a2f] test\n 1 file changed, 0 insertions(+), 0 deletions(-)\n create mode 100644 subfile\n$ cd ..\n$ git add subrepo/subfile\n$ git status\nOn branch master\n\nInitial commit\n\nChanges to be committed:\n (use \"git rm --cached ...\" to unstage)\n\n new file: subrepo/subfile\n==\n\nIt is this more complex case that fails to add the file from the nested repo:\n==\n$ mkdir parent\n$ cd parent\n$ git init\nInitialized empty Git repository in /home/ajohnson/parent/.git/\n$ mkdir subrepo\n$ cd subrepo\n$ git init\nInitialized empty Git repository in /home/ajohnson/parent/subrepo/.git/\n$ touch subfile\n$ git add subfile\n$ git commit -m test\n[master (root-commit) 36adec4] test\n 1 file changed, 0 insertions(+), 0 deletions(-)\n create mode 100644 subfile\n$ cd ..\n$ touch parentfile\n$ git add parentfile subrepo/subfile\n$ git status\nOn branch master\n\nInitial commit\n\nChanges to be committed:\n (use \"git rm --cached ...\" to unstage)\n\n new file: parentfile\n\nUntracked files:\n (use \"git add ...\" to include in what will be committed)\n\n subrepo/\n==\n\nGiven that the very simplest case of adding a single nested file\nsucceeds, I had assumed that the intended world order was to be able\nto add files from nested repos, and that the more edgy case of adding\na mixture of files from the parent and nested repos failing was the\nbug.\n\nFor my part, I'd *much* prefer to be able to add files from nested\nrepos, so all this \"adding any files from nested repos is a bug\" talk\nis bumming me out!\n\nThanks,\n\nAndrew\n"},{"id":"284889","messageId":"xmqqd1p92kll.fsf@gitster.mtv.corp.google.com","threadId":"42152","inReplyTo":"xmqq1t5p7kmp.fsf@gitster.mtv.corp.google.com","subject":"Re: Strangeness with git-add and nested repositories","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2016-04-29T02:51:34Z","receivedAt":"2016-04-29T02:51:34Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Junio C Hamano writes:\n\n> My undertanding of what _should_ happen in the world order as\n> currently defined (not necessarily implemented) is:\n>\n> * \"git add -- A B\" must work the same way as \"git add -- B A\" and\n> \"git add -- A; git add -- B\"\n>\n> * \"git add -- path/to/subdir/file\", when any of path/, path/to/,\n> path/to/subdir/ is a Git repository that is different from the\n> current Git repository, must fail.\n>\n> IOW, if 'path' is a repository (whether it is known as a submodule\n> to the repository whose working tree contains it, or it is an\n> untracked directory from the containing repository's point of view),\n> the index of the containing repository cannot get path/$anything in\n> it. If you managed to do so, you found a bug [*1*].\n>\n> path/.git/index can of course have \"to/subdir/file\" in it, and from\n> that point of view, \"git -C path/to/subdir add file\" may one day\n> become an improved world order. It is just we haven't discussed\n> that possibility or reached concensus that it is a good idea.\n\nA cursory bisection suggests that this used to work up to at least\nv2.7.4; this must be a recent regression.\n"},{"id":"284890","messageId":"xmqq8tzx2kg0.fsf@gitster.mtv.corp.google.com","threadId":"42152","inReplyTo":"xmqqd1p92kll.fsf@gitster.mtv.corp.google.com","subject":"Re: Strangeness with git-add and nested repositories","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2016-04-29T02:54:55Z","receivedAt":"2016-04-29T02:54:55Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Junio C Hamano writes:\n\n> A cursory bisection suggests that this used to work up to at least\n> v2.7.4; this must be a recent regression.\n\nNo, false report. This seems to be broken from ages ago.\n"}]}