* [PATCHv2 0/2] Reroll submodule sync --recursive
From: Phil Hord @ 2012-10-26 17:31 UTC (permalink / raw)
To: git; +Cc: phil.hord, Jens Lehmann, Jeff King
In-Reply-To: <5089BBE1.3040107@web.de>
This one fixes "$orig_flags" problems noticed by Jens.
Copy-and-paste is gift from Satan.
Phil
^ permalink raw reply
* [PATCHv2 1/2] Teach --recursive to submodule sync
From: Phil Hord @ 2012-10-26 17:31 UTC (permalink / raw)
To: git; +Cc: phil.hord, Jens Lehmann, Jeff King, Phil Hord
In-Reply-To: <1351272691-24718-1-git-send-email-hordp@cisco.com>
The submodule sync command was somehow left out when
--recursive was added to the other submodule commands.
Teach sync to handle the --recursive switch by recursing
when we're in a submodule we are sync'ing.
Change the report during sync to show submodule-path
instead of submodule-name to be consistent with the other
submodule commands and to help recursed paths make sense.
Signed-off-by: Phil Hord <hordp@cisco.com>
---
git-submodule.sh | 19 ++++++++++++++++---
1 file changed, 16 insertions(+), 3 deletions(-)
diff --git a/git-submodule.sh b/git-submodule.sh
index ab6b110..f40da72 100755
--- a/git-submodule.sh
+++ b/git-submodule.sh
@@ -11,7 +11,7 @@ USAGE="[--quiet] add [-b branch] [-f|--force] [--reference <repository>] [--] <r
or: $dashless [--quiet] update [--init] [-N|--no-fetch] [-f|--force] [--rebase] [--reference <repository>] [--merge] [--recursive] [--] [<path>...]
or: $dashless [--quiet] summary [--cached|--files] [--summary-limit <n>] [commit] [--] [<path>...]
or: $dashless [--quiet] foreach [--recursive] <command>
- or: $dashless [--quiet] sync [--] [<path>...]"
+ or: $dashless [--quiet] sync [--recursive] [--] [<path>...]"
OPTIONS_SPEC=
. git-sh-setup
. git-sh-i18n
@@ -1003,12 +1003,15 @@ cmd_status()
#
cmd_sync()
{
+ orig_flags=
while test $# -ne 0
do
case "$1" in
-q|--quiet)
GIT_QUIET=1
- shift
+ ;;
+ --recursive)
+ recursive=1
;;
--)
shift
@@ -1021,6 +1024,8 @@ cmd_sync()
break
;;
esac
+ orig_flags="$orig_flags $(git rev-parse --sq-quote "$1")"
+ shift
done
cd_to_toplevel
module_list "$@" |
@@ -1051,7 +1056,7 @@ cmd_sync()
if git config "submodule.$name.url" >/dev/null 2>/dev/null
then
- say "$(eval_gettext "Synchronizing submodule url for '\$name'")"
+ say "$(eval_gettext "Synchronizing submodule url for '\$prefix\$sm_path'")"
git config submodule."$name".url "$super_config_url"
if test -e "$sm_path"/.git
@@ -1061,6 +1066,14 @@ cmd_sync()
cd "$sm_path"
remote=$(get_default_remote)
git config remote."$remote".url "$sub_origin_url"
+
+ if test -n "$recursive"
+ then
+ (
+ prefix="$prefix$sm_path/"
+ eval cmd_sync "$orig_flags"
+ )
+ fi
)
fi
fi
--
1.8.0.2.g35128e8
^ permalink raw reply related
* [PATCHv2 2/2] Add tests for submodule sync --recursive
From: Phil Hord @ 2012-10-26 17:31 UTC (permalink / raw)
To: git; +Cc: phil.hord, Jens Lehmann, Jeff King, Phil Hord
In-Reply-To: <1351272691-24718-1-git-send-email-hordp@cisco.com>
Signed-off-by: Phil Hord <hordp@cisco.com>
---
t/t7403-submodule-sync.sh | 55 +++++++++++++++++++++++++++++++++++++++++++++--
1 file changed, 53 insertions(+), 2 deletions(-)
diff --git a/t/t7403-submodule-sync.sh b/t/t7403-submodule-sync.sh
index 524d5c1..94e26c4 100755
--- a/t/t7403-submodule-sync.sh
+++ b/t/t7403-submodule-sync.sh
@@ -17,18 +17,25 @@ test_expect_success setup '
git commit -m upstream &&
git clone . super &&
git clone super submodule &&
+ (cd submodule &&
+ git submodule add ../submodule sub-submodule &&
+ test_tick &&
+ git commit -m "sub-submodule"
+ ) &&
(cd super &&
git submodule add ../submodule submodule &&
test_tick &&
git commit -m "submodule"
) &&
git clone super super-clone &&
- (cd super-clone && git submodule update --init) &&
+ (cd super-clone && git submodule update --init --recursive) &&
git clone super empty-clone &&
(cd empty-clone && git submodule init) &&
git clone super top-only-clone &&
git clone super relative-clone &&
- (cd relative-clone && git submodule update --init)
+ (cd relative-clone && git submodule update --init --recursive) &&
+ git clone super recursive-clone &&
+ (cd recursive-clone && git submodule update --init --recursive)
'
test_expect_success 'change submodule' '
@@ -46,6 +53,11 @@ test_expect_success 'change submodule url' '
git pull
) &&
mv submodule moved-submodule &&
+ (cd moved-submodule &&
+ git config -f .gitmodules submodule.sub-submodule.url ../moved-submodule &&
+ test_tick &&
+ git commit -a -m moved-sub-submodule
+ ) &&
(cd super &&
git config -f .gitmodules submodule.submodule.url ../moved-submodule &&
test_tick &&
@@ -61,6 +73,9 @@ test_expect_success '"git submodule sync" should update submodule URLs' '
test -d "$(cd super-clone/submodule &&
git config remote.origin.url
)" &&
+ test ! -d "$(cd super-clone/submodule/sub-submodule &&
+ git config remote.origin.url
+ )" &&
(cd super-clone/submodule &&
git checkout master &&
git pull
@@ -70,6 +85,25 @@ test_expect_success '"git submodule sync" should update submodule URLs' '
)
'
+test_expect_success '"git submodule sync --recursive" should update all submodule URLs' '
+ (cd super-clone &&
+ (cd submodule &&
+ git pull --no-recurse-submodules
+ ) &&
+ git submodule sync --recursive
+ ) &&
+ test -d "$(cd super-clone/submodule &&
+ git config remote.origin.url
+ )" &&
+ test -d "$(cd super-clone/submodule/sub-submodule &&
+ git config remote.origin.url
+ )" &&
+ (cd super-clone/submodule/sub-submodule &&
+ git checkout master &&
+ git pull
+ )
+'
+
test_expect_success '"git submodule sync" should update known submodule URLs' '
(cd empty-clone &&
git pull &&
@@ -107,6 +141,23 @@ test_expect_success '"git submodule sync" handles origin URL of the form foo/bar
#actual foo/submodule
test "$(git config remote.origin.url)" = "../foo/submodule"
)
+ (cd submodule/sub-submodule &&
+ test "$(git config remote.origin.url)" != "../../foo/submodule"
+ )
+ )
+'
+
+test_expect_success '"git submodule sync --recursive" propagates changes in origin' '
+ (cd recursive-clone &&
+ git remote set-url origin foo/bar &&
+ git submodule sync --recursive &&
+ (cd submodule &&
+ #actual foo/submodule
+ test "$(git config remote.origin.url)" = "../foo/submodule"
+ )
+ (cd submodule/sub-submodule &&
+ test "$(git config remote.origin.url)" = "../../foo/submodule"
+ )
)
'
--
1.8.0.2.g35128e8
^ permalink raw reply related
* Re: [PATCH 1/2] Teach --recursive to submodule sync
From: Phil Hord @ 2012-10-26 17:55 UTC (permalink / raw)
To: Phil Hord; +Cc: Jens Lehmann, git, Jeff King
In-Reply-To: <508AC63E.6010502@cisco.com>
On Fri, Oct 26, 2012 at 1:19 PM, Phil Hord <hordp@cisco.com> wrote:
>
> Yes, thanks for catching that. I think I should add a test for that
> except I notice that sync doesn't take any other flags useful for passing.
Which, of course, suggests that I should not add this
flag-propagating-machinery to submodule-sync at all. yes?
Phil
^ permalink raw reply
* Re: git push tags
From: Drew Northup @ 2012-10-26 18:07 UTC (permalink / raw)
To: Kacper Kornet; +Cc: Angelo Borsotti, git
In-Reply-To: <20121026174255.GE10560@camk.edu.pl>
On Fri, Oct 26, 2012 at 1:42 PM, Kacper Kornet <draenog@pld-linux.org> wrote:
> On Thu, Oct 25, 2012 at 05:16:00PM -0400, Drew Northup wrote:
>> On Thu, Oct 25, 2012 at 3:05 PM, Angelo Borsotti
>> <angelo.borsotti@gmail.com> wrote:
>> > Are remote repositories less protected than the local ones? I
>> > think that to be consistent, the same strategy should be used on all
>> > repositories, i.e. rejecting changes on tags by default, unless they
>> > are forced.
>
>> So here we come to the core argument. Is sounds to me like you want
>> changes to remote tags to work differently from push updates to ALL
>> other references. The required change, if I'm not mistaken, would be
>> for tags to not permit fast-forward updates while all other references
>> would be pushed normally. From my brief and un-enlightened look at the
>> push code I can't see that being as easy as it sounds.
>
> I think the patch below obtains the requested behaviour:
>
> diff --git a/remote.c b/remote.c
> index 04fd9ea..7fcb51e 100644
> --- a/remote.c
> +++ b/remote.c
> @@ -1320,7 +1320,7 @@ void set_ref_status_for_push(struct ref *remote_refs, int send_mirror,
> !ref->deletion &&
> !is_null_sha1(ref->old_sha1) &&
> (!has_sha1_file(ref->old_sha1)
> - || !ref_newer(ref->new_sha1, ref->old_sha1));
> + || !prefixcmp(ref->name, "refs/tags") || !ref_newer(ref->new_sha1, ref->old_sha1));
>
> if (ref->nonfastforward && !ref->force && !force_update) {
> ref->status = REF_STATUS_REJECT_NONFASTFORWARD;
>
> --
> Kacper Kornet
Kacper,
I obviously didn't dig deep enough. In any case, I presume that's what
he's asking for. I can't remember if git is still forcing tags to
always be located in "refs/tags" however. I didn't think it was.
--
-Drew Northup
--------------------------------------------------------------
"As opposed to vegetable or mineral error?"
-John Pescatore, SANS NewsBites Vol. 12 Num. 59
^ permalink raw reply
* Re: git push tags
From: Kacper Kornet @ 2012-10-26 17:42 UTC (permalink / raw)
To: Drew Northup; +Cc: Angelo Borsotti, git
In-Reply-To: <CAM9Z-nkosbe1NXYnu7x6v4seLqCnMBWg-jrdH2eJ9RetaZBTyQ@mail.gmail.com>
On Thu, Oct 25, 2012 at 05:16:00PM -0400, Drew Northup wrote:
> On Thu, Oct 25, 2012 at 3:05 PM, Angelo Borsotti
> <angelo.borsotti@gmail.com> wrote:
> > Are remote repositories less protected than the local ones? I
> > think that to be consistent, the same strategy should be used on all
> > repositories, i.e. rejecting changes on tags by default, unless they
> > are forced.
> So here we come to the core argument. Is sounds to me like you want
> changes to remote tags to work differently from push updates to ALL
> other references. The required change, if I'm not mistaken, would be
> for tags to not permit fast-forward updates while all other references
> would be pushed normally. From my brief and un-enlightened look at the
> push code I can't see that being as easy as it sounds.
I think the patch below obtains the requested behaviour:
diff --git a/remote.c b/remote.c
index 04fd9ea..7fcb51e 100644
--- a/remote.c
+++ b/remote.c
@@ -1320,7 +1320,7 @@ void set_ref_status_for_push(struct ref *remote_refs, int send_mirror,
!ref->deletion &&
!is_null_sha1(ref->old_sha1) &&
(!has_sha1_file(ref->old_sha1)
- || !ref_newer(ref->new_sha1, ref->old_sha1));
+ || !prefixcmp(ref->name, "refs/tags") || !ref_newer(ref->new_sha1, ref->old_sha1));
if (ref->nonfastforward && !ref->force && !force_update) {
ref->status = REF_STATUS_REJECT_NONFASTFORWARD;
--
Kacper Kornet
^ permalink raw reply related
* Re: git push tags
From: Kacper Kornet @ 2012-10-26 18:20 UTC (permalink / raw)
To: Drew Northup; +Cc: Angelo Borsotti, git
In-Reply-To: <CAM9Z-nkBO1dbF-sBFLuxM_S_MT79Cx=gLEL+83XKB7ys8VTqNQ@mail.gmail.com>
On Fri, Oct 26, 2012 at 02:07:09PM -0400, Drew Northup wrote:
> On Fri, Oct 26, 2012 at 1:42 PM, Kacper Kornet <draenog@pld-linux.org> wrote:
> > On Thu, Oct 25, 2012 at 05:16:00PM -0400, Drew Northup wrote:
> >> On Thu, Oct 25, 2012 at 3:05 PM, Angelo Borsotti
> >> <angelo.borsotti@gmail.com> wrote:
> >> > Are remote repositories less protected than the local ones? I
> >> > think that to be consistent, the same strategy should be used on all
> >> > repositories, i.e. rejecting changes on tags by default, unless they
> >> > are forced.
> >> So here we come to the core argument. Is sounds to me like you want
> >> changes to remote tags to work differently from push updates to ALL
> >> other references. The required change, if I'm not mistaken, would be
> >> for tags to not permit fast-forward updates while all other references
> >> would be pushed normally. From my brief and un-enlightened look at the
> >> push code I can't see that being as easy as it sounds.
> > I think the patch below obtains the requested behaviour:
> > diff --git a/remote.c b/remote.c
> > index 04fd9ea..7fcb51e 100644
> > --- a/remote.c
> > +++ b/remote.c
> > @@ -1320,7 +1320,7 @@ void set_ref_status_for_push(struct ref *remote_refs, int send_mirror,
> > !ref->deletion &&
> > !is_null_sha1(ref->old_sha1) &&
> > (!has_sha1_file(ref->old_sha1)
> > - || !ref_newer(ref->new_sha1, ref->old_sha1));
> > + || !prefixcmp(ref->name, "refs/tags") || !ref_newer(ref->new_sha1, ref->old_sha1));
> > if (ref->nonfastforward && !ref->force && !force_update) {
> > ref->status = REF_STATUS_REJECT_NONFASTFORWARD;
> > --
> > Kacper Kornet
> Kacper,
> I obviously didn't dig deep enough. In any case, I presume that's what
> he's asking for. I can't remember if git is still forcing tags to
> always be located in "refs/tags" however. I didn't think it was.
I have based my assumption about location of tags on gitglossary:
tag
A ref under refs/tags/ namespace that points to an object of an
arbitrary type (typically a tag points to either a tag or a commit
object).
--
Kacper
^ permalink raw reply
* Re: git push tags
From: Angelo Borsotti @ 2012-10-26 18:35 UTC (permalink / raw)
To: Kacper Kornet; +Cc: Drew Northup, git
In-Reply-To: <20121026182020.GF10560@camk.edu.pl>
Hello
Drew,
I made some further tests on git-push to see if it handled branches
and tags in the same way, and have discovered the following
differences:
- git push origin --delete master
remote: error: By default, deleting the current branch is denied
- git push origin --delete vx (where vx is a tag)
... accepted
This is consistent with what is done on the local repo: deleting the
current branch is disallowed, but deleting a tag is allowed (even when
HEAD points to it).
That means that git-push does not handle branches and tags exactly the same.
Kacper
thank you for the patch. To keep downward compatibility, the denial to
update tags should perhaps be enabled with some option.
-Angelo
^ permalink raw reply
* Re: [PATCH 1/2] Teach --recursive to submodule sync
From: Jens Lehmann @ 2012-10-26 18:50 UTC (permalink / raw)
To: Phil Hord; +Cc: Phil Hord, git, Jeff King
In-Reply-To: <CABURp0q0uELTS4DQ=Kqfr+8Pr-KgLcGJaBY_kJE_AiO=BZu+Bw@mail.gmail.com>
Am 26.10.2012 19:55, schrieb Phil Hord:
> On Fri, Oct 26, 2012 at 1:19 PM, Phil Hord <hordp@cisco.com> wrote:
>>
>> Yes, thanks for catching that. I think I should add a test for that
>> except I notice that sync doesn't take any other flags useful for passing.
>
> Which, of course, suggests that I should not add this
> flag-propagating-machinery to submodule-sync at all. yes?
Nope, the new --recursive option has to be passed on!
To catch that bug in your test you'd need another submodule inside your
sub-submodule. The first level submodule is initialized by sync anyways,
the sub-submodule is initialized by the --recursive logic you added but
the sub-sub-submodule would not have been synced because the option was
dropped. I really can't blame you for not adding that third level of
submodules ;-)
^ permalink raw reply
* Re: git push tags
From: Kacper Kornet @ 2012-10-26 19:00 UTC (permalink / raw)
To: Angelo Borsotti; +Cc: Drew Northup, git
In-Reply-To: <CAB9Jk9AR7vFBH6E7-hFabyD9XgRrF5PVZU-HtABS85wkwVTt+Q@mail.gmail.com>
On Fri, Oct 26, 2012 at 08:35:50PM +0200, Angelo Borsotti wrote:
> Hello
> Drew,
> I made some further tests on git-push to see if it handled branches
> and tags in the same way, and have discovered the following
> differences:
> - git push origin --delete master
> remote: error: By default, deleting the current branch is denied
> - git push origin --delete vx (where vx is a tag)
> ... accepted
> This is consistent with what is done on the local repo: deleting the
> current branch is disallowed, but deleting a tag is allowed (even when
> HEAD points to it).
> That means that git-push does not handle branches and tags exactly the same.
> Kacper
> thank you for the patch. To keep downward compatibility, the denial to
> update tags should perhaps be enabled with some option.
You are probably right. The proper submission should also contain a
test. I have sent a crude patch to show that the behaviour asked by you
is possible to obtain.
I will try to prepare a formal submission patch, but I can't say how
long it will take me. So if you want to do it by yourself feel free.
--
Kacper Kornet
^ permalink raw reply
* Re: [PATCH] submodule status: properly pass options with --recursive
From: Phil Hord @ 2012-10-26 19:07 UTC (permalink / raw)
To: Jeff King; +Cc: Jens Lehmann, Git Mailing List, Junio C Hamano
In-Reply-To: <20121026131507.GA2747@sigill.intra.peff.net>
On Fri, Oct 26, 2012 at 9:15 AM, Jeff King <peff@peff.net> wrote:
> On Fri, Oct 26, 2012 at 12:20:29AM +0200, Jens Lehmann wrote:
>
>> When renaming orig_args to orig_flags in 98dbe63d (submodule: only
>> preserve flags across recursive status/update invocations) the call site
>> of the recursive cmd_status was forgotten. At that place orig_args is
>> still passed into the recursion, which is always empty now. This clears
>> all options when recursing, as that variable is never set.
>>
>> Fix that by renaming orig_args to orig_flags there too and add a test to
>> catch that bug.
>
> Thanks. I back-ported your patch on top of 98dbe63d so it can go to
> 'maint'. I'm curious, though: why didn't the test right before (which
> checks recursion for --cached) catch this?
I was wondering the same thing about why 'git submodule sync
--recursive --quiet' succeeded, so I checked on it. The answer is
that "--quiet" sets GIT_QUIET=1, which is then inherited by the
recursive call. Indeed, Jens' new test passes even without the
accompanying fix. :-\
The 'status --recursive --cached' test passes for two reasons. The
first is that the test is checking for a cached change in a level-1
submodule, but it is the level-2+ submodules which do not get the
flags passed down correctly in "$@". The 2nd reason is that the
$cached variable is _also_ inherited by the recursive call to
cmd_status, specifically because it is a call to cmd_status() and not
to '$SHELL git submodule status', where the latter would have cleared
the flags at startup, but the former does not.
I'm a bit wary about "fixing" the flags for the recursion machinery.
I'm starting to think $orig_flags is moot in almost all cases. It
looks like clearing all the flags on each iteration would break 'git
submodule foreach --recursive --quiet' because it does not use
$orig_flags at all. Who knows what other surprises lurk there.
Here's a fix for the test, though, even though it still does not fail
in the absence of the $orig_flags patch.
-- >8 --
diff --git i/t/t7407-submodule-foreach.sh w/t/t7407-submodule-foreach.sh
index 9b69fe2..8442b32 100755
--- i/t/t7407-submodule-foreach.sh
+++ w/t/t7407-submodule-foreach.sh
@@ -226,14 +226,14 @@ test_expect_success 'test "status --recursive"' '
test_cmp expect actual
'
-sed -e "/nested1 /s/.*/+$nested1sha1 nested1 (file2~1)/;/sub[1-3]/d"
< expect > expect2
+sed -e "/nested2 /s/.*/+$nested2sha1 nested1\/nested2
(file2~1)/;/sub[1-3]/d" < expect > expect2
mv -f expect2 expect
test_expect_success 'ensure "status --cached --recursive" preserves
the --cached flag' '
(
cd clone3 &&
(
- cd nested1 &&
+ cd nested1/nested2 &&
test_commit file2
) &&
git submodule status --cached --recursive -- nested1 > ../actual
@@ -245,6 +245,14 @@ test_expect_success 'ensure "status --cached
--recursive" preserves the --cached
test_cmp expect actual
'
test_expect_success 'use "git clone --recursive" to checkout all submodules' '
git clone --recursive super clone4 &&
(
Phil
^ permalink raw reply related
* Re: Can't understand the behaviour of git-diff --submodule
From: Jens Lehmann @ 2012-10-26 19:08 UTC (permalink / raw)
To: Francis Moreau; +Cc: git
In-Reply-To: <CAC9WiBgzbsury2f9FyAu=Pgn31f2uCtq7AvsVWGWEwoV6KbyjA@mail.gmail.com>
Am 26.10.2012 16:07, schrieb Francis Moreau:
> I'm trying to use the --submodule switch with git-diff but doesnt
> understand the following behaviour:
>
> $ git diff 2c9a257718d1803de720f95766ff256d33accad5 HEAD
> diff --git a/configs b/configs
> index 16c6a89..ce12289 160000
> --- a/configs
> +++ b/configs
> @@ -1 +1 @@
> -Subproject commit 16c6a89f245f0eed7fb0bce8e027c59fcf1d543e
> +Subproject commit ce12289c5bfca7b2c423d9f1871c13ad1ba1dc32
>
> but adding the --submodule option gives:
>
> $ git diff --submodule=log 2c9a257718d1803de720f95766ff256d33accad5 HEAD
> Submodule configs 16c6a89...ce12289 (commits not present)
>
> Could anybody enlight me ?
The output "Submodule configs 16c6a89...ce12289 (commits not present)"
contains same SHA-1s, only in their abbreviated form. That is the same
information you get without the --submodule option, but in shorter
format: it says the submodule moved from 16c6a89 to ce12289 in the
given commit range of the superproject (and the "..." part tells us it
wasn't a fast-forward). The "(commits not present)" part indicates that
even though git diff would have wanted to show you what happened in the
submodule between 16c6a89 and ce12289 by displaying the first line of
each commit message, it couldn't because these commit(s) are not present
in the submodule repo. If you do a "git log --oneline 16c6a89...ce12289"
inside the submodule you'll get an "unknown revision" error for the same
reason.
The "(commits not present)" part should go away and the output of the
short commit messages should appear when you do a "git fetch" inside the
submodule (at least unless someone forgot to push these submodule commits
upstream). As "git fetch" does attempt to fetch all referenced submodules
too, I suspect that you have unpushed commits (e.g. they might have been
dropped because of a rebase). In this case a "git submodule update" will
fail for the same reason.
Does that make it clearer?
^ permalink raw reply
* Re: git push tags
From: Drew Northup @ 2012-10-26 19:08 UTC (permalink / raw)
To: Kacper Kornet; +Cc: Angelo Borsotti, git
In-Reply-To: <20121026190029.GB15328@camk.edu.pl>
On Fri, Oct 26, 2012 at 3:00 PM, Kacper Kornet <draenog@pld-linux.org> wrote:
> On Fri, Oct 26, 2012 at 08:35:50PM +0200, Angelo Borsotti wrote:
>> Hello
>
>> Drew,
....
>> Kacper
>
>> thank you for the patch. To keep downward compatibility, the denial to
>> update tags should perhaps be enabled with some option.
>
> You are probably right. The proper submission should also contain a
> test. I have sent a crude patch to show that the behaviour asked by you
> is possible to obtain.
>
> I will try to prepare a formal submission patch, but I can't say how
> long it will take me. So if you want to do it by yourself feel free.
Kacper,
I have been rebuilding my local so that generating a proper patch for
the manpage change part of the set won't make a horrible mess over
here.
It looks to me like the changeset requires the following:
Kacper's patch, some manpage changes, a test.
Angelo, may we use your original mail as the source for the cover page?
--
-Drew Northup
--------------------------------------------------------------
"As opposed to vegetable or mineral error?"
-John Pescatore, SANS NewsBites Vol. 12 Num. 59
^ permalink raw reply
* [PATCH] t7407: Fix recursive submodule test
From: Phil Hord @ 2012-10-26 19:13 UTC (permalink / raw)
To: Git Mailing List; +Cc: phil.hord, Jens Lehmann, Jeff King, Phil Hord
In-Reply-To: <CABURp0op2+QUvusUmAFUxT8s8c02bB9V3=ag9gTTSiiN4t96OA@mail.gmail.com>
A test in t7404-submodule-foreach purports to test that
the --cached flag is properly noticed by --recursive calls
to the foreach command as it descends into nested
submodules. However, the test really does not perform this
test since the change it looks for is in a top-level
submodule handled by the first invocation of the command.
To properly test for the flag being passed to recursive
invocations, the change must be buried deeper in the
hierarchy.
Move the change one level deeper so it properly verifies
the recursive machinery of the 'git submodule status'
command.
Signed-off-by: Phil Hord <hordp@cisco.com>
---
t/t7407-submodule-foreach.sh | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/t/t7407-submodule-foreach.sh b/t/t7407-submodule-foreach.sh
index 9b69fe2..107b4b7 100755
--- a/t/t7407-submodule-foreach.sh
+++ b/t/t7407-submodule-foreach.sh
@@ -226,14 +226,14 @@ test_expect_success 'test "status --recursive"' '
test_cmp expect actual
'
-sed -e "/nested1 /s/.*/+$nested1sha1 nested1 (file2~1)/;/sub[1-3]/d" < expect > expect2
+sed -e "/nested2 /s/.*/+$nested2sha1 nested1\/nested2 (file2~1)/;/sub[1-3]/d" < expect > expect2
mv -f expect2 expect
test_expect_success 'ensure "status --cached --recursive" preserves the --cached flag' '
(
cd clone3 &&
(
- cd nested1 &&
+ cd nested1/nested2 &&
test_commit file2
) &&
git submodule status --cached --recursive -- nested1 > ../actual
--
1.8.0.2.gd008466.dirty
^ permalink raw reply related
* Re: [PATCH] submodule status: properly pass options with --recursive
From: Jens Lehmann @ 2012-10-26 19:26 UTC (permalink / raw)
To: Phil Hord; +Cc: Jeff King, Git Mailing List, Junio C Hamano
In-Reply-To: <CABURp0op2+QUvusUmAFUxT8s8c02bB9V3=ag9gTTSiiN4t96OA@mail.gmail.com>
Am 26.10.2012 21:07, schrieb Phil Hord:
> On Fri, Oct 26, 2012 at 9:15 AM, Jeff King <peff@peff.net> wrote:
>> On Fri, Oct 26, 2012 at 12:20:29AM +0200, Jens Lehmann wrote:
>>
>>> When renaming orig_args to orig_flags in 98dbe63d (submodule: only
>>> preserve flags across recursive status/update invocations) the call site
>>> of the recursive cmd_status was forgotten. At that place orig_args is
>>> still passed into the recursion, which is always empty now. This clears
>>> all options when recursing, as that variable is never set.
>>>
>>> Fix that by renaming orig_args to orig_flags there too and add a test to
>>> catch that bug.
>>
>> Thanks. I back-ported your patch on top of 98dbe63d so it can go to
>> 'maint'. I'm curious, though: why didn't the test right before (which
>> checks recursion for --cached) catch this?
>
> I was wondering the same thing about why 'git submodule sync
> --recursive --quiet' succeeded, so I checked on it. The answer is
> that "--quiet" sets GIT_QUIET=1, which is then inherited by the
> recursive call. Indeed, Jens' new test passes even without the
> accompanying fix. :-\
Dang, you're right! At least that explains why nobody noticed that
so far ... (and that's what you get for skipping the "does the test
fail without your fix?" part because the fix is so obvious :-( )
> The 'status --recursive --cached' test passes for two reasons. The
> first is that the test is checking for a cached change in a level-1
> submodule, but it is the level-2+ submodules which do not get the
> flags passed down correctly in "$@". The 2nd reason is that the
> $cached variable is _also_ inherited by the recursive call to
> cmd_status, specifically because it is a call to cmd_status() and not
> to '$SHELL git submodule status', where the latter would have cleared
> the flags at startup, but the former does not.
>
> I'm a bit wary about "fixing" the flags for the recursion machinery.
> I'm starting to think $orig_flags is moot in almost all cases. It
> looks like clearing all the flags on each iteration would break 'git
> submodule foreach --recursive --quiet' because it does not use
> $orig_flags at all. Who knows what other surprises lurk there.
I agree, it looks like ripping out the orig_flags would be the way
to go.
^ permalink raw reply
* Re: [PATCH] t7407: Fix recursive submodule test
From: Jens Lehmann @ 2012-10-26 19:29 UTC (permalink / raw)
To: Phil Hord; +Cc: Git Mailing List, phil.hord, Jeff King
In-Reply-To: <1351278834-28867-1-git-send-email-hordp@cisco.com>
Am 26.10.2012 21:13, schrieb Phil Hord:
> A test in t7404-submodule-foreach purports to test that
> the --cached flag is properly noticed by --recursive calls
> to the foreach command as it descends into nested
> submodules. However, the test really does not perform this
> test since the change it looks for is in a top-level
> submodule handled by the first invocation of the command.
> To properly test for the flag being passed to recursive
> invocations, the change must be buried deeper in the
> hierarchy.
>
> Move the change one level deeper so it properly verifies
> the recursive machinery of the 'git submodule status'
> command.
Me thinks we should definitely do this.
> Signed-off-by: Phil Hord <hordp@cisco.com>
> ---
> t/t7407-submodule-foreach.sh | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/t/t7407-submodule-foreach.sh b/t/t7407-submodule-foreach.sh
> index 9b69fe2..107b4b7 100755
> --- a/t/t7407-submodule-foreach.sh
> +++ b/t/t7407-submodule-foreach.sh
> @@ -226,14 +226,14 @@ test_expect_success 'test "status --recursive"' '
> test_cmp expect actual
> '
>
> -sed -e "/nested1 /s/.*/+$nested1sha1 nested1 (file2~1)/;/sub[1-3]/d" < expect > expect2
> +sed -e "/nested2 /s/.*/+$nested2sha1 nested1\/nested2 (file2~1)/;/sub[1-3]/d" < expect > expect2
> mv -f expect2 expect
>
> test_expect_success 'ensure "status --cached --recursive" preserves the --cached flag' '
> (
> cd clone3 &&
> (
> - cd nested1 &&
> + cd nested1/nested2 &&
> test_commit file2
> ) &&
> git submodule status --cached --recursive -- nested1 > ../actual
>
^ permalink raw reply
* [PATCHv3 0/2] Teach --recursive to submodule sync
From: Phil Hord @ 2012-10-26 19:44 UTC (permalink / raw)
To: git; +Cc: phil.hord, Jens Lehmann, Jeff King
In-Reply-To: <508AE3E9.6000304@web.de>
[PATCHv3 1/2] Teach --recursive to submodule sync
Now with less noise and no redundant flags passed to the recursive call.
[PATCHv3 2/2] Add tests for submodule sync --recursive
The test remains unchanged.
^ permalink raw reply
* [PATCHv3 1/2] Teach --recursive to submodule sync
From: Phil Hord @ 2012-10-26 19:44 UTC (permalink / raw)
To: git; +Cc: phil.hord, Jens Lehmann, Jeff King, Phil Hord
In-Reply-To: <1351280683-8402-1-git-send-email-hordp@cisco.com>
The submodule sync command was somehow left out when
--recursive was added to the other submodule commands.
Teach sync to handle the --recursive switch by recursing
when we're in a submodule we are sync'ing.
Change the report during sync to show submodule-path
instead of submodule-name to be consistent with the other
submodule commands and to help recursed paths make sense.
Signed-off-by: Phil Hord <hordp@cisco.com>
---
git-submodule.sh | 14 ++++++++++++--
1 file changed, 12 insertions(+), 2 deletions(-)
diff --git a/git-submodule.sh b/git-submodule.sh
index ab6b110..0ca3af2 100755
--- a/git-submodule.sh
+++ b/git-submodule.sh
@@ -11,7 +11,7 @@ USAGE="[--quiet] add [-b branch] [-f|--force] [--reference <repository>] [--] <r
or: $dashless [--quiet] update [--init] [-N|--no-fetch] [-f|--force] [--rebase] [--reference <repository>] [--merge] [--recursive] [--] [<path>...]
or: $dashless [--quiet] summary [--cached|--files] [--summary-limit <n>] [commit] [--] [<path>...]
or: $dashless [--quiet] foreach [--recursive] <command>
- or: $dashless [--quiet] sync [--] [<path>...]"
+ or: $dashless [--quiet] sync [--recursive] [--] [<path>...]"
OPTIONS_SPEC=
. git-sh-setup
. git-sh-i18n
@@ -1010,6 +1010,10 @@ cmd_sync()
GIT_QUIET=1
shift
;;
+ --recursive)
+ recursive=1
+ shift
+ ;;
--)
shift
break
@@ -1051,7 +1055,7 @@ cmd_sync()
if git config "submodule.$name.url" >/dev/null 2>/dev/null
then
- say "$(eval_gettext "Synchronizing submodule url for '\$name'")"
+ say "$(eval_gettext "Synchronizing submodule url for '\$prefix\$sm_path'")"
git config submodule."$name".url "$super_config_url"
if test -e "$sm_path"/.git
@@ -1061,6 +1065,12 @@ cmd_sync()
cd "$sm_path"
remote=$(get_default_remote)
git config remote."$remote".url "$sub_origin_url"
+
+ if test -n "$recursive"
+ then
+ prefix="$prefix$sm_path/"
+ eval cmd_sync
+ fi
)
fi
fi
--
1.8.0.3.g9dae067
^ permalink raw reply related
* [PATCHv3 2/2] Add tests for submodule sync --recursive
From: Phil Hord @ 2012-10-26 19:44 UTC (permalink / raw)
To: git; +Cc: phil.hord, Jens Lehmann, Jeff King, Phil Hord
In-Reply-To: <1351280683-8402-1-git-send-email-hordp@cisco.com>
Signed-off-by: Phil Hord <hordp@cisco.com>
---
t/t7403-submodule-sync.sh | 55 +++++++++++++++++++++++++++++++++++++++++++++--
1 file changed, 53 insertions(+), 2 deletions(-)
diff --git a/t/t7403-submodule-sync.sh b/t/t7403-submodule-sync.sh
index 524d5c1..94e26c4 100755
--- a/t/t7403-submodule-sync.sh
+++ b/t/t7403-submodule-sync.sh
@@ -17,18 +17,25 @@ test_expect_success setup '
git commit -m upstream &&
git clone . super &&
git clone super submodule &&
+ (cd submodule &&
+ git submodule add ../submodule sub-submodule &&
+ test_tick &&
+ git commit -m "sub-submodule"
+ ) &&
(cd super &&
git submodule add ../submodule submodule &&
test_tick &&
git commit -m "submodule"
) &&
git clone super super-clone &&
- (cd super-clone && git submodule update --init) &&
+ (cd super-clone && git submodule update --init --recursive) &&
git clone super empty-clone &&
(cd empty-clone && git submodule init) &&
git clone super top-only-clone &&
git clone super relative-clone &&
- (cd relative-clone && git submodule update --init)
+ (cd relative-clone && git submodule update --init --recursive) &&
+ git clone super recursive-clone &&
+ (cd recursive-clone && git submodule update --init --recursive)
'
test_expect_success 'change submodule' '
@@ -46,6 +53,11 @@ test_expect_success 'change submodule url' '
git pull
) &&
mv submodule moved-submodule &&
+ (cd moved-submodule &&
+ git config -f .gitmodules submodule.sub-submodule.url ../moved-submodule &&
+ test_tick &&
+ git commit -a -m moved-sub-submodule
+ ) &&
(cd super &&
git config -f .gitmodules submodule.submodule.url ../moved-submodule &&
test_tick &&
@@ -61,6 +73,9 @@ test_expect_success '"git submodule sync" should update submodule URLs' '
test -d "$(cd super-clone/submodule &&
git config remote.origin.url
)" &&
+ test ! -d "$(cd super-clone/submodule/sub-submodule &&
+ git config remote.origin.url
+ )" &&
(cd super-clone/submodule &&
git checkout master &&
git pull
@@ -70,6 +85,25 @@ test_expect_success '"git submodule sync" should update submodule URLs' '
)
'
+test_expect_success '"git submodule sync --recursive" should update all submodule URLs' '
+ (cd super-clone &&
+ (cd submodule &&
+ git pull --no-recurse-submodules
+ ) &&
+ git submodule sync --recursive
+ ) &&
+ test -d "$(cd super-clone/submodule &&
+ git config remote.origin.url
+ )" &&
+ test -d "$(cd super-clone/submodule/sub-submodule &&
+ git config remote.origin.url
+ )" &&
+ (cd super-clone/submodule/sub-submodule &&
+ git checkout master &&
+ git pull
+ )
+'
+
test_expect_success '"git submodule sync" should update known submodule URLs' '
(cd empty-clone &&
git pull &&
@@ -107,6 +141,23 @@ test_expect_success '"git submodule sync" handles origin URL of the form foo/bar
#actual foo/submodule
test "$(git config remote.origin.url)" = "../foo/submodule"
)
+ (cd submodule/sub-submodule &&
+ test "$(git config remote.origin.url)" != "../../foo/submodule"
+ )
+ )
+'
+
+test_expect_success '"git submodule sync --recursive" propagates changes in origin' '
+ (cd recursive-clone &&
+ git remote set-url origin foo/bar &&
+ git submodule sync --recursive &&
+ (cd submodule &&
+ #actual foo/submodule
+ test "$(git config remote.origin.url)" = "../foo/submodule"
+ )
+ (cd submodule/sub-submodule &&
+ test "$(git config remote.origin.url)" = "../../foo/submodule"
+ )
)
'
--
1.8.0.3.g9dae067
^ permalink raw reply related
* Re: git submodule summary doesn't return an error when passed a wrong commit/rev
From: Jens Lehmann @ 2012-10-26 19:53 UTC (permalink / raw)
To: Francis Moreau; +Cc: git
In-Reply-To: <CAC9WiBgdgy1bwh0c16jd017q2rqQAq-suDADn2-vGw9eubBs_w@mail.gmail.com>
Am 26.10.2012 16:03, schrieb Francis Moreau:
> it seems to me that when passed an unknown rev or a wrong commit/sha1,
> git-submodule-summary should at least exit with an error status. Even better
> would be a error output.
>
> Test was done with git version 1.7.10.4 from debian wheezy.
Thanks for your report, I think you found a real issue. Some quick
tests showed some problems with other parameter combinations too.
I'll take a deeper look the next days.
^ permalink raw reply
* Re: Can't understand the behaviour of git-diff --submodule
From: Francis Moreau @ 2012-10-26 19:54 UTC (permalink / raw)
To: Jens Lehmann; +Cc: git
In-Reply-To: <508ADFAE.1050800@web.de>
Hi,
Thanks for answering
On Fri, Oct 26, 2012 at 9:08 PM, Jens Lehmann <Jens.Lehmann@web.de> wrote:
> Am 26.10.2012 16:07, schrieb Francis Moreau:
>> I'm trying to use the --submodule switch with git-diff but doesnt
>> understand the following behaviour:
>>
>> $ git diff 2c9a257718d1803de720f95766ff256d33accad5 HEAD
>> diff --git a/configs b/configs
>> index 16c6a89..ce12289 160000
>> --- a/configs
>> +++ b/configs
>> @@ -1 +1 @@
>> -Subproject commit 16c6a89f245f0eed7fb0bce8e027c59fcf1d543e
>> +Subproject commit ce12289c5bfca7b2c423d9f1871c13ad1ba1dc32
>>
>> but adding the --submodule option gives:
>>
>> $ git diff --submodule=log 2c9a257718d1803de720f95766ff256d33accad5 HEAD
>> Submodule configs 16c6a89...ce12289 (commits not present)
>>
>> Could anybody enlight me ?
>
> The output "Submodule configs 16c6a89...ce12289 (commits not present)"
> contains same SHA-1s, only in their abbreviated form. That is the same
> information you get without the --submodule option, but in shorter
> format: it says the submodule moved from 16c6a89 to ce12289 in the
> given commit range of the superproject (and the "..." part tells us it
> wasn't a fast-forward). The "(commits not present)" part indicates that
> even though git diff would have wanted to show you what happened in the
> submodule between 16c6a89 and ce12289 by displaying the first line of
> each commit message, it couldn't because these commit(s) are not present
> in the submodule repo. If you do a "git log --oneline 16c6a89...ce12289"
> inside the submodule you'll get an "unknown revision" error for the same
> reason.
Well, no the commits are present in the submodule, that's what I tried
to show with the first 'git-diff' command I did in my previous post
(without the --submodule switch).
And to check again, this is the result of git log:
$ cd configs
$ git log --oneline 16c6a89...ce12289
ce12289 test 2
[...]
> Does that make it clearer?
Unforunately not really.
Thanks.
--
Francis
^ permalink raw reply
* Re: git submodule summary doesn't return an error when passed a wrong commit/rev
From: Francis Moreau @ 2012-10-26 19:56 UTC (permalink / raw)
To: Jens Lehmann; +Cc: git
In-Reply-To: <508AEA39.2020205@web.de>
On Fri, Oct 26, 2012 at 9:53 PM, Jens Lehmann <Jens.Lehmann@web.de> wrote:
> Am 26.10.2012 16:03, schrieb Francis Moreau:
>> it seems to me that when passed an unknown rev or a wrong commit/sha1,
>> git-submodule-summary should at least exit with an error status. Even better
>> would be a error output.
>>
>> Test was done with git version 1.7.10.4 from debian wheezy.
>
> Thanks for your report, I think you found a real issue. Some quick
> tests showed some problems with other parameter combinations too.
> I'll take a deeper look the next days.
Glad to help :)
--
Francis
^ permalink raw reply
* Re: Can't understand the behaviour of git-diff --submodule
From: Jens Lehmann @ 2012-10-26 20:05 UTC (permalink / raw)
To: Francis Moreau; +Cc: git
In-Reply-To: <CAC9WiBjiHLJggUzmmx4sPpXNNq=Kz0TOZAzmRShc1AZcPjGvig@mail.gmail.com>
Am 26.10.2012 21:54, schrieb Francis Moreau:
> On Fri, Oct 26, 2012 at 9:08 PM, Jens Lehmann <Jens.Lehmann@web.de> wrote:
>> Am 26.10.2012 16:07, schrieb Francis Moreau:
>>> I'm trying to use the --submodule switch with git-diff but doesnt
>>> understand the following behaviour:
>>>
>>> $ git diff 2c9a257718d1803de720f95766ff256d33accad5 HEAD
>>> diff --git a/configs b/configs
>>> index 16c6a89..ce12289 160000
>>> --- a/configs
>>> +++ b/configs
>>> @@ -1 +1 @@
>>> -Subproject commit 16c6a89f245f0eed7fb0bce8e027c59fcf1d543e
>>> +Subproject commit ce12289c5bfca7b2c423d9f1871c13ad1ba1dc32
>>>
>>> but adding the --submodule option gives:
>>>
>>> $ git diff --submodule=log 2c9a257718d1803de720f95766ff256d33accad5 HEAD
>>> Submodule configs 16c6a89...ce12289 (commits not present)
>>>
>>> Could anybody enlight me ?
>>
>> The output "Submodule configs 16c6a89...ce12289 (commits not present)"
>> contains same SHA-1s, only in their abbreviated form. That is the same
>> information you get without the --submodule option, but in shorter
>> format: it says the submodule moved from 16c6a89 to ce12289 in the
>> given commit range of the superproject (and the "..." part tells us it
>> wasn't a fast-forward). The "(commits not present)" part indicates that
>> even though git diff would have wanted to show you what happened in the
>> submodule between 16c6a89 and ce12289 by displaying the first line of
>> each commit message, it couldn't because these commit(s) are not present
>> in the submodule repo. If you do a "git log --oneline 16c6a89...ce12289"
>> inside the submodule you'll get an "unknown revision" error for the same
>> reason.
>
> Well, no the commits are present in the submodule, that's what I tried
> to show with the first 'git-diff' command I did in my previous post
> (without the --submodule switch).
Oh, that only shows the commits of the submodule recorded in the
superproject and not that they are present there (you'll even get
that output when the submodule was never initialized and is empty).
> And to check again, this is the result of git log:
>
> $ cd configs
> $ git log --oneline 16c6a89...ce12289
> ce12289 test 2
>
> [...]
That is weird, "git diff --submodule" should show that too. Is there
anything unusual about your setup? (The only explanation I can come
up with after checking the code is that your submodule has neither a
.git directory nor a gitfile or the objects directory in there doesn't
contain these commits)
^ permalink raw reply
* Re: Can't understand the behaviour of git-diff --submodule
From: Francis Moreau @ 2012-10-26 20:43 UTC (permalink / raw)
To: Jens Lehmann; +Cc: git
In-Reply-To: <508AED26.3090805@web.de>
On Fri, Oct 26, 2012 at 10:05 PM, Jens Lehmann <Jens.Lehmann@web.de> wrote:
[...]
>
> That is weird, "git diff --submodule" should show that too. Is there
> anything unusual about your setup? (The only explanation I can come
> up with after checking the code is that your submodule has neither a
> .git directory nor a gitfile or the objects directory in there doesn't
> contain these commits)
Oh now you're asking, I think the submodule has been added by using
the --reference option of git-submodule-add.
$ cd configs
$ cat .git
gitdir: ../.git/modules/configs
Thanks
--
Francis
^ permalink raw reply
* Strange behaviour of git diff branch1 ... branch2
From: Aaron Schrab @ 2012-10-26 21:26 UTC (permalink / raw)
To: git
I came across this odd question on stackoverflow:
http://stackoverflow.com/q/13092854/1507392
If git diff is run with "..." as a separate argument between two
commit-ish arguments causes it to produce strange output. The
differences seem to be the same as if "..." was left out, but change
lines begin with 4 + or - characters rather than just 1.
Can anybody explain what is happening here? I don't have any reason to
want to use that form myself, but I'm very curious about why it produces
this odd output.
Thanks.
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox