threads / patch / 43284

patch, 5 partsRe: [PATCH 3/5] allow cloning a repository "shallowly"

Subject: Re: [PATCH 3/5] allow cloning a repository "shallowly"

## tl;dr

5 messages between Oct 30, 2006 and Nov 15, 2006. Diffs are folded; open one to read it.

replies: 4people: 2as markdown or json

Johannes Schindelin· Oct 30, 2006, 19:09 UTC · lore

[PATCH 3/5] allow cloning a repository "shallowly"

By specifying a depth, you can now clone a repository such that all fetched ancestor-chains' length is at most "depth". For example, if the upstream repository has only 2 branches ("A" and "B"), which are linear, and you specify depth 3, you will get A, A~1, A~2, A~3, B, B~1, B~2, and B~3. The ends are automatically made shallow commits.

Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
---
 fetch-pack.c  |   61 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++-
 git-clone.sh  |   19 +++++++++++++++--
 upload-pack.c |   21 ++++++++++++++++++-
 3 files changed, 96 insertions(+), 5 deletions(-)
Show changes to 3 files +96 −5

fetch-pack.c, git-clone.sh, upload-pack.c

diff --git a/fetch-pack.c b/fetch-pack.c
index 488adc9..9619d6e 100644
--- a/fetch-pack.c
+++ b/fetch-pack.c
@@ -8,8 +8,9 @@ static int keep_pack;
 static int quiet;
 static int verbose;
 static int fetch_all;
+static int depth;
 static const char fetch_pack_usage[] =
-"git-fetch-pack [--all] [-q] [-v] [-k] [--thin] [--exec=upload-pack] [host:]directory <refs>...";
+"git-fetch-pack [--all] [-q] [-v] [-k] [--thin] [--exec=upload-pack] [--depth=<n>] [host:]directory <refs>...";
 static const char *exec = "git-upload-pack";
 
 #define COMPLETE	(1U << 0)
@@ -179,10 +180,29 @@ static int find_common(int fd[2], unsign
 	}
 	if (is_repository_shallow())
 		write_shallow_commits(fd[1], 1);
+	if (depth > 0)
+		packet_write(fd[1], "deepen %d", depth);
 	packet_flush(fd[1]);
 	if (!fetching)
 		return 1;
 
+	if (depth >  0) {
+		char line[1024];
+		unsigned char sha1[20];
+		int len;
+
+		while ((len = packet_read_line(fd[0], line, sizeof(line)))) {
+			if (!strncmp("shallow ", line, 8)) {
+				if (get_sha1_hex(line + 8, sha1))
+					die("invalid shallow line: %s", line);
+				/* no need making it shallow if we have it already */
+				if (lookup_object(sha1))
+					continue;
+				register_shallow(sha1);
+			}
+		}
+	}
+
 	flushes = 0;
 	retval = -1;
 	while ((sha1 = get_rev())) {
@@ -480,6 +500,8 @@ int main(int argc, char **argv)
 	char *dest = NULL, **heads;
 	int fd[2];
 	pid_t pid;
+	struct stat st;
+	struct lock_file lock;
 
 	setup_git_directory();
 
@@ -513,6 +535,12 @@ int main(int argc, char **argv)
 				verbose = 1;
 				continue;
 			}
+			if (!strncmp("--depth=", arg, 8)) {
+				depth = strtol(arg + 8, NULL, 0);
+				if (stat(git_path("shallow"), &st))
+					st.st_mtime = 0;
+				continue;
+			}
 			usage(fetch_pack_usage);
 		}
 		dest = arg;
@@ -522,6 +550,8 @@ int main(int argc, char **argv)
 	}
 	if (!dest)
 		usage(fetch_pack_usage);
+	if (is_repository_shallow() && depth > 0)
+		die("Deepening of a shallow repository not yet supported!");
 	pid = git_connect(fd, dest, exec);
 	if (pid < 0)
 		return 1;
@@ -543,5 +573,34 @@ int main(int argc, char **argv)
 			}
 	}
 
+	if (!ret && depth > 0) {
+		struct cache_time mtime;
+		char *shallow = git_path("shallow");
+		int fd;
+
+		mtime.sec = st.st_mtime;
+#ifdef USE_NSEC
+		mtime.usec = st.st_mtim.usec;
+#endif
+		if (stat(shallow, &st)) {
+			if (mtime.sec)
+				die("shallow file was removed during fetch");
+		} else if (st.st_mtime != mtime.sec
+#ifdef USE_NSEC
+				|| st.st_mtim.usec != mtime.usec
+#endif
+			  )
+			die("shallow file was changed during fetch");
+
+		fd = hold_lock_file_for_update(&lock, shallow, 1);
+		if (!write_shallow_commits(fd, 0)) {
+			unlink(lock.filename);
+			rollback_lock_file(&lock);
+		} else {
+			close(fd);
+			commit_lock_file(&lock);
+		}
+	}
+
 	return !!ret;
 }
diff --git a/git-clone.sh b/git-clone.sh
index 3f006d1..595c070 100755
--- a/git-clone.sh
+++ b/git-clone.sh
@@ -14,7 +14,7 @@ die() {
 }
 
 usage() {
-	die "Usage: $0 [--template=<template_directory>] [--use-separate-remote] [--reference <reference-repo>] [--bare] [-l [-s]] [-q] [-u <upload-pack>] [--origin <name>] [-n] <repo> [<dir>]"
+	die "Usage: $0 [--template=<template_directory>] [--use-separate-remote] [--reference <reference-repo>] [--bare] [-l [-s]] [-q] [-u <upload-pack>] [--origin <name>] [--depth <n>] [-n] <repo> [<dir>]"
 }
 
 get_repo_base() {
@@ -116,6 +116,7 @@ reference=
 origin=
 origin_override=
 use_separate_remote=
+depth=
 while
 	case "$#,$1" in
 	0,*) break ;;
@@ -158,6 +159,10 @@ while
 	*,-u|*,--upload-pack)
 		shift
 		upload_pack="--exec=$1" ;;
+	1,--depth) usage;;
+	*,--depth)
+		shift
+		depth="--depth=$1";;
 	*,-*) usage ;;
 	*) break ;;
 	esac
@@ -265,6 +270,10 @@ yes,yes)
 *)
 	case "$repo" in
 	rsync://*)
+		case "$depth" in
+		"") ;;
+		*) die "shallow over rsync not supported" ;;
+		esac
 		rsync $quiet -av --ignore-existing  \
 			--exclude info "$repo/objects/" "$GIT_DIR/objects/" ||
 		exit
@@ -293,6 +302,10 @@ yes,yes)
 		git-ls-remote "$repo" >"$GIT_DIR/CLONE_HEAD" || exit 1
 		;;
 	https://*|http://*|ftp://*)
+		case "$depth" in
+		"") ;;
+		*) die "shallow over http or ftp not supported" ;;
+		esac
 		if test -z "@@NO_CURL@@"
 		then
 			clone_dumb_http "$repo" "$D"
@@ -302,8 +315,8 @@ yes,yes)
 		;;
 	*)
 		case "$upload_pack" in
-		'') git-fetch-pack --all -k $quiet "$repo" ;;
-		*) git-fetch-pack --all -k $quiet "$upload_pack" "$repo" ;;
+		'') git-fetch-pack --all -k $quiet $depth "$repo" ;;
+		*) git-fetch-pack --all -k $quiet "$upload_pack" $depth "$repo" ;;
 		esac >"$GIT_DIR/CLONE_HEAD" ||
 			die "fetch-pack from '$repo' failed."
 		;;
diff --git a/upload-pack.c b/upload-pack.c
index 8dd6121..ebe1e5a 100644
--- a/upload-pack.c
+++ b/upload-pack.c
@@ -488,7 +488,7 @@ static void receive_needs(void)
 {
 	struct object_array shallows = {0, 0, NULL};
 	static char line[1000];
-	int len;
+	int len, depth = 0;
 
 	for (;;) {
 		struct object *o;
@@ -509,6 +509,13 @@ static void receive_needs(void)
 			add_object_array(object, NULL, &shallows);
 			continue;
 		}
+		if (!strncmp("deepen ", line, 7)) {
+			char *end;
+			depth = strtol(line + 7, &end, 0);
+			if (end == line + 7 || depth <= 0)
+				die("Invalid deepen: %s", line);
+			continue;
+		}
 		if (strncmp("want ", line, 5) ||
 		    get_sha1_hex(line+5, sha1_buf))
 			die("git-upload-pack: protocol error, "
@@ -540,6 +547,18 @@ static void receive_needs(void)
 			add_object_array(o, NULL, &want_obj);
 		}
 	}
+	if (depth > 0) {
+		struct commit_list *result, *backup;
+		if (shallows.nr > 0)
+			die("Deepening a shallow repository not yet supported");
+		backup = result = get_shallow_commits(&want_obj, depth);
+		while (result) {
+			packet_write(1, "shallow %s",
+					sha1_to_hex(result->item->object.sha1));
+			result = result->next;
+		}
+		free_commit_list(backup);
+	}
 	if (shallows.nr > 0) {
 		int i;
 		for (i = 0; i < shallows.nr; i++)
-- 
1.4.3.3.gca42
Junio C Hamano· Nov 14, 2006, 07:24 UTC · re: Johannes Schindelin · lore
Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:
Show 30 quoted lines
> By specifying a depth, you can now clone a repository such that
> all fetched ancestor-chains' length is at most "depth". For example,
> if the upstream repository has only 2 branches ("A" and "B"), which
> are linear, and you specify depth 3, you will get A, A~1, A~2, A~3,
> B, B~1, B~2, and B~3. The ends are automatically made shallow
> commits.
>
> Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
> ---
>  fetch-pack.c  |   61 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++-
>  git-clone.sh  |   19 +++++++++++++++--
>  upload-pack.c |   21 ++++++++++++++++++-
>  3 files changed, 96 insertions(+), 5 deletions(-)
>
> diff --git a/fetch-pack.c b/fetch-pack.c
> index 488adc9..9619d6e 100644
> --- a/fetch-pack.c
> +++ b/fetch-pack.c
>...
> +		while ((len = packet_read_line(fd[0], line, sizeof(line)))) {
> +			if (!strncmp("shallow ", line, 8)) {
> +				if (get_sha1_hex(line + 8, sha1))
> +					die("invalid shallow line: %s", line);
> +				/* no need making it shallow if we have it already */
> +				if (lookup_object(sha1))
> +					continue;
> +				register_shallow(sha1);
> +			}
> +		}
> +	}

I understand "no need making it shallow", but I am not sure if a non-NULL return from lookup_object() tells us that.

Before this part of the code, we have scanned the local refs in everything_local (which parses their refs).

I think register_shallow() can take commits that are already shallow() so maybe we can remove this "if() continue"?

Johannes Schindelin· Nov 14, 2006, 10:49 UTC · re: Junio C Hamano · lore
Hi,
On Mon, 13 Nov 2006, Junio C Hamano wrote:
Show 35 quoted lines
> Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:
> 
> > By specifying a depth, you can now clone a repository such that
> > all fetched ancestor-chains' length is at most "depth". For example,
> > if the upstream repository has only 2 branches ("A" and "B"), which
> > are linear, and you specify depth 3, you will get A, A~1, A~2, A~3,
> > B, B~1, B~2, and B~3. The ends are automatically made shallow
> > commits.
> >
> > Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
> > ---
> >  fetch-pack.c  |   61 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++-
> >  git-clone.sh  |   19 +++++++++++++++--
> >  upload-pack.c |   21 ++++++++++++++++++-
> >  3 files changed, 96 insertions(+), 5 deletions(-)
> >
> > diff --git a/fetch-pack.c b/fetch-pack.c
> > index 488adc9..9619d6e 100644
> > --- a/fetch-pack.c
> > +++ b/fetch-pack.c
> >...
> > +		while ((len = packet_read_line(fd[0], line, sizeof(line)))) {
> > +			if (!strncmp("shallow ", line, 8)) {
> > +				if (get_sha1_hex(line + 8, sha1))
> > +					die("invalid shallow line: %s", line);
> > +				/* no need making it shallow if we have it already */
> > +				if (lookup_object(sha1))
> > +					continue;
> > +				register_shallow(sha1);
> > +			}
> > +		}
> > +	}
> 
> I understand "no need making it shallow", but I am not sure if a
> non-NULL return from lookup_object() tells us that.
You are probably right, how about has_sha1_file()?
> I think register_shallow() can take commits that are already shallow() 
> so maybe we can remove this "if() continue"?

Yes, it can, but that is not necessarily correct: since .git/shallow is constructed from the registered shallow commits, we would make a commit shallow which is really not shallow.

So, how about
> > +				if (lookup_object(sha1) || has_sha1_file(sha1))
> > +					continue;

Ciao, Dscho

Junio C Hamano· Nov 14, 2006, 17:46 UTC · re: Johannes Schindelin · lore
Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:
Show 16 quoted lines
>> I understand "no need making it shallow", but I am not sure if a
>> non-NULL return from lookup_object() tells us that.
>
> You are probably right, how about has_sha1_file()?
>
>> I think register_shallow() can take commits that are already shallow() 
>> so maybe we can remove this "if() continue"?
>
> Yes, it can, but that is not necessarily correct: since .git/shallow is 
> constructed from the registered shallow commits, we would make a commit 
> shallow which is really not shallow.
>
> So, how about
>
>> > +				if (lookup_object(sha1) || has_sha1_file(sha1))
>> > +					continue;

If I understand the code correctly, this loop is reading what the other side thinks your shallows should be (based on your earlier "deepen" request or if this is initial fetch based on your depth). Even if we already have that object, if that object _is_ shallow on our end, don't we need to keep it marked as shallow? Will we get ancestors of this commit from the other end (and "shallow" lines for some of them to properly cauterize the chain)?

Johannes Schindelin· Nov 15, 2006, 10:57 UTC · re: Junio C Hamano · lore
Hi,
On Tue, 14 Nov 2006, Junio C Hamano wrote:
Show 27 quoted lines
> Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:
> 
> >> I understand "no need making it shallow", but I am not sure if a
> >> non-NULL return from lookup_object() tells us that.
> >
> > You are probably right, how about has_sha1_file()?
> >
> >> I think register_shallow() can take commits that are already shallow() 
> >> so maybe we can remove this "if() continue"?
> >
> > Yes, it can, but that is not necessarily correct: since .git/shallow is 
> > constructed from the registered shallow commits, we would make a commit 
> > shallow which is really not shallow.
> >
> > So, how about
> >
> >> > +				if (lookup_object(sha1) || has_sha1_file(sha1))
> >> > +					continue;
> 
> If I understand the code correctly, this loop is reading what
> the other side thinks your shallows should be (based on your
> earlier "deepen" request or if this is initial fetch based on
> your depth).  Even if we already have that object, if that
> object _is_ shallow on our end, don't we need to keep it marked
> as shallow?  Will we get ancestors of this commit from the other
> end (and "shallow" lines for some of them to properly cauterize
> the chain)?

My thinking was: if we have that object already, and it is _not_ marked as shallow, we probably have either all its ancestors, or at least all ancestors up until shallow commit(s).

Thinking about it again, it seems very fragile: on purpose, we only trust to have the objects which are reachable by at least one ref, so that an interrupted fetch does not corrupt the repository.

So yes, I agree, that "if() continue;" should go away. Even if all of a sudden, commits we already have are no longer reachable.

Ciao, Dscho

← back to recent threads