[PATCH] submodule update - don't run git-fetch if sha1 available

[PATCH] submodule update - don't run git-fetch if sha1 available

[Torgil Svensson]

It's un-necessary (both time-consuming and annoying if you are
disconnected) for git submodule update to always running git-fetch
prior to the detached head checkout of the submodule if the commit
already exists.

Signed-off-by: Torgil Svensson <torgil.svensson@gmail.com>
---
 git-submodule.sh |    9 +++++++--
 1 files changed, 7 insertions(+), 2 deletions(-)

diff --git a/git-submodule.sh b/git-submodule.sh
index 2cfeadd..dc2ed4b 100755
--- a/git-submodule.sh
+++ b/git-submodule.sh
@@ -39,6 +39,11 @@ get_repo_base() {
        ) 2>/dev/null
 }

+has_sha1()
+{
+       git rev-list 2>/dev/null --max-count=0 $1
+}
+
 #
 # Map submodule path to submodule name
 #
@@ -193,9 +198,9 @@ modules_update()

                if test "$subsha1" != "$sha1"
                then
-                       (unset GIT_DIR && cd "$path" && git-fetch &&
+                       (unset GIT_DIR && cd "$path" && (has_sha1
"$sha1" || git-fetch) &&
                                git-checkout -q "$sha1") ||
-                       die "Unable to checkout '$sha1' in submodule
path '$path'"
+                       die "Unable to checkout '$sha1' in submodule
path '$path'"

                        say "Submodule path '$path': checked out '$sha1'"
                fi
--
1.5.3.rc4.730.ga8c3-dirty

Re: [PATCH] submodule update - don't run git-fetch if sha1 available

[Torgil Svensson]

Hi

Sorry for a broken patch [inserted line-feeds]. If anyone knows how to
disable that in gmail web client, please let me know.

I also tried to follow "Using msmtp to send your patches"
[http://git.or.cz/gitwiki/GitTips ] on msys/mingw/WindowsXP with the
following result (at best) from git-send-email:

"Can't call method "mail" on an undefined value at
C:/msysgit/git/git-send-email line 551."

So I would appreciate if someone that has used the combination gmail
SMTP+Windows+git-send-email successfully (cygwin,msys or "pure") to
let us know how you did it.

Best Regards,

//Torgil


On 8/11/07, Torgil Svensson <torgil.svensson@gmail.com> wrote:
> It's un-necessary (both time-consuming and annoying if you are
> disconnected) for git submodule update to always running git-fetch
> prior to the detached head checkout of the submodule if the commit
> already exists.
>
> Signed-off-by: Torgil Svensson <torgil.svensson@gmail.com>
> ---
>  git-submodule.sh |    9 +++++++--
>  1 files changed, 7 insertions(+), 2 deletions(-)
>
> diff --git a/git-submodule.sh b/git-submodule.sh
> index 2cfeadd..dc2ed4b 100755
> --- a/git-submodule.sh
> +++ b/git-submodule.sh
> @@ -39,6 +39,11 @@ get_repo_base() {
>         ) 2>/dev/null
>  }
>
> +has_sha1()
> +{
> +       git rev-list 2>/dev/null --max-count=0 $1
> +}
> +
>  #
>  # Map submodule path to submodule name
>  #
> @@ -193,9 +198,9 @@ modules_update()
>
>                 if test "$subsha1" != "$sha1"
>                 then
> -                       (unset GIT_DIR && cd "$path" && git-fetch &&
> +                       (unset GIT_DIR && cd "$path" && (has_sha1
> "$sha1" || git-fetch) &&
>                                 git-checkout -q "$sha1") ||
> -                       die "Unable to checkout '$sha1' in submodule
> path '$path'"
> +                       die "Unable to checkout '$sha1' in submodule
> path '$path'"
>
>                         say "Submodule path '$path': checked out '$sha1'"
>                 fi
> --
> 1.5.3.rc4.730.ga8c3-dirty
>

Re: [PATCH] submodule update - don't run git-fetch if sha1 available

[Junio C Hamano]

"Torgil Svensson" <torgil.svensson@gmail.com> writes:

> It's un-necessary (both time-consuming and annoying if you are
> disconnected) for git submodule update to always running git-fetch
> prior to the detached head checkout of the submodule if the commit
> already exists.

This is wrong.  Existence of the commit object alone does not
mean the necessary tree and blob objects to check out that
commit, let alone all the history that leads to the commit,
exist in the repository (think of a commit walker fetch that was
interrupted in the middle).  You need to make sure that the
commit exists *AND* is reachable from one of the refs.

Re: [PATCH] submodule update - don't run git-fetch if sha1 available

[Torgil Svensson]

On 8/11/07, Junio C Hamano <gitster@pobox.com> wrote:

> This is wrong.  Existence of the commit object alone does not
> mean the necessary tree and blob objects to check out that
> commit, let alone all the history that leads to the commit,
> exist in the repository (think of a commit walker fetch that was
> interrupted in the middle).  You need to make sure that the
> commit exists *AND* is reachable from one of the refs.

That made sense. Good point. Consider this case:

$ git clone <superproject>
$ git submodule init
$ git submodule update
$ cd <submodule>
$ git checkout master
$ cd ..
$ git status
Modified <submodule>
$ git submodule update

Do we know in this state that the ref can be reached from a reference?
Say you've managed to do this:

$ cd <submodule>
$ git checkout master
$ work.. commit .. work ..commit
$ cd ..
$ git add <submodule>
$ git commit
$ cd <submodule>
$ git reset --hard HEAD~2

Is it okay to fail the supermodule update in this state? Obviously
we've thrown away things for a purpose.

//Torgil

Re: [PATCH] submodule update - don't run git-fetch if sha1 available

[Junio C Hamano]

"Torgil Svensson" <torgil.svensson@gmail.com> writes:

> Do we know in this state that the ref can be reached from a reference?
> Say you've managed to do this:
>
> $ cd <submodule>
> $ git checkout master
> $ work.. commit .. work ..commit
> $ cd ..
> $ git add <submodule>
> $ git commit
> $ cd <submodule>
> $ git reset --hard HEAD~2
>
> Is it okay to fail the supermodule update in this state? Obviously
> we've thrown away things for a purpose.

It would make more sense if the last command you ran in the
subproject directory were "git checkout HEAD~2".  If you have
checked out the subproject in the context of the superproject,
you have no business *resetting* its branches.  Then the updated
HEAD is reachable from the branch tip.

But even when you did "reset --hard" to rewind, you _could_
avoid fetching if you wanted to.  In fact, this alternative
technique is probably needed if you built new commits on top of
existing refs of the 'origin' repository of subproject, or even
on a detached HEAD.

What you need to prove is that whatever commit the superproject
wants to check out can be checked out.  Strictly speaking, the
commit does not even have to have histories leading to it in the
repository.  So in addition to the check you initially proposed
in your patch (by the way, I would do "git cat-file -t $commit"
and see if it is a commit object, instead of "rev-list" of count
zero), minimally you need to prove that all trees and blobs
exist in the repository.

That can be done from the script level by doing something silly
like:

	git archive "$commit^{tree}" >/dev/null

To make it more efficient (the above proves all the trees and
blobs exist by actually extracting all of them, which is an
unnecessary work), you may want to add a more lightweight
built-in command to do so.  Perhaps...

-- >8 --
Subject: ls-tree --check

You can run "ls-tree --check" to make sure that all necessary
objects exist in the repository to complete a tree object.  This
may be useful to make sure that the subproject commit bound to a
superproject index can be checked out in its entirety.

---

 builtin-ls-tree.c |   21 +++++++++++++++++----
 1 files changed, 17 insertions(+), 4 deletions(-)

diff --git a/builtin-ls-tree.c b/builtin-ls-tree.c
index cb4be4f..4a19e86 100644
--- a/builtin-ls-tree.c
+++ b/builtin-ls-tree.c
@@ -16,6 +16,7 @@ static int line_termination = '\n';
 #define LS_SHOW_TREES 4
 #define LS_NAME_ONLY 8
 #define LS_SHOW_SIZE 16
+#define LS_CHECK 32
 static int abbrev;
 static int ls_options;
 static const char **pathspec;
@@ -23,7 +24,7 @@ static int chomp_prefix;
 static const char *ls_tree_prefix;
 
 static const char ls_tree_usage[] =
-	"git-ls-tree [-d] [-r] [-t] [-l] [-z] [--name-only] [--name-status] [--full-name] [--abbrev[=<n>]] <tree-ish> [path...]";
+	"git-ls-tree [-d] [-r] [-t] [-l] [-z] [--check] [--name-only] [--name-status] [--full-name] [--abbrev[=<n>]] <tree-ish> [path...]";
 
 static int show_recursive(const char *base, int baselen, const char *pathname)
 {
@@ -107,6 +108,16 @@ static int show_tree(const unsigned char *sha1, const char *base, int baselen,
 				       abbrev ? find_unique_abbrev(sha1, abbrev)
 				              : sha1_to_hex(sha1),
 				       '-');
+		} else if (ls_options & LS_CHECK) {
+			if (S_ISGITLINK(mode))
+				/*
+				 * subproject commit does not have to be
+				 * in this repository.
+				 */
+				; /* noop */
+			else if (sha1_object_info(sha1, &size) < 0)
+				retval = -1;
+			return retval;
 		} else
 			printf("%06o %s %s\t", mode, type,
 			       abbrev ? find_unique_abbrev(sha1, abbrev)
@@ -171,6 +182,10 @@ int cmd_ls_tree(int argc, const char **argv, const char *prefix)
 				abbrev = DEFAULT_ABBREV;
 				break;
 			}
+			if (!strcmp(argv[1]+2, "check")) {
+				ls_options = LS_CHECK|LS_RECURSIVE;
+				break;
+			}
 			/* otherwise fallthru */
 		default:
 			usage(ls_tree_usage);
@@ -191,7 +206,5 @@ int cmd_ls_tree(int argc, const char **argv, const char *prefix)
 	tree = parse_tree_indirect(sha1);
 	if (!tree)
 		die("not a tree object");
-	read_tree_recursive(tree, "", 0, 0, pathspec, show_tree);
-
-	return 0;
+	return read_tree_recursive(tree, "", 0, 0, pathspec, show_tree);
 }

Re: [PATCH] submodule update - don't run git-fetch if sha1 available

[Torgil Svensson]

On 8/12/07, Junio C Hamano <gitster@pobox.com> wrote:

> unnecessary work), you may want to add a more lightweight
> built-in command to do so.  Perhaps...
>
> -- >8 --
> Subject: ls-tree --check
>
> You can run "ls-tree --check" to make sure that all necessary
> objects exist in the repository to complete a tree object.  This
> may be useful to make sure that the subproject commit bound to a
> superproject index can be checked out in its entirety.

This looks good, and the subproject part of it should be trivial to
implement but i'm struggling with applying your patch.

If I copy the whole mail with headers (I get them with a menu item
"show original" in gmail web client), paste it into a file ("*p in
vim) I get the following:

$ git apply junio.mail
Adds trailing whitespace.
junio.mail:103:#define LS_CHECK 32
Adds trailing whitespace.
junio.mail:112: "git-ls-tree [-d] [-r] [-t] [-l] [-z] [--check]
[--name-only] [--name-status] [--full-name] [--abbrev[=<n>]] <tree-i
sh> [path...]";
Adds trailing whitespace.
junio.mail:120:         } else if (ls_options & LS_CHECK) {
Adds trailing whitespace.
junio.mail:121:                 if (S_ISGITLINK(mode))
Adds trailing whitespace.
junio.mail:122:                         /*
error: patch failed: builtin-ls-tree.c:16
error: builtin-ls-tree.c: patch does not apply

git am gives me similar behaviour.

$ git am junio.mail

Applying submodule update - don't run git-fetch if sha1 available

Adds trailing whitespace.
.dotest/patch:14:#define LS_CHECK 32
Adds trailing whitespace.
.dotest/patch:23:       "git-ls-tree [-d] [-r] [-t] [-l] [-z]
[--check] [--name-only] [--name-status] [--full-name] [--abbrev[=<n>]]
 <tree-ish> [path...]";
Adds trailing whitespace.
.dotest/patch:31:               } else if (ls_options & LS_CHECK) {
Adds trailing whitespace.
.dotest/patch:32:                       if (S_ISGITLINK(mode))
Adds trailing whitespace.
.dotest/patch:33:                               /*
error: patch failed: builtin-ls-tree.c:16
error: builtin-ls-tree.c: patch does not apply
Patch failed at 0001.
When you have resolved this problem run "git-am --resolved".
If you would prefer to skip this patch, instead run "git-am --skip".



Is the "-- >8 --" -line something git-apply looks for?

Short of manual patching, abandoning the web-client or switch to
another mail-account, is there any simple way to get around the above
issues ?    [I'm trying to do this on windows/msysgit]


Thank you for good support on this list!

//Torgil

Re: [PATCH] submodule update - don't run git-fetch if sha1 available

[Junio C Hamano]

"Torgil Svensson" <torgil.svensson@gmail.com> writes:

> $ git apply junio.mail

I have no idea how you saved the junio.mail file.  "git am"
knows how to grok e-mails in mbox and maildir formats.

> ...
> Is the "-- >8 --" -line something git-apply looks for?

git-apply does not have anything to do with any of this.  You
are looking for "git am".

"git-am" does not currently pay attention to the "-- >8 --"
marker, either.  It is there primarily so that you can do the
interactive "git am -i" so that you know up to which point you
would want to remove with its [e]dit subcommand.