RFC: Allow missing objects during packing

RFC: Allow missing objects during packing

[Shawn O. Pearce]

So I have this horribly bizarre case that I just feel dirty telling
you about.  But I have to deal with it.  So here goes...

I want to publish a fork of an open source project, but I'm not
allowed to publish the base history of the project.  For political
reasons I can only publish patches/deltas, even though the project
is licensed under the LGPL.  Don't ask.  Seriously.  In 6 months
everyone will probably forget about this and I can just publish
the whole thing.  But not right now.

With this silly little patch to builtin-pack-objects I can make
it work by publishing what amounts to a shallow clone.  The fork
was created by doing something like this:

	$ git clone --bare git://upstream.org/proj.git upstream.git
	$ cd fork
	$ git remote add -f upstream ../upstream.git
	$ echo ../upstream.git/objects >.git/objects/info/alternates
	$ git repack -a -d -f -l

	# now fork/.git has only its "delta"
	# add the base trees, but not blobs
	#
	$ jgit --git-dir=../upstream.git tree-copy upstream/master .

	# remove the shared odb
	#
    $ rm .git/objects/info/alternates

Users who want to clone from fork.git have to do this:

	$ git clone -o upstream git://upstream.org/proj.git proj
	$ cd proj
	$ git remote add -f origin git://forky.org/fork.git
	remote: Counting objects: 5, done.
	remote: error: unable to find d30dcb7f07b6bf1a6cc5da4f3c1d0e6fc690dc45
	remote: git: unable to get type of object d30dremote: cb7f07b6bf1a6cc5da4f3c1d0e6fc690dc45: No such file or directory
	remote: Compressing objects: 100% (3/3), done.
	remote: Total 3 (delta 1), reused 0 (delta 0)
	Unpacking objects: 100% (3/3), done.

Basically this little patch just makes pack-objects issue the
warning, but then skip over the object and move on.

Later during deltification the selected base object has a type which
does not match any other object, so it never gets considered, and
there is no error caused by it not being present.  We just cannot
generate a delta during the fetch and the client winds up getting
a little larger download.

Thoughts?  Should I work this up into a real patch and try to get
it into the tree?  It seems pretty harmless to allow an object we
aren't going to transmit but that we want to use as a delta base
in a thin pack to be missing.  At worst we just get a little bit
more data transfer.

--8<--
diff --git a/builtin-pack-objects.c b/builtin-pack-objects.c
index 2dadec1..0f29f14 100644
--- a/builtin-pack-objects.c
+++ b/builtin-pack-objects.c
@@ -1097,7 +1097,7 @@ static void check_object(struct object_entry *entry)
 
 	entry->type = sha1_object_info(entry->idx.sha1, &entry->size);
 	if (entry->type < 0)
-		die("unable to get type of object %s",
+		warn("unable to get type of object %s",
 		    sha1_to_hex(entry->idx.sha1));
 }


And this is the Java program to build up the base tree, as otherwise
the thin pack creation code crashes out trying to build a list of
what the client already has/knows.  Since this is only the base
commit and its trees (no blobs, and no history beyond the base
commit) I should be able to get away with publishing it as part of
the fork.

--8<--
import org.kohsuke.args4j.Argument;
import org.spearce.jgit.lib.Constants;
import org.spearce.jgit.lib.ObjectId;
import org.spearce.jgit.lib.PackWriter;
import org.spearce.jgit.lib.Repository;
import org.spearce.jgit.lib.TextProgressMonitor;
import org.spearce.jgit.pgm.Command;
import org.spearce.jgit.pgm.TextBuiltin;
import org.spearce.jgit.revwalk.RevCommit;
import org.spearce.jgit.revwalk.RevObject;
import org.spearce.jgit.treewalk.TreeWalk;

import java.io.BufferedOutputStream;
import java.io.File;
import java.io.FileOutputStream;
import java.util.LinkedList;
import java.util.List;

@Command(name = "tree-copy", usage = "Copy a base commit and its trees")
public class TreeCopy extends TextBuiltin {
  @Argument(index = 0, required = true, metaVar = "commit", usage = "base commit")
  private RevCommit baseCommit;

  @Argument(index = 1, required = true, metaVar = "dir", usage = "destination repository")
  private File destGitDir;

  @Override
  protected void run() throws Exception {
    final List<RevObject> toCopy = new LinkedList<RevObject>();

    toCopy.add(baseCommit);
    toCopy.add(baseCommit.getTree());

    final TreeWalk tw = new TreeWalk(db);
    tw.reset();
    tw.addTree(baseCommit.getTree());

    while (tw.next()) {
      switch (tw.getFileMode(0).getObjectType()) {
        case Constants.OBJ_TREE:
          toCopy.add(argWalk.lookupTree(tw.getObjectId(0)));
          tw.enterSubtree();
          break;
        default:
          break;
      }
    }

    final Repository destdb = new Repository(destGitDir);
    final PackWriter packer = new PackWriter(db, new TextProgressMonitor());
    packer.preparePack(toCopy.iterator());
    final String packName = "pack-" + packer.computeName();
    final File packDir = new File(destdb.getDirectory(), "objects/pack");
    final File packPath = new File(packDir, packName + ".pack");
    final File idxPath = new File(packDir, packName + ".idx");

    if (packPath.exists() && idxPath.exists())
      throw die(packPath.getName() + " already exists.");

    {
      final BufferedOutputStream os =
          new BufferedOutputStream(new FileOutputStream(packPath));
      packer.writePack(os);
      os.close();
      packPath.setReadOnly();
    }
    {
      final BufferedOutputStream os =
          new BufferedOutputStream(new FileOutputStream(idxPath));
      packer.writeIndex(os);
      os.close();
      idxPath.setReadOnly();
    }
  }
}
 
-- 
Shawn.

Re: RFC: Allow missing objects during packing

[Junio C Hamano]

"Shawn O. Pearce" <spearce@spearce.org> writes:

> ... It seems pretty harmless to allow an object we
> aren't going to transmit but that we want to use as a delta base
> in a thin pack to be missing.  At worst we just get a little bit
> more data transfer.

If the check is only about a thin delta base that is not going to be
transmit, I'd agree.  But I do not see how you are distinguishing that
case and the case where an object you are actually sending is missing (in
which case we would want to error out, wouldn't we?)

Re: RFC: Allow missing objects during packing

[Shawn O. Pearce]

Junio C Hamano <gitster@pobox.com> wrote:
> "Shawn O. Pearce" <spearce@spearce.org> writes:
> 
> > ... It seems pretty harmless to allow an object we
> > aren't going to transmit but that we want to use as a delta base
> > in a thin pack to be missing.  At worst we just get a little bit
> > more data transfer.
> 
> If the check is only about a thin delta base that is not going to be
> transmit, I'd agree.  But I do not see how you are distinguishing that
> case and the case where an object you are actually sending is missing (in
> which case we would want to error out, wouldn't we?)

Arrgh.  Good catch.  My patch is flawed in that it does not correctly
fail if we really needed the missing object in this output pack.

I don't think that would be hard to fix.  I'll respin something
shortly.

-- 
Shawn.

Re: RFC: Allow missing objects during packing

[Shawn O. Pearce]

> Junio C Hamano <gitster@pobox.com> wrote:
> > If the check is only about a thin delta base that is not going to be
> > transmit, I'd agree.  But I do not see how you are distinguishing that
> > case and the case where an object you are actually sending is missing (in
> > which case we would want to error out, wouldn't we?)

Turns out to be pretty simple I think.  We just delay the
error handling for ->type < 0 until write_object().  If we
get this far we know we wanted to include the object but
we really don't have it.  Up until that point its fine
for us to get objects which are missing, we'll just wind
up with a suboptimal pack.

We also don't even need to report the error from sha1_object_info
as it already issues an error message (see sha1_loose_object_info).

--8<--
pack-objects: Allow missing base objects when creating thin packs

If we are building a thin pack and one of the base objects we would
consider for deltification is missing its OK, the other side already
has that base object.  We may be able to get a delta from another
object, or we can simply send the new object whole (no delta).

This allows a shallow clone which may have only commits and trees
(but only partial blobs) to generate a pack for a fetch client,
so the shallow clone only needs to contain objects that are not
in the common base.

Signed-off-by: Shawn O. Pearce <spearce@spearce.org>
---
 builtin-pack-objects.c |    5 ++---
 1 files changed, 2 insertions(+), 3 deletions(-)

diff --git a/builtin-pack-objects.c b/builtin-pack-objects.c
index 2dadec1..187cb19 100644
--- a/builtin-pack-objects.c
+++ b/builtin-pack-objects.c
@@ -243,6 +243,8 @@ static unsigned long write_object(struct sha1file *f,
 		crc32_begin(f);
 
 	type = entry->type;
+	if (type < 0)
+		die("unable to read object %s", sha1_to_hex(entry->idx.sha1));
 
 	/* write limit if limited packsize and not first object */
 	limit = pack_size_limit && nr_written ?
@@ -1096,9 +1098,6 @@ static void check_object(struct object_entry *entry)
 	}
 
 	entry->type = sha1_object_info(entry->idx.sha1, &entry->size);
-	if (entry->type < 0)
-		die("unable to get type of object %s",
-		    sha1_to_hex(entry->idx.sha1));
 }
 
 static int pack_offset_sort(const void *_a, const void *_b)
-- 
1.6.0.rc2.22.g71b99

-- 
Shawn.

Re: RFC: Allow missing objects during packing

[Junio C Hamano]

"Shawn O. Pearce" <spearce@spearce.org> writes:

> If we are building a thin pack and one of the base objects we would
> consider for deltification is missing its OK, the other side already
> has that base object.  We may be able to get a delta from another
> object, or we can simply send the new object whole (no delta).
>
> This allows a shallow clone which may have only commits and trees
> (but only partial blobs) to generate a pack for a fetch client,
> so the shallow clone only needs to contain objects that are not
> in the common base.

I'd queue this to 'pu' (or 'next' if you want, but that amounts to about
the same thing as we will be rebuilding 'next' soon after 1.6.0 ships),
but this would probably be better demonstrated with a test script that
demonstrates in what workflow this would be useful.

Re: RFC: Allow missing objects during packing

[Nicolas Pitre]

On Mon, 11 Aug 2008, Shawn O. Pearce wrote:

> > Junio C Hamano <gitster@pobox.com> wrote:
> > > If the check is only about a thin delta base that is not going to be
> > > transmit, I'd agree.  But I do not see how you are distinguishing that
> > > case and the case where an object you are actually sending is missing (in
> > > which case we would want to error out, wouldn't we?)
> 
> Turns out to be pretty simple I think.  We just delay the
> error handling for ->type < 0 until write_object().  If we
> get this far we know we wanted to include the object but
> we really don't have it.  Up until that point its fine
> for us to get objects which are missing, we'll just wind
> up with a suboptimal pack.

If you're going to die anyway due to an object with unknown type, better 
do so _before_ going through the delta search phase and leaving a 
partial pack behind.  IOW, the type check can be performed in 
prepare_pack() instead of write_object() like:

diff --git a/builtin-pack-objects.c b/builtin-pack-objects.c
index 2dadec1..01ab49c 100644
--- a/builtin-pack-objects.c
+++ b/builtin-pack-objects.c
@@ -1722,8 +1733,12 @@ static void prepare_pack(int window, int depth)
 		if (entry->no_try_delta)
 			continue;
 
-		if (!entry->preferred_base)
+		if (!entry->preferred_base) {
 			nr_deltas++;
+			if (entry->type < 0)
+				die("unable to get type of object %s",
+				    sha1_to_hex(entry->idx.sha1));
+		}
 
 		delta_list[n++] = entry;
 	}

Also a comment in check_object() mentioning where the return value of 
sha1_object_info() is verified would be in order.

And I also agree with Junio about a test script for this so the usage is 
fully demonstrated, and to ensure it keeps on working as intended 
(most people will simply never exercise this otherwise).


Nicolas

[PATCH] pack-objects: Allow missing base objects when creating thin packs

[Shawn O. Pearce]

If we are building a thin pack and one of the base objects we would
consider for deltification is missing its OK, the other side already
has that base object.  We may be able to get a delta from another
object, or we can simply send the new object whole (no delta).

This change allows a shallow clone to store only the objects which
are unique to it, as well as the boundary commit and its trees, but
avoids storing the boundary blobs.  This special form of a shallow
clone is able to represent just the difference between two trees.

Pack objects change suggested by Nicolas Pitre.

Signed-off-by: Shawn O. Pearce <spearce@spearce.org>
---

 Nicolas Pitre <nico@cam.org> wrote:
 > On Mon, 11 Aug 2008, Shawn O. Pearce wrote:
 > > > Junio C Hamano <gitster@pobox.com> wrote:
 > > > > If the check is only about a thin delta base that is not going to be
 > > > > transmit, I'd agree.
 > 
 > If you're going to die anyway due to an object with unknown type, better 
 > do so _before_ going through the delta search phase and leaving a 
 > partial pack behind.

 Now with a test! :-)
 
 builtin-pack-objects.c |   15 ++++++--
 t/t5306-pack-nobase.sh |   80 ++++++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 91 insertions(+), 4 deletions(-)
 create mode 100755 t/t5306-pack-nobase.sh

diff --git a/builtin-pack-objects.c b/builtin-pack-objects.c
index 2dadec1..a90302d 100644
--- a/builtin-pack-objects.c
+++ b/builtin-pack-objects.c
@@ -1096,9 +1096,12 @@ static void check_object(struct object_entry *entry)
 	}
 
 	entry->type = sha1_object_info(entry->idx.sha1, &entry->size);
-	if (entry->type < 0)
-		die("unable to get type of object %s",
-		    sha1_to_hex(entry->idx.sha1));
+	/*
+ 	 * The error condition is checked in prepare_pack().  This is
+ 	 * to permit a missing preferred base object to be ignored
+ 	 * as a preferred base.  Doing so can result in a larger
+ 	 * pack file, but the transfer will still take place.
+ 	 */
 }
 
 static int pack_offset_sort(const void *_a, const void *_b)
@@ -1722,8 +1725,12 @@ static void prepare_pack(int window, int depth)
 		if (entry->no_try_delta)
 			continue;
 
-		if (!entry->preferred_base)
+		if (!entry->preferred_base) {
 			nr_deltas++;
+			if (entry->type < 0)
+				die("unable to get type of object %s",
+				    sha1_to_hex(entry->idx.sha1));
+		}
 
 		delta_list[n++] = entry;
 	}
diff --git a/t/t5306-pack-nobase.sh b/t/t5306-pack-nobase.sh
new file mode 100755
index 0000000..503e9d4
--- /dev/null
+++ b/t/t5306-pack-nobase.sh
@@ -0,0 +1,80 @@
+#!/bin/sh
+#
+# Copyright (c) 2008 Google Inc.
+#
+
+test_description='git-pack-object with missing base
+
+'
+. ./test-lib.sh
+
+# Create A-B chain
+#
+test_expect_success \
+    'setup base' \
+    'for a in a b c d e f g h i; do echo $a >>text; done &&
+     echo side >side &&
+     git update-index --add text side &&
+     A=$(echo A | git commit-tree $(git write-tree)) &&
+
+     echo m >>text &&
+     git update-index text &&
+     B=$(echo B | git commit-tree $(git write-tree) -p $A) &&
+     git update-ref HEAD $B
+    '
+
+# Create repository with C whose parent is B.
+# Repository contains C, C^{tree}, C:text, B, B^{tree}.
+# Repository is missing B:text (best delta base for C:text).
+# Repository is missing A (parent of B).
+# Repository is missing A:side.
+#
+test_expect_success \
+    'setup patch_clone' \
+    'base_objects=$(pwd)/.git/objects &&
+     (mkdir patch_clone &&
+      cd patch_clone &&
+      git init &&
+      echo "$base_objects" >.git/objects/info/alternates &&
+      echo q >>text &&
+      git read-tree $B &&
+      git update-index text &&
+      git update-ref HEAD $(echo C | git commit-tree $(git write-tree) -p $B) &&
+      rm .git/objects/info/alternates &&
+
+      git --git-dir=../.git cat-file commit $B |
+      git hash-object -t commit -w --stdin &&
+
+      git --git-dir=../.git cat-file tree "$B^{tree}" |
+      git hash-object -t tree -w --stdin
+     ) &&
+     C=$(git --git-dir=patch_clone/.git rev-parse HEAD)
+    '
+
+# Clone patch_clone indirectly by cloning base and fetching.
+#
+test_expect_success \
+    'indirectly clone patch_clone' \
+    '(mkdir user_clone &&
+      cd user_clone &&
+      git init &&
+      git pull ../.git &&
+      test $(git rev-parse HEAD) = $B
+
+      git pull ../patch_clone/.git &&
+      test $(git rev-parse HEAD) = $C
+     )
+    '
+
+# Cloning the patch_clone directly should fail.
+#
+test_expect_success \
+    'clone of patch_clone is incomplete' \
+    '(mkdir user_direct &&
+      cd user_direct &&
+      git init &&
+      test_must_fail git fetch ../patch_clone/.git
+     )
+    '
+
+test_done
-- 
1.6.0.rc2.22.g71b99


-- 
Shawn.

Re: [PATCH] pack-objects: Allow missing base objects when creating thin packs

[Nicolas Pitre]

On Tue, 12 Aug 2008, Shawn O. Pearce wrote:

>  Now with a test! :-)
>  
[...]
> +# Clone patch_clone indirectly by cloning base and fetching.
> +#
> +test_expect_success \
> +    'indirectly clone patch_clone' \
> +    '(mkdir user_clone &&
> +      cd user_clone &&
> +      git init &&
> +      git pull ../.git &&
> +      test $(git rev-parse HEAD) = $B
> +
> +      git pull ../patch_clone/.git &&
> +      test $(git rev-parse HEAD) = $C
> +     )
> +    '

What if the first test command fails?  Won't its result be ignored?


Nicolas

Re: [PATCH] pack-objects: Allow missing base objects when creating thin packs

[Shawn O. Pearce]

Nicolas Pitre <nico@cam.org> wrote:
> On Tue, 12 Aug 2008, Shawn O. Pearce wrote:
> > +# Clone patch_clone indirectly by cloning base and fetching.
> > +#
> > +test_expect_success \
> > +    'indirectly clone patch_clone' \
> > +    '(mkdir user_clone &&
> > +      cd user_clone &&
> > +      git init &&
> > +      git pull ../.git &&
> > +      test $(git rev-parse HEAD) = $B
> > +
> > +      git pull ../patch_clone/.git &&
> > +      test $(git rev-parse HEAD) = $C
> > +     )
> > +    '
> 
> What if the first test command fails?  Won't its result be ignored?

Isn't the exit status of the subshell the exit status of the last
command in the subshell?

I just changed the test line to compare to "x$C" instead of $C
and it correctly detected the error condition:

$ git diff
diff --git a/t/t5306-pack-nobase.sh b/t/t5306-pack-nobase.sh
index 503e9d4..7c55e9e 100755
--- a/t/t5306-pack-nobase.sh
+++ b/t/t5306-pack-nobase.sh
@@ -62,7 +62,7 @@ test_expect_success \
       test $(git rev-parse HEAD) = $B

       git pull ../patch_clone/.git &&
-      test $(git rev-parse HEAD) = $C
+      test $(git rev-parse HEAD) = x$C
      )
     '

$ ./t5306-pack-nobase.sh
*   ok 1: setup base
*   ok 2: setup patch_clone
* FAIL 3: indirectly clone patch_clone
        (mkdir user_clone &&
              cd user_clone &&
              git init &&
              git pull ../.git &&
              test $(git rev-parse HEAD) = $B

              git pull ../patch_clone/.git &&
              test $(git rev-parse HEAD) = x$C
             )

*   ok 4: clone of patch_clone is incomplete
* failed 1 among 4 test(s)

-- 
Shawn.

Re: [PATCH] pack-objects: Allow missing base objects when creating thin packs

[Nicolas Pitre]

On Tue, 12 Aug 2008, Shawn O. Pearce wrote:

> Nicolas Pitre <nico@cam.org> wrote:
> > On Tue, 12 Aug 2008, Shawn O. Pearce wrote:
> > > +# Clone patch_clone indirectly by cloning base and fetching.
> > > +#
> > > +test_expect_success \
> > > +    'indirectly clone patch_clone' \
> > > +    '(mkdir user_clone &&
> > > +      cd user_clone &&
> > > +      git init &&
> > > +      git pull ../.git &&
> > > +      test $(git rev-parse HEAD) = $B
> > > +
> > > +      git pull ../patch_clone/.git &&
> > > +      test $(git rev-parse HEAD) = $C
> > > +     )
> > > +    '
> > 
> > What if the first test command fails?  Won't its result be ignored?
> 
> Isn't the exit status of the subshell the exit status of the last
> command in the subshell?

I'm not talking about the last command but the "test $(git rev-parse 
HEAD) = $B" line.


Nicolas

[PATCH v2] pack-objects: Allow missing base objects when creating thin packs

[Shawn O. Pearce]

If we are building a thin pack and one of the base objects we would
consider for deltification is missing its OK, the other side already
has that base object.  We may be able to get a delta from another
object, or we can simply send the new object whole (no delta).

This change allows a shallow clone to store only the objects which
are unique to it, as well as the boundary commit and its trees, but
avoids storing the boundary blobs.  This special form of a shallow
clone is able to represent just the difference between two trees.

Pack objects change suggested by Nicolas Pitre.

Signed-off-by: Shawn O. Pearce <spearce@spearce.org>
---

 Nicolas Pitre <nico@cam.org> wrote:
 > On Tue, 12 Aug 2008, Shawn O. Pearce wrote:
 > I'm not talking about the last command but the "test $(git rev-parse 
 > HEAD) = $B" line.
 
 Oh, right.  Good catch.

 builtin-pack-objects.c |   15 ++++++--
 t/t5306-pack-nobase.sh |   80 ++++++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 91 insertions(+), 4 deletions(-)
 create mode 100755 t/t5306-pack-nobase.sh

diff --git a/builtin-pack-objects.c b/builtin-pack-objects.c
index 2dadec1..a90302d 100644
--- a/builtin-pack-objects.c
+++ b/builtin-pack-objects.c
@@ -1096,9 +1096,12 @@ static void check_object(struct object_entry *entry)
 	}
 
 	entry->type = sha1_object_info(entry->idx.sha1, &entry->size);
-	if (entry->type < 0)
-		die("unable to get type of object %s",
-		    sha1_to_hex(entry->idx.sha1));
+	/*
+ 	 * The error condition is checked in prepare_pack().  This is
+ 	 * to permit a missing preferred base object to be ignored
+ 	 * as a preferred base.  Doing so can result in a larger
+ 	 * pack file, but the transfer will still take place.
+ 	 */
 }
 
 static int pack_offset_sort(const void *_a, const void *_b)
@@ -1722,8 +1725,12 @@ static void prepare_pack(int window, int depth)
 		if (entry->no_try_delta)
 			continue;
 
-		if (!entry->preferred_base)
+		if (!entry->preferred_base) {
 			nr_deltas++;
+			if (entry->type < 0)
+				die("unable to get type of object %s",
+				    sha1_to_hex(entry->idx.sha1));
+		}
 
 		delta_list[n++] = entry;
 	}
diff --git a/t/t5306-pack-nobase.sh b/t/t5306-pack-nobase.sh
new file mode 100755
index 0000000..f4931c0
--- /dev/null
+++ b/t/t5306-pack-nobase.sh
@@ -0,0 +1,80 @@
+#!/bin/sh
+#
+# Copyright (c) 2008 Google Inc.
+#
+
+test_description='git-pack-object with missing base
+
+'
+. ./test-lib.sh
+
+# Create A-B chain
+#
+test_expect_success \
+    'setup base' \
+    'for a in a b c d e f g h i; do echo $a >>text; done &&
+     echo side >side &&
+     git update-index --add text side &&
+     A=$(echo A | git commit-tree $(git write-tree)) &&
+
+     echo m >>text &&
+     git update-index text &&
+     B=$(echo B | git commit-tree $(git write-tree) -p $A) &&
+     git update-ref HEAD $B
+    '
+
+# Create repository with C whose parent is B.
+# Repository contains C, C^{tree}, C:text, B, B^{tree}.
+# Repository is missing B:text (best delta base for C:text).
+# Repository is missing A (parent of B).
+# Repository is missing A:side.
+#
+test_expect_success \
+    'setup patch_clone' \
+    'base_objects=$(pwd)/.git/objects &&
+     (mkdir patch_clone &&
+      cd patch_clone &&
+      git init &&
+      echo "$base_objects" >.git/objects/info/alternates &&
+      echo q >>text &&
+      git read-tree $B &&
+      git update-index text &&
+      git update-ref HEAD $(echo C | git commit-tree $(git write-tree) -p $B) &&
+      rm .git/objects/info/alternates &&
+
+      git --git-dir=../.git cat-file commit $B |
+      git hash-object -t commit -w --stdin &&
+
+      git --git-dir=../.git cat-file tree "$B^{tree}" |
+      git hash-object -t tree -w --stdin
+     ) &&
+     C=$(git --git-dir=patch_clone/.git rev-parse HEAD)
+    '
+
+# Clone patch_clone indirectly by cloning base and fetching.
+#
+test_expect_success \
+    'indirectly clone patch_clone' \
+    '(mkdir user_clone &&
+      cd user_clone &&
+      git init &&
+      git pull ../.git &&
+      test $(git rev-parse HEAD) = $B &&
+
+      git pull ../patch_clone/.git &&
+      test $(git rev-parse HEAD) = $C
+     )
+    '
+
+# Cloning the patch_clone directly should fail.
+#
+test_expect_success \
+    'clone of patch_clone is incomplete' \
+    '(mkdir user_direct &&
+      cd user_direct &&
+      git init &&
+      test_must_fail git fetch ../patch_clone/.git
+     )
+    '
+
+test_done
-- 
1.6.0.rc2.22.g71b99


-- 
Shawn.

Re: [PATCH v2] pack-objects: Allow missing base objects when creating thin packs

[Nicolas Pitre]

On Tue, 12 Aug 2008, Shawn O. Pearce wrote:

> If we are building a thin pack and one of the base objects we would
> consider for deltification is missing its OK, the other side already
> has that base object.  We may be able to get a delta from another
> object, or we can simply send the new object whole (no delta).
> 
> This change allows a shallow clone to store only the objects which
> are unique to it, as well as the boundary commit and its trees, but
> avoids storing the boundary blobs.  This special form of a shallow
> clone is able to represent just the difference between two trees.
> 
> Pack objects change suggested by Nicolas Pitre.
> 
> Signed-off-by: Shawn O. Pearce <spearce@spearce.org>
> ---
> 
>  Nicolas Pitre <nico@cam.org> wrote:
>  > On Tue, 12 Aug 2008, Shawn O. Pearce wrote:
>  > I'm not talking about the last command but the "test $(git rev-parse 
>  > HEAD) = $B" line.
>  
>  Oh, right.  Good catch.

Acked-by: Nicolas Pitre <nico@cam.org>

> +++ b/t/t5306-pack-nobase.sh
> @@ -0,0 +1,80 @@
> +#!/bin/sh
> +#
> +# Copyright (c) 2008 Google Inc.

Heh.  ;-)


Nicolas