* [PATCH] submodule: no [--merge|--rebase] when newly cloned
@ 2011-02-16 12:46 Spencer E. Olson
2011-02-16 19:51 ` Junio C Hamano
0 siblings, 1 reply; 9+ messages in thread
From: Spencer E. Olson @ 2011-02-16 12:46 UTC (permalink / raw)
To: git; +Cc: Spencer E. Olson
Previously, when a submodule was cloned in the same command execution, --rebase
or --merge would attempt to run (instead of and) before a checkout, resulting in
an empty working directory. This patch ignores --rebase or --merge for a
particular submodule when that submodule is newly cloned and instead simply
checks out the appropriate revision.
Signed-off-by: Spencer E. Olson <olsonse@umich.edu>
---
git-submodule.sh | 28 ++++++++++++++++++++++++++++
1 files changed, 28 insertions(+), 0 deletions(-)
diff --git a/git-submodule.sh b/git-submodule.sh
index 8b90589..44d8741 100755
--- a/git-submodule.sh
+++ b/git-submodule.sh
@@ -365,6 +365,27 @@ cmd_init()
}
#
+# Test whether an element of the $3(=IFS) separated list $2 matches $1
+#
+str_contains()
+{
+ if test -n "$3"
+ then
+ local IFS="$3"
+ fi
+ for i in $2
+ do
+ if test "$i" = "$1"
+ then
+ echo "yes"
+ return
+ fi
+ done
+ echo "no"
+ return
+}
+
+#
# Update each submodule path to correct revision, using clone and checkout as needed
#
# $@ = requested paths (default to all)
@@ -423,6 +444,7 @@ cmd_update()
cmd_init "--" "$@" || return
fi
+ cloned_modules=
module_list "$@" |
while read mode sha1 stage path
do
@@ -442,6 +464,7 @@ cmd_update()
if ! test -d "$path"/.git -o -f "$path"/.git
then
module_clone "$path" "$url" "$reference"|| exit
+ cloned_modules="$cloned_modules;$name"
subsha1=
else
subsha1=$(clear_local_git_env; cd "$path" &&
@@ -469,6 +492,11 @@ cmd_update()
die "Unable to fetch in submodule path '$path'"
fi
+ if test "$(str_contains "$name" "$cloned_modules" '\;')" = "yes"
+ then
+ update_module=
+ fi
+
case "$update_module" in
rebase)
command="git rebase"
--
1.7.4.1.42.g43f9f
^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [PATCH] submodule: no [--merge|--rebase] when newly cloned
2011-02-16 12:46 [PATCH] submodule: no [--merge|--rebase] when newly cloned Spencer E. Olson
@ 2011-02-16 19:51 ` Junio C Hamano
2011-02-16 20:10 ` Jens Lehmann
2011-02-17 7:10 ` Johannes Sixt
0 siblings, 2 replies; 9+ messages in thread
From: Junio C Hamano @ 2011-02-16 19:51 UTC (permalink / raw)
To: Spencer E. Olson; +Cc: git
"Spencer E. Olson" <olsonse@umich.edu> writes:
> Previously, when a submodule was cloned in the same command execution, --rebase
> or --merge would attempt to run (instead of and) before a checkout, resulting in
> an empty working directory. This patch ignores --rebase or --merge for a
> particular submodule when that submodule is newly cloned and instead simply
> checks out the appropriate revision.
Sorry, but I cannot parse the problem description, "(instead of and)" part.
Why is it a better thing to do to ignore these options, instead of
detecting the situation and error it out, saying "you are initially
cloning, don't say --rebase"?
> +# Test whether an element of the $3(=IFS) separated list $2 matches $1
> +#
> +str_contains()
> +{
> + if test -n "$3"
> + then
> + local IFS="$3"
> + fi
> + for i in $2
> + do
> + if test "$i" = "$1"
> + then
> + echo "yes"
> + return
> + fi
> + done
> + echo "no"
> + return
> +}
We don't allow bash-ism outside POSIX shell in our primary Porcelain set
(the rule is looser for stuff in contrib/), so "local" is out.
Here is how to write the above more concisely, efficiently and portably.
case "$2;" in
*";$1;"*)
echo yes ;;
*)
echo no ;;
esac
The trailing ';' takes care of the case where cloned_modules has only one
element, in which case you have ";name" in "$2". No need for a loop.
> + cloned_modules=
> ...
> while read mode sha1 stage path
> do
> @@ -442,6 +464,7 @@ cmd_update()
> if ! test -d "$path"/.git -o -f "$path"/.git
> then
> module_clone "$path" "$url" "$reference"|| exit
> + cloned_modules="$cloned_modules;$name"
> subsha1=
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] submodule: no [--merge|--rebase] when newly cloned
2011-02-16 19:51 ` Junio C Hamano
@ 2011-02-16 20:10 ` Jens Lehmann
2011-02-16 21:41 ` Junio C Hamano
2011-02-17 7:10 ` Johannes Sixt
1 sibling, 1 reply; 9+ messages in thread
From: Jens Lehmann @ 2011-02-16 20:10 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Spencer E. Olson, git
Am 16.02.2011 20:51, schrieb Junio C Hamano:
> "Spencer E. Olson" <olsonse@umich.edu> writes:
>
>> Previously, when a submodule was cloned in the same command execution, --rebase
>> or --merge would attempt to run (instead of and) before a checkout, resulting in
>> an empty working directory. This patch ignores --rebase or --merge for a
>> particular submodule when that submodule is newly cloned and instead simply
>> checks out the appropriate revision.
>
> Sorry, but I cannot parse the problem description, "(instead of and)" part.
I think what he wanted to say was that after a new submodule is cloned
by running "git submodule update" it will attempt to do a rebase or
merge without having checked out the submodule at all, which obviously
can't work. A plain checkout looks like the right thing to do, as there
aren't any local commits to rebase or merge yet.
> Why is it a better thing to do to ignore these options, instead of
> detecting the situation and error it out, saying "you are initially
> cloning, don't say --rebase"?
It should be fine to use these options when new submodules appear.
(And even without explicitly specifing these command line options
this bug can also be triggered by having the "submodule.<name>.update"
option set in .gitmodules to either "rebase" or "merge", which is easy
to miss)
So this looks like a worthwhile fix. The commit message and POSIX issue
need to be addressed, tests would be a good thing to add too, but apart
from that it looks sane.
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] submodule: no [--merge|--rebase] when newly cloned
2011-02-16 20:10 ` Jens Lehmann
@ 2011-02-16 21:41 ` Junio C Hamano
2011-02-17 0:21 ` Spencer E. Olson
0 siblings, 1 reply; 9+ messages in thread
From: Junio C Hamano @ 2011-02-16 21:41 UTC (permalink / raw)
To: Jens Lehmann; +Cc: Spencer E. Olson, git
Jens Lehmann <Jens.Lehmann@web.de> writes:
> So this looks like a worthwhile fix. The commit message and POSIX issue
> need to be addressed, tests would be a good thing to add too, but apart
> from that it looks sane.
Thanks for a translation ;-). I agree everthing you said above.
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] submodule: no [--merge|--rebase] when newly cloned
2011-02-16 21:41 ` Junio C Hamano
@ 2011-02-17 0:21 ` Spencer E. Olson
0 siblings, 0 replies; 9+ messages in thread
From: Spencer E. Olson @ 2011-02-17 0:21 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Jens Lehmann, git
I'll fix the POSIX issue and commit message and re-submit. (I wasn't aware
that for loops were from a bash-ism).
On Wednesday 16 February 2011 14:41, Junio C Hamano wrote:
> Jens Lehmann <Jens.Lehmann@web.de> writes:
> > So this looks like a worthwhile fix. The commit message and POSIX issue
> > need to be addressed, tests would be a good thing to add too, but apart
> > from that it looks sane.
>
> Thanks for a translation ;-). I agree everthing you said above.
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] submodule: no [--merge|--rebase] when newly cloned
2011-02-16 19:51 ` Junio C Hamano
2011-02-16 20:10 ` Jens Lehmann
@ 2011-02-17 7:10 ` Johannes Sixt
2011-02-18 0:34 ` Martin von Zweigbergk
1 sibling, 1 reply; 9+ messages in thread
From: Johannes Sixt @ 2011-02-17 7:10 UTC (permalink / raw)
To: Spencer E. Olson; +Cc: Junio C Hamano, git
Am 2/16/2011 20:51, schrieb Junio C Hamano:
> Here is how to write the above more concisely, efficiently and portably.
>
> case "$2;" in
> *";$1;"*)
> echo yes ;;
> *)
> echo no ;;
> esac
>
> The trailing ';' takes care of the case where cloned_modules has only one
> element, in which case you have ";name" in "$2". No need for a loop.
And while you are here, you could make this:
list_contains()
{
case "$2;" in
*";$1;"*)
: yes ;;
*)
! : no ;;
esac
}
and test for the exit code of this function rather than its output at the
call site.
-- Hannes
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] submodule: no [--merge|--rebase] when newly cloned
2011-02-17 7:10 ` Johannes Sixt
@ 2011-02-18 0:34 ` Martin von Zweigbergk
2011-02-18 0:48 ` Spencer E. Olson
0 siblings, 1 reply; 9+ messages in thread
From: Martin von Zweigbergk @ 2011-02-18 0:34 UTC (permalink / raw)
To: Johannes Sixt; +Cc: Spencer E. Olson, Junio C Hamano, git
On Thu, 17 Feb 2011, Johannes Sixt wrote:
> Am 2/16/2011 20:51, schrieb Junio C Hamano:
> > Here is how to write the above more concisely, efficiently and portably.
> >
> > case "$2;" in
> > *";$1;"*)
> > echo yes ;;
> > *)
> > echo no ;;
> > esac
> >
> > The trailing ';' takes care of the case where cloned_modules has only one
> > element, in which case you have ";name" in "$2". No need for a loop.
>
> And while you are here, you could make this:
>
> list_contains()
> {
> case "$2;" in
> *";$1;"*)
> : yes ;;
> *)
> ! : no ;;
> esac
> }
>
> and test for the exit code of this function rather than its output at the
> call site.
According to Brandon Casey: "Some platforms (IRIX 6.5, Solaris 7) do
not provide the 'yes' utility." See 8648732 (t/test-lib.sh: provide a
shell implementation of the 'yes' utility, 2009-08-28).
/Martin
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] submodule: no [--merge|--rebase] when newly cloned
2011-02-18 0:34 ` Martin von Zweigbergk
@ 2011-02-18 0:48 ` Spencer E. Olson
2011-02-18 0:59 ` Martin von Zweigbergk
0 siblings, 1 reply; 9+ messages in thread
From: Spencer E. Olson @ 2011-02-18 0:48 UTC (permalink / raw)
To: Martin von Zweigbergk; +Cc: Johannes Sixt, Junio C Hamano, git
We are not actually using the yes utility (think y\ny\ny\ny\ny\ny\n...)
As far as I understand, in this particular use, "yes" and "no" are just
comments. We could also write
: I like banannas ;;
and
! : I do not like anchovies ;;
On Thursday 17 February 2011 17:34, Martin von Zweigbergk wrote:
> On Thu, 17 Feb 2011, Johannes Sixt wrote:
> > Am 2/16/2011 20:51, schrieb Junio C Hamano:
> > > Here is how to write the above more concisely, efficiently and
> > > portably.
> > >
> > > case "$2;" in
> > > *";$1;"*)
> > > echo yes ;;
> > > *)
> > > echo no ;;
> > > esac
> > >
> > > The trailing ';' takes care of the case where cloned_modules has only
> > > one element, in which case you have ";name" in "$2". No need for a
> > > loop.
> >
> > And while you are here, you could make this:
> >
> > list_contains()
> > {
> > case "$2;" in
> > *";$1;"*)
> >
> > : yes ;;
> >
> > *)
> > ! : no ;;
> > esac
> > }
> >
> > and test for the exit code of this function rather than its output at the
> > call site.
>
> According to Brandon Casey: "Some platforms (IRIX 6.5, Solaris 7) do
> not provide the 'yes' utility." See 8648732 (t/test-lib.sh: provide a
> shell implementation of the 'yes' utility, 2009-08-28).
>
>
> /Martin
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] submodule: no [--merge|--rebase] when newly cloned
2011-02-18 0:48 ` Spencer E. Olson
@ 2011-02-18 0:59 ` Martin von Zweigbergk
0 siblings, 0 replies; 9+ messages in thread
From: Martin von Zweigbergk @ 2011-02-18 0:59 UTC (permalink / raw)
To: Spencer E. Olson
Cc: Martin von Zweigbergk, Johannes Sixt, Junio C Hamano, git
On Thu, 17 Feb 2011, Spencer E. Olson wrote:
> We are not actually using the yes utility (think y\ny\ny\ny\ny\ny\n...)
>
> As far as I understand, in this particular use, "yes" and "no" are just
> comments. We could also write
> : I like banannas ;;
> and
> ! : I do not like anchovies ;;
Ah, I'm sure you're right. Need to read up on that colon operator. Thanks.
>
> On Thursday 17 February 2011 17:34, Martin von Zweigbergk wrote:
> > On Thu, 17 Feb 2011, Johannes Sixt wrote:
> > > Am 2/16/2011 20:51, schrieb Junio C Hamano:
> > > > Here is how to write the above more concisely, efficiently and
> > > > portably.
> > > >
> > > > case "$2;" in
> > > > *";$1;"*)
> > > > echo yes ;;
> > > > *)
> > > > echo no ;;
> > > > esac
> > > >
> > > > The trailing ';' takes care of the case where cloned_modules has only
> > > > one element, in which case you have ";name" in "$2". No need for a
> > > > loop.
> > >
> > > And while you are here, you could make this:
> > >
> > > list_contains()
> > > {
> > > case "$2;" in
> > > *";$1;"*)
> > >
> > > : yes ;;
> > >
> > > *)
> > > ! : no ;;
> > > esac
> > > }
> > >
> > > and test for the exit code of this function rather than its output at the
> > > call site.
> >
> > According to Brandon Casey: "Some platforms (IRIX 6.5, Solaris 7) do
> > not provide the 'yes' utility." See 8648732 (t/test-lib.sh: provide a
> > shell implementation of the 'yes' utility, 2009-08-28).
> >
> >
> > /Martin
>
^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2011-02-18 0:59 UTC | newest]
Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2011-02-16 12:46 [PATCH] submodule: no [--merge|--rebase] when newly cloned Spencer E. Olson
2011-02-16 19:51 ` Junio C Hamano
2011-02-16 20:10 ` Jens Lehmann
2011-02-16 21:41 ` Junio C Hamano
2011-02-17 0:21 ` Spencer E. Olson
2011-02-17 7:10 ` Johannes Sixt
2011-02-18 0:34 ` Martin von Zweigbergk
2011-02-18 0:48 ` Spencer E. Olson
2011-02-18 0:59 ` Martin von Zweigbergk
Code repositories for project(s) associated with this public inbox
https://80x24.org/mirrors/git.git
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 read-only IMAP folder(s) and NNTP newsgroup(s).