git.vger.kernel.org archive mirror
 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 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).