[PATCH 0/2] fast-import: tag any object by sha1

[PATCH 0/2] fast-import: tag any object by sha1

[Dmitry Ivankov]

fast-export can export annotated tags that annotate any type of object.
It specifies objects via mark references and fast-import accepts this.

fast-import also allows to specify objects via sha1, and to query sha1
for a object being imported. So it should allow to tag a pre-existing
or being-imported objects by their sha1. And it currently does not:
- for pre-existing it kind of assumes it is a OBJ_COMMIT, read_sha1_file()s
  and checks only for (size >= 46), weird
- for being-imported objects it calls read_sha1_file too and fails

Just make it produce expected tags in these cases. Add a test for this.

Dmitry Ivankov (2):
  fast-import: add tests for tagging blobs
  fast-import: allow to tag newly created objects

 fast-import.c          |   14 +++++-----
 t/t9300-fast-import.sh |   67 ++++++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 74 insertions(+), 7 deletions(-)

-- 
1.7.3.4

[PATCH 1/2] fast-import: add tests for tagging blobs

[Dmitry Ivankov]

fast-import allows to create an annotated tag that annotates a blob,
via mark or direct sha1 specification.

For mark it works, for sha1 it tries to read the object. It tries to
do so via read_sha1_file, and then checks the size to be at least 46.

That's weird, let's just allow to (annotated) tag any object referenced
by sha1. If the object originates from our packfile, we still fail though.

Signed-off-by: Dmitry Ivankov <divanorama@gmail.com>
---
 fast-import.c          |   10 +++-------
 t/t9300-fast-import.sh |   41 +++++++++++++++++++++++++++++++++++++++++
 2 files changed, 44 insertions(+), 7 deletions(-)

diff --git a/fast-import.c b/fast-import.c
index 7cc2262..0b0f598 100644
--- a/fast-import.c
+++ b/fast-import.c
@@ -2690,13 +2690,9 @@ static void parse_new_tag(void)
 		type = oe->type;
 		hashcpy(sha1, oe->idx.sha1);
 	} else if (!get_sha1(from, sha1)) {
-		unsigned long size;
-		char *buf;
-
-		buf = read_sha1_file(sha1, &type, &size);
-		if (!buf || size < 46)
-			die("Not a valid commit: %s", from);
-		free(buf);
+		type = sha1_object_info(sha1, NULL);
+		if (type < 0)
+			die("Not a valid object: %s", from);
 	} else
 		die("Invalid ref name or SHA1 expression: %s", from);
 	read_next_command();
diff --git a/t/t9300-fast-import.sh b/t/t9300-fast-import.sh
index f256475..41f0d02 100755
--- a/t/t9300-fast-import.sh
+++ b/t/t9300-fast-import.sh
@@ -94,6 +94,12 @@ data <<EOF
 An annotated tag without a tagger
 EOF
 
+tag series-A-blob
+from :3
+data <<EOF
+An annotated tag that annotates a blob.
+EOF
+
 INPUT_END
 test_expect_success \
     'A: create pack from stdin' \
@@ -152,6 +158,18 @@ test_expect_success 'A: verify tag/series-A' '
 '
 
 cat >expect <<EOF
+object $(git rev-parse refs/heads/master:file3)
+type blob
+tag series-A-blob
+
+An annotated tag that annotates a blob.
+EOF
+test_expect_success 'A: verify tag/series-A-blob' '
+	git cat-file tag tags/series-A-blob >actual &&
+	test_cmp expect actual
+'
+
+cat >expect <<EOF
 :2 `git rev-parse --verify master:file2`
 :3 `git rev-parse --verify master:file3`
 :4 `git rev-parse --verify master:file4`
@@ -171,6 +189,29 @@ test_expect_success \
 
 test_tick
 cat >input <<INPUT_END
+tag series-A-blob-2
+from $(git rev-parse refs/heads/master:file3)
+data <<EOF
+Tag blob by sha1.
+EOF
+INPUT_END
+
+cat >expect <<EOF
+object $(git rev-parse refs/heads/master:file3)
+type blob
+tag series-A-blob-2
+
+Tag blob by sha1.
+EOF
+
+test_expect_success \
+	'A: tag blob by sha1' \
+	'git fast-import <input &&
+	git cat-file tag tags/series-A-blob-2 >actual &&
+	test_cmp expect actual'
+
+test_tick
+cat >input <<INPUT_END
 commit refs/heads/verify--import-marks
 committer $GIT_COMMITTER_NAME <$GIT_COMMITTER_EMAIL> $GIT_COMMITTER_DATE
 data <<COMMIT
-- 
1.7.3.4

[PATCH 2/2] fast-import: allow to tag newly created objects

[Dmitry Ivankov]

fast-import allows to tag objects by sha1 and to query sha1 of objects
being imported. So it should allow to tag these objects, make it do so.

Signed-off-by: Dmitry Ivankov <divanorama@gmail.com>
---
 fast-import.c          |   10 +++++++---
 t/t9300-fast-import.sh |   26 ++++++++++++++++++++++++++
 2 files changed, 33 insertions(+), 3 deletions(-)

diff --git a/fast-import.c b/fast-import.c
index 0b0f598..11eb6bf 100644
--- a/fast-import.c
+++ b/fast-import.c
@@ -2690,9 +2690,13 @@ static void parse_new_tag(void)
 		type = oe->type;
 		hashcpy(sha1, oe->idx.sha1);
 	} else if (!get_sha1(from, sha1)) {
-		type = sha1_object_info(sha1, NULL);
-		if (type < 0)
-			die("Not a valid object: %s", from);
+		struct object_entry *oe = find_object(sha1);
+		if (!oe) {
+			type = sha1_object_info(sha1, NULL);
+			if (type < 0)
+				die("Not a valid object: %s", from);
+		} else
+			type = oe->type;
 	} else
 		die("Invalid ref name or SHA1 expression: %s", from);
 	read_next_command();
diff --git a/t/t9300-fast-import.sh b/t/t9300-fast-import.sh
index 41f0d02..efe9779 100755
--- a/t/t9300-fast-import.sh
+++ b/t/t9300-fast-import.sh
@@ -188,12 +188,32 @@ test_expect_success \
 	test_cmp expect marks.new'
 
 test_tick
+new_blob=$(echo testing | git hash-object --stdin)
 cat >input <<INPUT_END
 tag series-A-blob-2
 from $(git rev-parse refs/heads/master:file3)
 data <<EOF
 Tag blob by sha1.
 EOF
+
+blob
+mark :6
+data <<EOF
+testing
+EOF
+
+commit refs/heads/new_blob
+committer  <> 0 +0000
+data 0
+M 644 :6 new_blob
+#pretend we got sha1 from fast-import
+ls "new_blob"
+
+tag series-A-blob-3
+from $new_blob
+data <<EOF
+Tag new_blob.
+EOF
 INPUT_END
 
 cat >expect <<EOF
@@ -202,12 +222,18 @@ type blob
 tag series-A-blob-2
 
 Tag blob by sha1.
+object $new_blob
+type blob
+tag series-A-blob-3
+
+Tag new_blob.
 EOF
 
 test_expect_success \
 	'A: tag blob by sha1' \
 	'git fast-import <input &&
 	git cat-file tag tags/series-A-blob-2 >actual &&
+	git cat-file tag tags/series-A-blob-3 >>actual &&
 	test_cmp expect actual'
 
 test_tick
-- 
1.7.3.4

Re: [PATCH 2/2] fast-import: allow to tag newly created objects

[Junio C Hamano]

Dmitry Ivankov <divanorama@gmail.com> writes:

>  	} else if (!get_sha1(from, sha1)) {
> -		type = sha1_object_info(sha1, NULL);
> -		if (type < 0)
> -			die("Not a valid object: %s", from);
> +		struct object_entry *oe = find_object(sha1);
> +		if (!oe) {
> +			type = sha1_object_info(sha1, NULL);
> +			if (type < 0)
> +				die("Not a valid object: %s", from);
> +		} else
> +			type = oe->type;

It might be just a "taste" thing, but I would have expected the above to
be written like so:

	struct object_entry *oe = find_object(sha1);
	if (!oe)
		type = sha1_object_info(sha1, NULL);
	else
		type = oe->type;
	if (type < 0)
		die("Not a valid object: %s", from);

The point being that find_object()->type and the return value of
sha1_object_info() are supposed to be compatible and interchangeably used,
which is exactly why the same variable "type" gets assigned and later be
used in the same codeflow, so they should get the same error checking,
even if it happens to be that the current implementation of find_object()
never returns an object with invalid type in it.