All of lore.kernel.org
 help / color / mirror / Atom feed
From: Junio C Hamano <gitster@pobox.com>
To: Jay Soffian <jaysoffian@gmail.com>
Cc: git@vger.kernel.org, Jens Lehmann <Jens.Lehmann@web.de>
Subject: Re: [PATCH] submodule: teach "foreach" command a --revision <tree-ish> option
Date: Tue, 09 Oct 2012 11:24:17 -0700	[thread overview]
Message-ID: <7vr4p7fqr2.fsf@alter.siamese.dyndns.org> (raw)
In-Reply-To: <CAG+J_Dw1iXJfgkmA2V-L11xCOOxO57U4Dh7=h7AzkFUqLc55=w@mail.gmail.com> (Jay Soffian's message of "Tue, 9 Oct 2012 02:50:10 -0400")

Jay Soffian <jaysoffian@gmail.com> writes:

> On Tue, Oct 9, 2012 at 2:12 AM, Junio C Hamano <gitster@pobox.com> wrote:
>> Junio C Hamano <gitster@pobox.com> writes:
>>
>>> Assuming that the above guess is correct (which is a huge
>>> assumption, given the lack of clarity in the description), I think
>>> the feature might make sense.  The example would have been a lot
>>> easier to follow if it were something like this:
>>>
>>>     $ git submodule foreach --revision v1.0 'git grep -e frotz $sha1'
>>
>> Imagine you have a checkout of v2.0 of the superproject in your
>> working tree, and you run "git submodule foreach --revision v1.0".
>> Further imagine a submodule S that used to exist back when the
>> superproject was at v1.0 no longer exists in the current codebase
>> (hence there is no such submodule in the working tree).
>>
>> Shouldn't the above "foreach ... grep" still try to find 'frotz' in
>> the submodule S that was bound to v1.0 of the superproject?
>>
>> Given that your patch does not touch the part of cmd_foreach where
>> it decides which submodule to descend into, it still will base its
>> decision solely on the set of submodules that are bound to and have
>> been "git submodule init"ed in the version of the superproject that
>> is _currently_ checked out, no?
>
> That's a good observation. My use-case for this (poorly explained in
> ...
> As you previously stated, I need to improve the documentation that
> goes along with this patch, so I'll call-out this limitation. I'm not
> sure what else can be done. You can't descend into a submodule that
> isn't there.

As recent "submodule rm" work by Jens indicates, since 501770e (Move
git-dir for submodules, 2011-08-15), you should be able to peek into
submodules that have been "git submodule init"ed but do not exist in
the current checkout of the superproject.

I think the right approach to implement this "recurse foreach in the
superproject tree that is not checkout out to the working tree"
feature should be:

 - Advertise it so that it is crystal clear that the command run by
   "foreach" may have to run in a bare repository of submodule to
   look at submodule's commit bound to the historical tree of the
   superproject;

 - Anytime you look at .gitmodules in the superproject, read from
   the historical tree's .gitmodules instead of from the working
   tree (I think this is necessary in order to get the $sm_name vs
   $sm_path mapping right); and

 - Locate submodule's $GIT_DIR in $GIT_DIR/modules/$sm_name of the
   superproject that corresponds to the submodule found in the
   historical tree in the superproject (or if it is the same
   repository as that is currently checked out, use $sm_path/.git),
   and error out when it is not available.

An implementation that works only when all the submodules necessary
in the historical tree in the superproject are still in the current
checkout of the superproject may be fine as a quick throw-away hack,
and it may even be acceptable as a good first step towards the real
feature, but at least it needs to be protected by an error checking
upfront (perhaps running "diff-tree -r" between the index and the
historical tree to make sure there are no removed submodules that
existed in the historical tree), if it does not bother to check with
$GIT_DIR/modules/$sm_name in the superproject.

Jens, anything I missed?

  reply	other threads:[~2012-10-09 18:24 UTC|newest]

Thread overview: 8+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2012-10-09  0:50 [PATCH] submodule: teach "foreach" command a --revision <tree-ish> option Jay Soffian
2012-10-09  5:55 ` Junio C Hamano
2012-10-09  6:12   ` Junio C Hamano
2012-10-09  6:50     ` Jay Soffian
2012-10-09 18:24       ` Junio C Hamano [this message]
2012-10-09 21:21         ` Jens Lehmann
2012-10-09 21:38           ` Jay Soffian
2012-10-09 21:48           ` 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=7vr4p7fqr2.fsf@alter.siamese.dyndns.org \
    --to=gitster@pobox.com \
    --cc=Jens.Lehmann@web.de \
    --cc=git@vger.kernel.org \
    --cc=jaysoffian@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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.