git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Jens Lehmann <Jens.Lehmann@web.de>
To: Junio C Hamano <gitster@pobox.com>, Duy Nguyen <pclouds@gmail.com>
Cc: Git Mailing List <git@vger.kernel.org>,
	Dennis Kaarsemaker <dennis@kaarsemaker.net>
Subject: Re: [PATCH] pathspec: adjust prefixlen after striping trailing slash
Date: Wed, 22 Apr 2015 21:14:37 +0200	[thread overview]
Message-ID: <5537F31D.4090704@web.de> (raw)
In-Reply-To: <xmqq8udlgpey.fsf@gitster.dls.corp.google.com>

Am 21.04.2015 um 23:08 schrieb Junio C Hamano:
> 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.

  reply	other threads:[~2015-04-22 19:14 UTC|newest]

Thread overview: 15+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2015-04-13 16:55 assert failed in submodule edge case Dennis Kaarsemaker
2015-04-13 16:57 ` Dennis Kaarsemaker
2015-04-16 19:27 ` Jens Lehmann
2015-04-18  1:19   ` [PATCH] pathspec: adjust prefixlen after striping trailing slash Nguyễn Thái Ngọc Duy
2015-04-19 12:53     ` Jens Lehmann
2015-04-20  1:34       ` Duy Nguyen
2015-04-20  5:37         ` Junio C Hamano
2015-04-20  5:52           ` Duy Nguyen
2015-04-21 21:08             ` Junio C Hamano
2015-04-22 19:14               ` Jens Lehmann [this message]
2015-04-22 19:58                 ` Junio C Hamano
2015-04-22 22:32                   ` Jens Lehmann
2015-04-23  3:47                     ` Junio C Hamano
2015-06-14 13:16                       ` Duy Nguyen
2015-06-14 21:34                         ` Junio C Hamano

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=5537F31D.4090704@web.de \
    --to=jens.lehmann@web.de \
    --cc=dennis@kaarsemaker.net \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    --cc=pclouds@gmail.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).