{"thread":{"id":"28190","subject":"[PATCH 0/2] fast-import: tag any object by sha1","startedAt":"2011-08-22T12:10:17Z","lastAt":"2011-08-23T18:32:08Z","messageCount":4,"participants":["Dmitry Ivankov","Junio C Hamano"],"isPatch":true,"patchVersion":1,"patchTotal":2},"messages":[{"id":"174010","messageId":"1314015019-6636-1-git-send-email-divanorama@gmail.com","threadId":"28190","inReplyTo":null,"subject":"[PATCH 0/2] fast-import: tag any object by sha1","fromName":"Dmitry Ivankov","fromEmail":"divanorama@gmail.com","sentAt":"2011-08-22T12:10:17Z","receivedAt":"2011-08-22T12:10:17Z","isPatch":true,"sender":{"key":"divanorama@gmail.com","avatar":"https://avatars.githubusercontent.com/u/158999?v=4"},"body":"fast-export can export annotated tags that annotate any type of object.\nIt specifies objects via mark references and fast-import accepts this.\n\nfast-import also allows to specify objects via sha1, and to query sha1\nfor a object being imported. So it should allow to tag a pre-existing\nor being-imported objects by their sha1. And it currently does not:\n- for pre-existing it kind of assumes it is a OBJ_COMMIT, read_sha1_file()s\n  and checks only for (size >= 46), weird\n- for being-imported objects it calls read_sha1_file too and fails\n\nJust make it produce expected tags in these cases. Add a test for this.\n\nDmitry Ivankov (2):\n  fast-import: add tests for tagging blobs\n  fast-import: allow to tag newly created objects\n\n fast-import.c          |   14 +++++-----\n t/t9300-fast-import.sh |   67 ++++++++++++++++++++++++++++++++++++++++++++++++\n 2 files changed, 74 insertions(+), 7 deletions(-)\n\n-- \n1.7.3.4\n"},{"id":"174011","messageId":"1314015019-6636-2-git-send-email-divanorama@gmail.com","threadId":"28190","inReplyTo":"1314015019-6636-1-git-send-email-divanorama@gmail.com","subject":"[PATCH 1/2] fast-import: add tests for tagging blobs","fromName":"Dmitry Ivankov","fromEmail":"divanorama@gmail.com","sentAt":"2011-08-22T12:10:18Z","receivedAt":"2011-08-22T12:10:18Z","isPatch":true,"sender":{"key":"divanorama@gmail.com","avatar":"https://avatars.githubusercontent.com/u/158999?v=4"},"body":"fast-import allows to create an annotated tag that annotates a blob,\nvia mark or direct sha1 specification.\n\nFor mark it works, for sha1 it tries to read the object. It tries to\ndo so via read_sha1_file, and then checks the size to be at least 46.\n\nThat's weird, let's just allow to (annotated) tag any object referenced\nby sha1. If the object originates from our packfile, we still fail though.\n\nSigned-off-by: Dmitry Ivankov <divanorama@gmail.com>\n---\n fast-import.c          |   10 +++-------\n t/t9300-fast-import.sh |   41 +++++++++++++++++++++++++++++++++++++++++\n 2 files changed, 44 insertions(+), 7 deletions(-)\n\ndiff --git a/fast-import.c b/fast-import.c\nindex 7cc2262..0b0f598 100644\n--- a/fast-import.c\n+++ b/fast-import.c\n@@ -2690,13 +2690,9 @@ static void parse_new_tag(void)\n \t\ttype = oe->type;\n \t\thashcpy(sha1, oe->idx.sha1);\n \t} else if (!get_sha1(from, sha1)) {\n-\t\tunsigned long size;\n-\t\tchar *buf;\n-\n-\t\tbuf = read_sha1_file(sha1, &type, &size);\n-\t\tif (!buf || size < 46)\n-\t\t\tdie(\"Not a valid commit: %s\", from);\n-\t\tfree(buf);\n+\t\ttype = sha1_object_info(sha1, NULL);\n+\t\tif (type < 0)\n+\t\t\tdie(\"Not a valid object: %s\", from);\n \t} else\n \t\tdie(\"Invalid ref name or SHA1 expression: %s\", from);\n \tread_next_command();\ndiff --git a/t/t9300-fast-import.sh b/t/t9300-fast-import.sh\nindex f256475..41f0d02 100755\n--- a/t/t9300-fast-import.sh\n+++ b/t/t9300-fast-import.sh\n@@ -94,6 +94,12 @@ data <<EOF\n An annotated tag without a tagger\n EOF\n \n+tag series-A-blob\n+from :3\n+data <<EOF\n+An annotated tag that annotates a blob.\n+EOF\n+\n INPUT_END\n test_expect_success \\\n     'A: create pack from stdin' \\\n@@ -152,6 +158,18 @@ test_expect_success 'A: verify tag/series-A' '\n '\n \n cat >expect <<EOF\n+object $(git rev-parse refs/heads/master:file3)\n+type blob\n+tag series-A-blob\n+\n+An annotated tag that annotates a blob.\n+EOF\n+test_expect_success 'A: verify tag/series-A-blob' '\n+\tgit cat-file tag tags/series-A-blob >actual &&\n+\ttest_cmp expect actual\n+'\n+\n+cat >expect <<EOF\n :2 `git rev-parse --verify master:file2`\n :3 `git rev-parse --verify master:file3`\n :4 `git rev-parse --verify master:file4`\n@@ -171,6 +189,29 @@ test_expect_success \\\n \n test_tick\n cat >input <<INPUT_END\n+tag series-A-blob-2\n+from $(git rev-parse refs/heads/master:file3)\n+data <<EOF\n+Tag blob by sha1.\n+EOF\n+INPUT_END\n+\n+cat >expect <<EOF\n+object $(git rev-parse refs/heads/master:file3)\n+type blob\n+tag series-A-blob-2\n+\n+Tag blob by sha1.\n+EOF\n+\n+test_expect_success \\\n+\t'A: tag blob by sha1' \\\n+\t'git fast-import <input &&\n+\tgit cat-file tag tags/series-A-blob-2 >actual &&\n+\ttest_cmp expect actual'\n+\n+test_tick\n+cat >input <<INPUT_END\n commit refs/heads/verify--import-marks\n committer $GIT_COMMITTER_NAME <$GIT_COMMITTER_EMAIL> $GIT_COMMITTER_DATE\n data <<COMMIT\n-- \n1.7.3.4\n"},{"id":"174012","messageId":"1314015019-6636-3-git-send-email-divanorama@gmail.com","threadId":"28190","inReplyTo":"1314015019-6636-1-git-send-email-divanorama@gmail.com","subject":"[PATCH 2/2] fast-import: allow to tag newly created objects","fromName":"Dmitry Ivankov","fromEmail":"divanorama@gmail.com","sentAt":"2011-08-22T12:10:19Z","receivedAt":"2011-08-22T12:10:19Z","isPatch":true,"sender":{"key":"divanorama@gmail.com","avatar":"https://avatars.githubusercontent.com/u/158999?v=4"},"body":"fast-import allows to tag objects by sha1 and to query sha1 of objects\nbeing imported. So it should allow to tag these objects, make it do so.\n\nSigned-off-by: Dmitry Ivankov <divanorama@gmail.com>\n---\n fast-import.c          |   10 +++++++---\n t/t9300-fast-import.sh |   26 ++++++++++++++++++++++++++\n 2 files changed, 33 insertions(+), 3 deletions(-)\n\ndiff --git a/fast-import.c b/fast-import.c\nindex 0b0f598..11eb6bf 100644\n--- a/fast-import.c\n+++ b/fast-import.c\n@@ -2690,9 +2690,13 @@ static void parse_new_tag(void)\n \t\ttype = oe->type;\n \t\thashcpy(sha1, oe->idx.sha1);\n \t} else if (!get_sha1(from, sha1)) {\n-\t\ttype = sha1_object_info(sha1, NULL);\n-\t\tif (type < 0)\n-\t\t\tdie(\"Not a valid object: %s\", from);\n+\t\tstruct object_entry *oe = find_object(sha1);\n+\t\tif (!oe) {\n+\t\t\ttype = sha1_object_info(sha1, NULL);\n+\t\t\tif (type < 0)\n+\t\t\t\tdie(\"Not a valid object: %s\", from);\n+\t\t} else\n+\t\t\ttype = oe->type;\n \t} else\n \t\tdie(\"Invalid ref name or SHA1 expression: %s\", from);\n \tread_next_command();\ndiff --git a/t/t9300-fast-import.sh b/t/t9300-fast-import.sh\nindex 41f0d02..efe9779 100755\n--- a/t/t9300-fast-import.sh\n+++ b/t/t9300-fast-import.sh\n@@ -188,12 +188,32 @@ test_expect_success \\\n \ttest_cmp expect marks.new'\n \n test_tick\n+new_blob=$(echo testing | git hash-object --stdin)\n cat >input <<INPUT_END\n tag series-A-blob-2\n from $(git rev-parse refs/heads/master:file3)\n data <<EOF\n Tag blob by sha1.\n EOF\n+\n+blob\n+mark :6\n+data <<EOF\n+testing\n+EOF\n+\n+commit refs/heads/new_blob\n+committer  <> 0 +0000\n+data 0\n+M 644 :6 new_blob\n+#pretend we got sha1 from fast-import\n+ls \"new_blob\"\n+\n+tag series-A-blob-3\n+from $new_blob\n+data <<EOF\n+Tag new_blob.\n+EOF\n INPUT_END\n \n cat >expect <<EOF\n@@ -202,12 +222,18 @@ type blob\n tag series-A-blob-2\n \n Tag blob by sha1.\n+object $new_blob\n+type blob\n+tag series-A-blob-3\n+\n+Tag new_blob.\n EOF\n \n test_expect_success \\\n \t'A: tag blob by sha1' \\\n \t'git fast-import <input &&\n \tgit cat-file tag tags/series-A-blob-2 >actual &&\n+\tgit cat-file tag tags/series-A-blob-3 >>actual &&\n \ttest_cmp expect actual'\n \n test_tick\n-- \n1.7.3.4\n"},{"id":"174109","messageId":"7vy5ykq8rb.fsf@alter.siamese.dyndns.org","threadId":"28190","inReplyTo":"1314015019-6636-3-git-send-email-divanorama@gmail.com","subject":"Re: [PATCH 2/2] fast-import: allow to tag newly created objects","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2011-08-23T18:32:08Z","receivedAt":"2011-08-23T18:32:08Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Dmitry Ivankov <divanorama@gmail.com> writes:\n\n>  \t} else if (!get_sha1(from, sha1)) {\n> -\t\ttype = sha1_object_info(sha1, NULL);\n> -\t\tif (type < 0)\n> -\t\t\tdie(\"Not a valid object: %s\", from);\n> +\t\tstruct object_entry *oe = find_object(sha1);\n> +\t\tif (!oe) {\n> +\t\t\ttype = sha1_object_info(sha1, NULL);\n> +\t\t\tif (type < 0)\n> +\t\t\t\tdie(\"Not a valid object: %s\", from);\n> +\t\t} else\n> +\t\t\ttype = oe->type;\n\nIt might be just a \"taste\" thing, but I would have expected the above to\nbe written like so:\n\n\tstruct object_entry *oe = find_object(sha1);\n\tif (!oe)\n\t\ttype = sha1_object_info(sha1, NULL);\n\telse\n\t\ttype = oe->type;\n\tif (type < 0)\n\t\tdie(\"Not a valid object: %s\", from);\n\nThe point being that find_object()->type and the return value of\nsha1_object_info() are supposed to be compatible and interchangeably used,\nwhich is exactly why the same variable \"type\" gets assigned and later be\nused in the same codeflow, so they should get the same error checking,\neven if it happens to be that the current implementation of find_object()\nnever returns an object with invalid type in it.\n"}]}