{"thread":{"id":"12477","subject":"[PATCH 1/3] git-pack-objects: Automatically pack annotated tags if object was packed","startedAt":"2008-03-04T02:36:07Z","lastAt":"2008-03-05T06:32:43Z","messageCount":8,"participants":["Shawn O. Pearce","Nicolas Pitre","Daniel Barkalow","Johannes Sixt"],"isPatch":true,"patchVersion":1,"patchTotal":3},"messages":[{"id":"70871","messageId":"20080304023607.GA16152@spearce.org","threadId":"12477","inReplyTo":null,"subject":"[PATCH 1/3] git-pack-objects: Automatically pack annotated tags if object was packed","fromName":"Shawn O. Pearce","fromEmail":"spearce@spearce.org","sentAt":"2008-03-04T02:36:07Z","receivedAt":"2008-03-04T02:36:07Z","isPatch":true,"sender":{"key":"spearce@spearce.org","avatar":"https://avatars.githubusercontent.com/u/34844?v=4"},"body":"The new option \"--auto-follow-tags\" allows the caller to request that\nany annotated tag be included into the packfile if the object the tag\nreferences was also included as part of the packfile.\n\nThis option can be useful on the server side of a native git transport,\nwhere the server knows what commits it is including into a packfile to\nupdate the client.  If new annotated tags have been introduced then we\ncan also include them in the packfile, saving the client from needing\nto request them through a second connection.\n\nThis change only introduces the backend option and provides a test.\nProtocol extensions to make this useful in fetch-pack/upload-pack\nare still necessary to activate the logic during transport.\n\nSigned-off-by: Shawn O. Pearce <spearce@spearce.org>\n---\n Documentation/git-pack-objects.txt |    5 ++\n builtin-pack-objects.c             |   24 +++++++++-\n t/t5305-autofollow-tag.sh          |   84 ++++++++++++++++++++++++++++++++++++\n 3 files changed, 111 insertions(+), 2 deletions(-)\n create mode 100755 t/t5305-autofollow-tag.sh\n\ndiff --git a/Documentation/git-pack-objects.txt b/Documentation/git-pack-objects.txt\nindex 5c1bd3b..0ba9cbe 100644\n--- a/Documentation/git-pack-objects.txt\n+++ b/Documentation/git-pack-objects.txt\n@@ -170,6 +170,11 @@ base-name::\n \tlength, this option typically shrinks the resulting\n \tpackfile by 3-5 per-cent.\n \n+--auto-follow-tags::\n+\tAutomatically include annotated tags if the object they\n+\treference was included in the resulting packfile.  This\n+\tcan be useful to send new tags to native git clients.\n+\n --threads=<n>::\n \tSpecifies the number of threads to spawn when searching for best\n \tdelta matches.  This requires that pack-objects be compiled with\ndiff --git a/builtin-pack-objects.c b/builtin-pack-objects.c\nindex 2799e68..8b5d90a 100644\n--- a/builtin-pack-objects.c\n+++ b/builtin-pack-objects.c\n@@ -15,6 +15,7 @@\n #include \"revision.h\"\n #include \"list-objects.h\"\n #include \"progress.h\"\n+#include \"refs.h\"\n \n #ifdef THREADED_DELTA_SEARCH\n #include \"thread-utils.h\"\n@@ -27,7 +28,8 @@ git-pack-objects [{ -q | --progress | --all-progress }] \\n\\\n \t[--window=N] [--window-memory=N] [--depth=N] \\n\\\n \t[--no-reuse-delta] [--no-reuse-object] [--delta-base-offset] \\n\\\n \t[--threads=N] [--non-empty] [--revs [--unpacked | --all]*] [--reflog] \\n\\\n-\t[--stdout | base-name] [--keep-unreachable] [<ref-list | <object-list]\";\n+\t[--stdout | base-name] [--auto-follow-tags] [--keep-unreachable] \\n\\\n+\t[<ref-list | <object-list]\";\n \n struct object_entry {\n \tstruct pack_idx_entry idx;\n@@ -63,7 +65,7 @@ static struct pack_idx_entry **written_list;\n static uint32_t nr_objects, nr_alloc, nr_result, nr_written;\n \n static int non_empty;\n-static int no_reuse_delta, no_reuse_object, keep_unreachable;\n+static int no_reuse_delta, no_reuse_object, keep_unreachable, auto_tags;\n static int local;\n static int incremental;\n static int allow_ofs_delta;\n@@ -1630,6 +1632,18 @@ static void ll_find_deltas(struct object_entry **list, unsigned list_size,\n #define ll_find_deltas(l, s, w, d, p)\tfind_deltas(l, &s, w, d, p)\n #endif\n \n+static int add_ref_tag(const char *path, const unsigned char *sha1, int flag, void *cb_data)\n+{\n+\tunsigned char peeled[20];\n+\n+\tif (!prefixcmp(path, \"refs/tags/\") && /* is a tag? */\n+\t    !peel_ref(path, peeled)        && /* peelable? */\n+\t    !is_null_sha1(peeled)          && /* annotated tag? */\n+\t    locate_object_entry(peeled))      /* object packed? */\n+\t\tadd_object_entry(sha1, OBJ_TAG, NULL, 0);\n+\treturn 0;\n+}\n+\n static void prepare_pack(int window, int depth)\n {\n \tstruct object_entry **delta_list;\n@@ -2033,6 +2047,10 @@ int cmd_pack_objects(int argc, const char **argv, const char *prefix)\n \t\t\tkeep_unreachable = 1;\n \t\t\tcontinue;\n \t\t}\n+\t\tif (!strcmp(\"--auto-follow-tags\", arg)) {\n+\t\t\tauto_tags = 1;\n+\t\t\tcontinue;\n+\t\t}\n \t\tif (!strcmp(\"--unpacked\", arg) ||\n \t\t    !prefixcmp(arg, \"--unpacked=\") ||\n \t\t    !strcmp(\"--reflog\", arg) ||\n@@ -2109,6 +2127,8 @@ int cmd_pack_objects(int argc, const char **argv, const char *prefix)\n \t\trp_av[rp_ac] = NULL;\n \t\tget_object_list(rp_ac, rp_av);\n \t}\n+\tif (auto_tags && nr_result)\n+\t\tfor_each_ref(add_ref_tag, NULL);\n \tstop_progress(&progress_state);\n \n \tif (non_empty && !nr_result)\ndiff --git a/t/t5305-autofollow-tag.sh b/t/t5305-autofollow-tag.sh\nnew file mode 100755\nindex 0000000..3d327ee\n--- /dev/null\n+++ b/t/t5305-autofollow-tag.sh\n@@ -0,0 +1,84 @@\n+#!/bin/sh\n+\n+test_description='git-pack-object --auto-follow-tags'\n+. ./test-lib.sh\n+\n+TRASH=`pwd`\n+\n+test_expect_success setup '\n+\techo c >d &&\n+\tgit update-index --add d &&\n+\ttree=`git write-tree` &&\n+\tcommit=`git commit-tree $tree </dev/null` &&\n+\techo \"object $commit\" >sig &&\n+\techo \"type commit\" >>sig &&\n+\techo \"tag mytag\" >>sig &&\n+\techo \"tagger $(git var GIT_COMMITTER_IDENT)\" >>sig &&\n+\techo >>sig &&\n+\techo \"our test tag\" >>sig &&\n+\ttag=`git mktag <sig` &&\n+\trm d sig &&\n+\tgit update-ref refs/tags/mytag $tag && {\n+\t\techo $tree &&\n+\t\techo $commit &&\n+\t\tgit ls-tree $tree | sed -e \"s/.* \\\\([0-9a-f]*\\\\)\t.*/\\\\1/\"\n+\t} >obj-list\n+'\n+\n+rm -rf clone.git\n+test_expect_success 'pack without --auto-follow-tags' '\n+\tpackname_1=$(git pack-objects \\\n+\t\t--window=0 \\\n+\t\ttest-1 <obj-list)\n+'\n+\n+test_expect_success 'unpack objects' '\n+\t(\n+\t\tGIT_DIR=clone.git &&\n+\t\texport GIT_DIR &&\n+\t\tgit init &&\n+\t\tgit unpack-objects -n <test-1-${packname_1}.pack &&\n+\t\tgit unpack-objects <test-1-${packname_1}.pack\n+\t)\n+'\n+\n+test_expect_success 'check unpacked result (have commit, no tag)' '\n+\tgit rev-list --objects $commit >list.expect &&\n+\t(\n+\t\tGIT_DIR=clone.git &&\n+\t\texport GIT_DIR &&\n+\t\ttest_must_fail git cat-file -e $tag &&\n+\t\tgit rev-list --objects $commit\n+\t) >list.actual &&\n+\tgit diff list.expect list.actual\n+'\n+\n+rm -rf clone.git\n+test_expect_success 'pack with --auto-follow-tags' '\n+\tpackname_1=$(git pack-objects \\\n+\t\t--window=0 \\\n+\t\t--auto-follow-tags \\\n+\t\ttest-2 <obj-list)\n+'\n+\n+test_expect_success 'unpack objects' '\n+\t(\n+\t\tGIT_DIR=clone.git &&\n+\t\texport GIT_DIR &&\n+\t\tgit init &&\n+\t\tgit unpack-objects -n <test-2-${packname_1}.pack &&\n+\t\tgit unpack-objects <test-2-${packname_1}.pack\n+\t)\n+'\n+\n+test_expect_success 'check unpacked result (have commit, have tag)' '\n+\tgit rev-list --objects mytag >list.expect &&\n+\t(\n+\t\tGIT_DIR=clone.git &&\n+\t\texport GIT_DIR &&\n+\t\tgit rev-list --objects $tag\n+\t) >list.actual &&\n+\tgit diff list.expect list.actual\n+'\n+\n+test_done\n-- \n1.5.4.3.529.gb25fb\n\n"},{"id":"70873","messageId":"alpine.LFD.1.00.0803032144110.2947@xanadu.home","threadId":"12477","inReplyTo":"20080304023607.GA16152@spearce.org","subject":"Re: [PATCH 1/3] git-pack-objects: Automatically pack annotated tags if object was packed","fromName":"Nicolas Pitre","fromEmail":"nico@cam.org","sentAt":"2008-03-04T02:45:21Z","receivedAt":"2008-03-04T02:45:21Z","isPatch":true,"sender":{"key":"nico@fluxnic.net","avatar":"https://avatars.githubusercontent.com/u/702790?v=4"},"body":"On Mon, 3 Mar 2008, Shawn O. Pearce wrote:\n\n> The new option \"--auto-follow-tags\" allows the caller to request that\n> any annotated tag be included into the packfile if the object the tag\n> references was also included as part of the packfile.\n\nWouldn't \"auto-include-tag\" a better name for this option?\n\n\nNicolas\n"},{"id":"70874","messageId":"alpine.LNX.1.00.0803032145480.19665@iabervon.org","threadId":"12477","inReplyTo":"20080304023607.GA16152@spearce.org","subject":"Re: [PATCH 1/3] git-pack-objects: Automatically pack annotated tags if object was packed","fromName":"Daniel Barkalow","fromEmail":"barkalow@iabervon.org","sentAt":"2008-03-04T02:49:12Z","receivedAt":"2008-03-04T02:49:12Z","isPatch":true,"sender":{"key":"barkalow@iabervon.org","avatar":"https://avatars.githubusercontent.com/u/55364219?v=4"},"body":"On Mon, 3 Mar 2008, Shawn O. Pearce wrote:\n\n> +\tif (auto_tags && nr_result)\n> +\t\tfor_each_ref(add_ref_tag, NULL);\n\nThere's for_each_tag_ref() that does the path-based filter internally, in \na possibly more efficient way, and avoids open-coding the test for whether \nthis is a tag sort of ref.\n\nAside from that, it looks good to me.\n\n\t-Daniel\n*This .sig left intentionally blank*\n"},{"id":"70878","messageId":"20080304030631.GN8410@spearce.org","threadId":"12477","inReplyTo":"alpine.LNX.1.00.0803032145480.19665@iabervon.org","subject":"Re: [PATCH 1/3] git-pack-objects: Automatically pack annotated tags if object was packed","fromName":"Shawn O. Pearce","fromEmail":"spearce@spearce.org","sentAt":"2008-03-04T03:06:31Z","receivedAt":"2008-03-04T03:06:31Z","isPatch":true,"sender":{"key":"spearce@spearce.org","avatar":"https://avatars.githubusercontent.com/u/34844?v=4"},"body":"Daniel Barkalow <barkalow@iabervon.org> wrote:\n> On Mon, 3 Mar 2008, Shawn O. Pearce wrote:\n> \n> > +\tif (auto_tags && nr_result)\n> > +\t\tfor_each_ref(add_ref_tag, NULL);\n> \n> There's for_each_tag_ref() that does the path-based filter internally, in \n> a possibly more efficient way, and avoids open-coding the test for whether \n> this is a tag sort of ref.\n\nYea, I know about that call, but I didn't use it because I was\ntrying to use peel_ref() for its optimized peeled caching in\na packed-refs file.\n\nTo use peel_ref() you have to pass the full ref name (\"refs/tags/v1.0\")\nand not a partial ref name (\"v1.0\") such as for_each_tag_ref() will\nreturn to the callback function.  So I'd have to actually do extra\nwork to reconstruct a ref name, and possibly an invalid ref name at\nthat if for_each_tag_ref() ever looked outside of the refs/tags/\nnamespace.\n\nToday there is no performance benefit to for_each_tag_ref() over\nfor_each_ref(), but using for_each_ref() lets me use peel_ref(),\nwhich is a performance improvement if the ref in question came from\nthe packed-refs file.\n\nSo.... do we keep things as-is or try to optimize peel_ref() (or\nsome new variant of it?!?) for use in for_each_tag_ref() just on\nthe off chance that we somehow can optimize for_each_tag_ref to\navoid scanning refs/heads/?  Note that I did look into trying to\ndo that optimization to for_each_tag_ref() earlier last week and\nconcluded it wasn't easily possible right now, and wasn't likely\nto be worth the development costs.\n\n-- \nShawn.\n"},{"id":"70879","messageId":"20080304030658.GO8410@spearce.org","threadId":"12477","inReplyTo":"alpine.LFD.1.00.0803032144110.2947@xanadu.home","subject":"Re: [PATCH 1/3] git-pack-objects: Automatically pack annotated tags if object was packed","fromName":"Shawn O. Pearce","fromEmail":"spearce@spearce.org","sentAt":"2008-03-04T03:06:58Z","receivedAt":"2008-03-04T03:06:58Z","isPatch":true,"sender":{"key":"spearce@spearce.org","avatar":"https://avatars.githubusercontent.com/u/34844?v=4"},"body":"Nicolas Pitre <nico@cam.org> wrote:\n> On Mon, 3 Mar 2008, Shawn O. Pearce wrote:\n> \n> > The new option \"--auto-follow-tags\" allows the caller to request that\n> > any annotated tag be included into the packfile if the object the tag\n> > references was also included as part of the packfile.\n> \n> Wouldn't \"auto-include-tag\" a better name for this option?\n\nOoooh.  Indeed it would!\n\nI shall respin this series based around this better name.\n\n-- \nShawn.\n"},{"id":"70880","messageId":"alpine.LNX.1.00.0803032210230.19665@iabervon.org","threadId":"12477","inReplyTo":"20080304030631.GN8410@spearce.org","subject":"Re: [PATCH 1/3] git-pack-objects: Automatically pack annotated tags if object was packed","fromName":"Daniel Barkalow","fromEmail":"barkalow@iabervon.org","sentAt":"2008-03-04T03:11:35Z","receivedAt":"2008-03-04T03:11:35Z","isPatch":true,"sender":{"key":"barkalow@iabervon.org","avatar":"https://avatars.githubusercontent.com/u/55364219?v=4"},"body":"On Mon, 3 Mar 2008, Shawn O. Pearce wrote:\n\n> Daniel Barkalow <barkalow@iabervon.org> wrote:\n> > On Mon, 3 Mar 2008, Shawn O. Pearce wrote:\n> > \n> > > +\tif (auto_tags && nr_result)\n> > > +\t\tfor_each_ref(add_ref_tag, NULL);\n> > \n> > There's for_each_tag_ref() that does the path-based filter internally, in \n> > a possibly more efficient way, and avoids open-coding the test for whether \n> > this is a tag sort of ref.\n> \n> Yea, I know about that call, but I didn't use it because I was\n> trying to use peel_ref() for its optimized peeled caching in\n> a packed-refs file.\n\nAh, okay. I hadn't noticed that it didn't pass on the full name. Using \nfor_each_ref() is better, then, I think.\n\n\t-Daniel\n*This .sig left intentionally blank*\n"},{"id":"70898","messageId":"47CCF9DA.4050304@viscovery.net","threadId":"12477","inReplyTo":"20080304030658.GO8410@spearce.org","subject":"Re: [PATCH 1/3] git-pack-objects: Automatically pack annotated tags if object was packed","fromName":"Johannes Sixt","fromEmail":"j.sixt@viscovery.net","sentAt":"2008-03-04T07:27:22Z","receivedAt":"2008-03-04T07:27:22Z","isPatch":true,"sender":{"key":"j6t@kdbg.org","avatar":"https://avatars.githubusercontent.com/u/14810926?v=4"},"body":"Shawn O. Pearce schrieb:\n> Nicolas Pitre <nico@cam.org> wrote:\n>> On Mon, 3 Mar 2008, Shawn O. Pearce wrote:\n>>\n>>> The new option \"--auto-follow-tags\" allows the caller to request that\n>>> any annotated tag be included into the packfile if the object the tag\n>>> references was also included as part of the packfile.\n>> Wouldn't \"auto-include-tag\" a better name for this option?\n> \n> Ooooh.  Indeed it would!\n> \n> I shall respin this series based around this better name.\n\nMore bikeshedding: Why are you calling it \"auto-\"? If I tell the program\nto \"--include-tags\", I expect it to happen automatically.\n\n-- Hannes\n\n"},{"id":"71034","messageId":"20080305063243.GG8410@spearce.org","threadId":"12477","inReplyTo":"47CCF9DA.4050304@viscovery.net","subject":"Re: [PATCH 1/3] git-pack-objects: Automatically pack annotated tags if object was packed","fromName":"Shawn O. Pearce","fromEmail":"spearce@spearce.org","sentAt":"2008-03-05T06:32:43Z","receivedAt":"2008-03-05T06:32:43Z","isPatch":true,"sender":{"key":"spearce@spearce.org","avatar":"https://avatars.githubusercontent.com/u/34844?v=4"},"body":"Johannes Sixt <j.sixt@viscovery.net> wrote:\n> Shawn O. Pearce schrieb:\n> > Nicolas Pitre <nico@cam.org> wrote:\n> >> On Mon, 3 Mar 2008, Shawn O. Pearce wrote:\n> >>\n> >>> The new option \"--auto-follow-tags\" allows the caller to request that\n> >>> any annotated tag be included into the packfile if the object the tag\n> >>> references was also included as part of the packfile.\n> >> Wouldn't \"auto-include-tag\" a better name for this option?\n> > \n> > Ooooh.  Indeed it would!\n> > \n> > I shall respin this series based around this better name.\n> \n> More bikeshedding: Why are you calling it \"auto-\"? If I tell the program\n> to \"--include-tags\", I expect it to happen automatically.\n\nBecause its more automatic than manually including tags?\nI don't know.\n\nJunio, if you haven't already merged this topic into someplace that\nI can't rewrite the series for a _3rd_ time, I might be willing to\nrespin it without the \"auto-\" prefix.  We are unlikely to ever have\na reason to support \"--manually-include-tags\".  Stone tablet based\ncomputing stopped being fun back in the '50s.\n\n-- \nShawn.\n"}]}