git/list[1] front-page[2] threads[3] people[4] search[5] about
 

Re: [PATCH] pathspec: adjust prefixlen after striping trailing slash

From
Jens Lehmann <jens.lehmann@web.de>
Date
Apr 22, 2015, 19:14 UTC
Message-ID
<5537F31D.4090704@web.de>
In-Reply-To
<xmqq8udlgpey.fsf@gitster.dls.corp.google.com>
Am 21.04.2015 um 23:08 schrieb Junio C Hamano:
Show 70 quoted lines
> Duy Nguyen <pclouds@gmail.com> writes:
>
>> On Mon, Apr 20, 2015 at 12:37 PM, Junio C Hamano <gitster@pobox.com> wrote:
>>> Duy Nguyen <pclouds@gmail.com> writes:
>>>
>>>> But if you look at it another way, "cd subrepo; git add ." should be
>>>> the same as "git add subrepo" ...
>>>
>>> Once you cd into "subrepo", you are in a different world, a world
>>> different from the toplevel project.  "git add ." over there should
>>> mean "add everything in subproject's working tree to subproject's
>>> index", shouldn't it?  On the other hand, "git add subrepo" without
>>> leavingin the working tree of the superproject is about binding the
>>> submodule to the superproject's index.
>>>
>>> I do not think these two should be the same.  Where am I mistaken?
>
>> I think I wrote this sentence and deleted it: I didn't say which way
>> was right.
>
> OK.
>
> I looked at the test script update.  The new test does (I am
> rephrasing to make it clearer):
>
>      mkdir -p dir/ectory
>      git init dir/ectory ;# a new directory inside top-level project
>      (
>          cd dir/ectory &&
>          >test && git add test && git commit -m test
>      )
>      git add dir/ectory
>
> to set it up.  At this point, the top-level index knows dir/ectory
> is a submodule.
>
> Then the test goes on to turn it a non submodule by
>
>      mv dir/ectory/.git dir/ectory/dotgit
>
> At this point, I think there are two valid things that can happen.
>
>   (1) "git add test" inside dir/ectory removes the submodule entry
>       "dir/ectory" and then adds dir/ectory/test as an individual
>       entry.  After all that is what happens when you replace a file
>       with a directory, e.g.
>
>         >folder && git add folder
>         rm folder
>         mkdir folder && >folder/file &&
>         git -C folder add file
>
>       will first register a regular file "folder" and then replace
>       that with paths underneath.
>
>       It would be the same if you did any of the following:
>
>         git -C . add dir/ectory/test
>         git -C dir add ectory/test
>         git -C dir/ectory add test
>
>   (2) "git add test" inside "dir/ectory" would barf, saying
>       "dir/ectory is supposed to be a submodule and you have to first
>       remove it".  Again, it would be the same if you did so from any
>       directory with relative paths.
>
> I think (2) is less optimal than (1), but it could be the best we
> could do if the submodule infrastracture around .gitmodules and
> links to $GIT_DIR/modules/$name may have to get involved in an
> operation like this (I didn't check).
We already do (2) in the cases you describe:
    $ git add subrepo/a
    fatal: Pathspec 'subrepo/a' is in submodule 'subrepo'
    $ git -C subrepo add a
    fatal: Pathspec 'a' is in submodule 'subrepo'

And I believe that is better than (1), because when removing a submodule its entry in .gitmodules should also be removed. And I suspect that adding a file in a submodule to the superproject will happen more by accident than on purpose (which cannot happen for files because when you add a new file in a directory for which git still records a file the latter can be safely removed as that entry cannot be a file in the worktree anymore).

So I'd vote to have (2) also for "git -C subrepo add .", which is what started this thread.

> In my quick test, it appeared that the behaviour with this patch
> applied was neither of the two and instead to silently do nothing,
> and if that is the case I think it is quite wrong.
Yep.
Previous: Junio C HamanoNext: Junio C Hamano
Message 10 of 15 in “assert failed in submodule edge case”
  1. Dennis KaarsemakerApr 13, 2015
  2. Dennis KaarsemakerApr 13, 2015
  3. Jens LehmannApr 16, 2015
  4. pathspec: adjust prefixlen after striping trailing slashNguyễn Thái Ngọc Duy, Apr 18, 2015
  5. Jens LehmannApr 19, 2015
  6. Duy NguyenApr 20, 2015
  7. Junio C HamanoApr 20, 2015
  8. Duy NguyenApr 20, 2015
  9. Junio C HamanoApr 21, 2015
  10. Jens LehmannApr 22, 2015
  11. Junio C HamanoApr 22, 2015
  12. Jens LehmannApr 22, 2015
  13. Junio C HamanoApr 23, 2015
  14. Duy NguyenJun 14, 2015
  15. Junio C HamanoJun 14, 2015

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.