All of lore.kernel.org
 help / color / mirror / Atom feed
From: Jens Lehmann <Jens.Lehmann@web.de>
To: John Keeping <john@keeping.me.uk>
Cc: Junio C Hamano <gitster@pobox.com>, git@vger.kernel.org
Subject: Re: What's cooking in git.git (May 2013, #09; Wed, 29)
Date: Mon, 03 Jun 2013 23:47:23 +0200	[thread overview]
Message-ID: <51AD0EEB.4020106@web.de> (raw)
In-Reply-To: <20130531194051.GC1072@serenity.lan>

Am 31.05.2013 21:40, schrieb John Keeping:
> On Thu, May 30, 2013 at 09:23:40PM +0200, Jens Lehmann wrote:
>> Am 30.05.2013 01:58, schrieb Junio C Hamano:
>>> * jk/submodule-subdirectory-ok (2013-04-24) 3 commits
>>>   (merged to 'next' on 2013-04-24 at 6306b29)
>>>  + submodule: fix quoting in relative_path()
>>>   (merged to 'next' on 2013-04-22 at f211e25)
>>>  + submodule: drop the top-level requirement
>>>  + rev-parse: add --prefix option
>>>
>>>  Allow various subcommands of "git submodule" to be run not from the
>>>  top of the working tree of the superproject.
>>
>> The summary and status commands are looking good in this version
>> (they are now showing the submodule directory paths relative to
>> the current directory). Apart from that my other remarks from
>> gmane $221575 still seem to apply. And this series has only tests
>> for status, summary and add (and that just with an absolute URL),
>> I'd rather like to see a test for each submodule command (and a
>> relative add to) to document the desired behavior.
> 
> To summarize what I think are the outstanding issues from your email:
> 
> * Should '$sm_path' be relative in "submodule foreach"?
> * "submodule add" with a relative path
> * "submodule init" initializes all submodules
> * Tests
> 
> The current version does make '$sm_path' relative in "submodule
> foreach", although it's hard to spot because we have to leave doing so
> until right before the "eval".

Yes. If I read the code correctly the submodule is cd'ed in before
the foreach command is executed, so $sm_path should only be used for
displaying info about where the command is executed anyway. Looks
like your code is doing the right thing adjusting $sm_path to be
relative to the directory the user is in. But a test showing that
would really be nice ;-)

> I'm not sure what you mean about "submodule add" - the new version
> treats the "path" argument as relative (providing it is not an absolute
> path).  The "repository" argument is not changed by running from a
> subdirectory but I think that's correct since it is documented as being
> relative to the superproject's origin repository.

Sorry, I should have been more specific here. I saw that you did some
changes to make "submodule add" do the right thing with relative paths,
but the following change to t7406 does not work like I believe it
should but instead makes the test fail:
-------------------8<---------------------
diff --git a/t/t7406-submodule-update.sh b/t/t7406-submodule-update.sh
index a4ffea0..9766b9e 100755
--- a/t/t7406-submodule-update.sh
+++ b/t/t7406-submodule-update.sh
@@ -559,7 +559,9 @@ test_expect_success 'add different submodules to the same pa
 test_expect_success 'submodule add places git-dir in superprojects git-dir' '
        (cd super &&
         mkdir deeper &&
-        git submodule add ../submodule deeper/submodule &&
+        (cd deeper &&
+         git submodule add ../../submodule submodule
+        ) &&
         (cd deeper/submodule &&
          git log > ../../expected
         ) &&
-------------------8<---------------------

> "submodule init" is behaving in the same way as "deinit" - if you say
> "submodule init ." then it will only initialize submodules below the
> current directory.  The difference is that "deinit" dies if it is not
> given any arguments whereas "init" will initialize everything from the
> top level down.  I'm not sure whether to change this; given the
> direction "git add -u" is heading in for 2.0 I think the current
> behaviour is the most consistent with the rest of Git.

I meant that both commands still print the submodule names from the
top-level directory, not the one the user is in.

  parent reply	other threads:[~2013-06-03 22:00 UTC|newest]

Thread overview: 23+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2013-05-29 23:58 What's cooking in git.git (May 2013, #09; Wed, 29) Junio C Hamano
2013-05-30  9:47 ` Thomas Rast
2013-05-30  9:56   ` Ramkumar Ramachandra
2013-06-02 23:44   ` Junio C Hamano
2013-05-30 19:18 ` Jens Lehmann
2013-06-02 18:50   ` Junio C Hamano
2013-06-03 21:27     ` Jens Lehmann
2013-07-01 22:05       ` Junio C Hamano
2013-05-30 19:23 ` Jens Lehmann
2013-05-31 19:40   ` John Keeping
2013-06-03 14:54     ` John Keeping
2013-06-03 15:30       ` Junio C Hamano
2013-06-03 21:47     ` Jens Lehmann [this message]
2013-06-03 22:23       ` John Keeping
2013-06-04  5:29         ` Heiko Voigt
2013-06-04  8:10           ` John Keeping
2013-06-04 11:17             ` Heiko Voigt
2013-06-04 12:48               ` John Keeping
2013-06-04 21:39                 ` Jens Lehmann
2013-06-04 22:04                   ` John Keeping
2013-06-04 22:57                 ` Re: " Phil Hord
2013-06-05  8:19                   ` John Keeping
2013-05-31  6:16 ` Øystein Walle

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=51AD0EEB.4020106@web.de \
    --to=jens.lehmann@web.de \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    --cc=john@keeping.me.uk \
    /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.