{"thread":{"id":"11805","subject":"git-push: forced update of tag shows unabbreviated SHA1","startedAt":"2008-01-31T09:27:43Z","lastAt":"2008-01-31T10:41:07Z","messageCount":9,"participants":["Johannes Sixt","Junio C Hamano","Jeff King"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"66970","messageId":"47A1948F.6080308@viscovery.net","threadId":"11805","inReplyTo":null,"subject":"git-push: forced update of tag shows unabbreviated SHA1","fromName":"Johannes Sixt","fromEmail":"j.sixt@viscovery.net","sentAt":"2008-01-31T09:27:43Z","receivedAt":"2008-01-31T09:27:43Z","isPatch":false,"sender":{"key":"j6t@kdbg.org","avatar":"https://avatars.githubusercontent.com/u/14810926?v=4"},"body":"This is just a cosmetical flaw:\n\nWhen a tag is changed to point to a new commit, then the tag is pushed to\na repo that still has the old tag, it is correctly pushed, but the old\nSHA1 is reported with all 40 digits:\n\n# make upstream repo\n$ mkdir A && cd A\n$ git init\n$ echo a > a; git add a; git commit -m a\n\n# clone, modify, push\n$ cd ..\n$ git clone A B\n$ cd B\n$ echo b > a; git commit -a -m b\n$ git push file://$PWD/../A\n\n# tag old state in upstream\n$ cd ../A\n$ git tag -m T T HEAD~1\n\n# tag new state in clone (same name) and force-push tag\n$ cd ../B\n$ git tag -m Tclone T\n$ git push -f file://$PWD/../A refs/tags/*:refs/tags/*\nCounting objects: 1, done.\nUnpacking objects: 100% (1/1), done.\nWriting objects: 100% (1/1), 160 bytes, done.\nTotal 1 (delta 0), reused 0 (delta 0)\nTo file:///home/jsixt/tmp/foo/B/../A\n + 639669ce44f84417f30842c622064827dda01461...475e55f T -> T (forced update)\n\nNotice that the original SHA1 is not abbreviated.\n\n-- Hannes\n"},{"id":"66971","messageId":"7vsl0ecp3i.fsf@gitster.siamese.dyndns.org","threadId":"11805","inReplyTo":"47A1948F.6080308@viscovery.net","subject":"Re: git-push: forced update of tag shows unabbreviated SHA1","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2008-01-31T09:37:53Z","receivedAt":"2008-01-31T09:37:53Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Johannes Sixt <j.sixt@viscovery.net> writes:\n\n> This is just a cosmetical flaw:\n> ...\n> To file:///home/jsixt/tmp/foo/B/../A\n>  + 639669ce44f84417f30842c622064827dda01461...475e55f T -> T (forced update)\n>\n> Notice that the original SHA1 is not abbreviated.\n\nI suspect that is because you do not _have_ the original object,\nso there is no uniquely usable abbreviation to name the object\nin your repository.\n\nThis obviously is not tested at all (not even compile tested),\nbut I think it would show you what is going on.\n\n---\n builtin-send-pack.c |    2 +-\n 1 files changed, 1 insertions(+), 1 deletions(-)\n\ndiff --git a/builtin-send-pack.c b/builtin-send-pack.c\nindex 8afb1d0..9c558ee 100644\n--- a/builtin-send-pack.c\n+++ b/builtin-send-pack.c\n@@ -265,7 +265,7 @@ static const char *status_abbrev(unsigned char sha1[20])\n {\n \tconst char *abbrev;\n \tabbrev = find_unique_abbrev(sha1, DEFAULT_ABBREV);\n-\treturn abbrev ? abbrev : sha1_to_hex(sha1);\n+\treturn abbrev ? abbrev : \"<you do not have it>\";\n }\n \n static void print_ok_ref_status(struct ref *ref)\n"},{"id":"66973","messageId":"47A19C7F.1060204@viscovery.net","threadId":"11805","inReplyTo":"7vsl0ecp3i.fsf@gitster.siamese.dyndns.org","subject":"Re: git-push: forced update of tag shows unabbreviated SHA1","fromName":"Johannes Sixt","fromEmail":"j.sixt@viscovery.net","sentAt":"2008-01-31T10:01:35Z","receivedAt":"2008-01-31T10:01:35Z","isPatch":false,"sender":{"key":"j6t@kdbg.org","avatar":"https://avatars.githubusercontent.com/u/14810926?v=4"},"body":"Junio C Hamano schrieb:\n> Johannes Sixt <j.sixt@viscovery.net> writes:\n> \n>> This is just a cosmetical flaw:\n>> ...\n>> To file:///home/jsixt/tmp/foo/B/../A\n>>  + 639669ce44f84417f30842c622064827dda01461...475e55f T -> T (forced update)\n>>\n>> Notice that the original SHA1 is not abbreviated.\n> \n> I suspect that is because you do not _have_ the original object,\n> so there is no uniquely usable abbreviation to name the object\n> in your repository.\n\nYes, you are right. This also happens for a commit.\n\n> This obviously is not tested at all (not even compile tested),\n> but I think it would show you what is going on.\n> \n> ---\n>  builtin-send-pack.c |    2 +-\n>  1 files changed, 1 insertions(+), 1 deletions(-)\n> \n> diff --git a/builtin-send-pack.c b/builtin-send-pack.c\n> index 8afb1d0..9c558ee 100644\n> --- a/builtin-send-pack.c\n> +++ b/builtin-send-pack.c\n> @@ -265,7 +265,7 @@ static const char *status_abbrev(unsigned char sha1[20])\n>  {\n>  \tconst char *abbrev;\n>  \tabbrev = find_unique_abbrev(sha1, DEFAULT_ABBREV);\n> -\treturn abbrev ? abbrev : sha1_to_hex(sha1);\n> +\treturn abbrev ? abbrev : \"<you do not have it>\";\n>  }\n\nNo, that would be information hiding. I prefer an unabbreviated name.\nNevertheless, if we know the name, we could be able to find a suitable\nabbreviation. But it's really not *that* important...\n\n-- Hannes\n"},{"id":"66974","messageId":"20080131100625.GB25546@coredump.intra.peff.net","threadId":"11805","inReplyTo":"47A1948F.6080308@viscovery.net","subject":"Re: git-push: forced update of tag shows unabbreviated SHA1","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2008-01-31T10:06:25Z","receivedAt":"2008-01-31T10:06:25Z","isPatch":false,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Thu, Jan 31, 2008 at 10:27:43AM +0100, Johannes Sixt wrote:\n\n> When a tag is changed to point to a new commit, then the tag is pushed to\n> a repo that still has the old tag, it is correctly pushed, but the old\n> SHA1 is reported with all 40 digits:\n\nAs Junio noted, this is because find_unique_abbrev returns NULL for\nobjects we don't have. Actually, it is somewhat worse -- we return an\nerroneous abbreviation in the rare case that we are trying to find the\nabbreviation for something we don't have, but for which we do have a\nmatching abbreviation. For example, if we have\n\n  1234567890123456789012345678901234567890\n\nthen for every 12345678* that we don't have, we will claim the correct\nabbreviation is 1234568.\n\nIn practice, I doubt this is a problem.\n\nBut getting back to your point: yes, I agree it is a little ugly.\nRewriting find_unique_abbrev would be necessary for fixing it, and I'm\nnot sure it is worth the trouble.\n\n-Peff\n"},{"id":"66976","messageId":"7vodb2cn2a.fsf@gitster.siamese.dyndns.org","threadId":"11805","inReplyTo":"20080131100625.GB25546@coredump.intra.peff.net","subject":"Re: git-push: forced update of tag shows unabbreviated SHA1","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2008-01-31T10:21:49Z","receivedAt":"2008-01-31T10:21:49Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jeff King <peff@peff.net> writes:\n\n> ... then for every 12345678* that we don't have, we will claim\n> the correct abbreviation is 1234568.\n>\n> In practice, I doubt this is a problem.\n>\n> But getting back to your point: yes, I agree it is a little ugly.\n> Rewriting find_unique_abbrev would be necessary for fixing it, and I'm\n> not sure it is worth the trouble.\n\nI think that needs to be done carefully.  I recall some callers\ndo expect it to return NULL for nonexistant objects, so the bug\nyou noted above as \"rare case\" may need to be fixed, which I\nthink is more important than coming up with a potentially too\nshort abbreviation.\n"},{"id":"66977","messageId":"20080131102716.GC25546@coredump.intra.peff.net","threadId":"11805","inReplyTo":"20080131100625.GB25546@coredump.intra.peff.net","subject":"Re: git-push: forced update of tag shows unabbreviated SHA1","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2008-01-31T10:27:16Z","receivedAt":"2008-01-31T10:27:16Z","isPatch":false,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Thu, Jan 31, 2008 at 05:06:25AM -0500, Jeff King wrote:\n\n> But getting back to your point: yes, I agree it is a little ugly.\n> Rewriting find_unique_abbrev would be necessary for fixing it, and I'm\n> not sure it is worth the trouble.\n\nActually, it looks like we already handle a similar case: the null sha1,\nso the change isn't that big (and the null sha1 case folds nicely into\nthe \"missing\" case). With the (lightly tested) patch below,\nfind_unique_abbrev _always_ returns an answer, and for missing objects\nwill show enough characters that it doesn't match any existing object.\n\ndiff --git a/sha1_name.c b/sha1_name.c\nindex 13e1164..c0e6af1 100644\n--- a/sha1_name.c\n+++ b/sha1_name.c\n@@ -192,26 +192,24 @@ static int get_short_sha1(const char *name, int len, unsigned char *sha1,\n \n const char *find_unique_abbrev(const unsigned char *sha1, int len)\n {\n-\tint status, is_null;\n+\tint status, missing;\n \tstatic char hex[41];\n \n-\tis_null = is_null_sha1(sha1);\n+\tmissing = !has_sha1_file(sha1);\n \tmemcpy(hex, sha1_to_hex(sha1), 40);\n \tif (len == 40 || !len)\n \t\treturn hex;\n \twhile (len < 40) {\n \t\tunsigned char sha1_ret[20];\n \t\tstatus = get_short_sha1(hex, len, sha1_ret, 1);\n-\t\tif (!status ||\n-\t\t    (is_null && status != SHORT_NAME_AMBIGUOUS)) {\n+\t\tif ((!missing && !status) ||\n+\t\t    (missing && status == SHORT_NAME_NOT_FOUND)) {\n \t\t\thex[len] = 0;\n \t\t\treturn hex;\n \t\t}\n-\t\tif (status != SHORT_NAME_AMBIGUOUS)\n-\t\t\treturn NULL;\n \t\tlen++;\n \t}\n-\treturn NULL;\n+\treturn hex;\n }\n \n static int ambiguous_path(const char *path, int len)\n"},{"id":"66979","messageId":"7vbq72cmaa.fsf@gitster.siamese.dyndns.org","threadId":"11805","inReplyTo":"20080131102716.GC25546@coredump.intra.peff.net","subject":"Re: git-push: forced update of tag shows unabbreviated SHA1","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2008-01-31T10:38:37Z","receivedAt":"2008-01-31T10:38:37Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jeff King <peff@peff.net> writes:\n\n> On Thu, Jan 31, 2008 at 05:06:25AM -0500, Jeff King wrote:\n>\n>> But getting back to your point: yes, I agree it is a little ugly.\n>> Rewriting find_unique_abbrev would be necessary for fixing it, and I'm\n>> not sure it is worth the trouble.\n>\n> Actually, it looks like we already handle a similar case: the null sha1,\n> so the change isn't that big (and the null sha1 case folds nicely into\n> the \"missing\" case).\n\nHeh, \"Make sure to abbreviate something that does not match\" ---\nI forgot about that trick I did looooooooong time ago ;-).\n\nI like the idea.  Does it not break anything?\n"},{"id":"66980","messageId":"20080131103947.GD25546@coredump.intra.peff.net","threadId":"11805","inReplyTo":"7vodb2cn2a.fsf@gitster.siamese.dyndns.org","subject":"Re: git-push: forced update of tag shows unabbreviated SHA1","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2008-01-31T10:39:48Z","receivedAt":"2008-01-31T10:39:48Z","isPatch":false,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Thu, Jan 31, 2008 at 02:21:49AM -0800, Junio C Hamano wrote:\n\n> I think that needs to be done carefully.  I recall some callers\n> do expect it to return NULL for nonexistant objects, so the bug\n> you noted above as \"rare case\" may need to be fixed, which I\n> think is more important than coming up with a potentially too\n> short abbreviation.\n\nThe patch I just posted just figures out ahead of time whether the\nobject exists. We can easily have it default to returning NULL but allow\na special flag for the new behavior. Something like:\n\ndiff --git a/builtin-send-pack.c b/builtin-send-pack.c\nindex 8afb1d0..454ad8f 100644\n--- a/builtin-send-pack.c\n+++ b/builtin-send-pack.c\n@@ -263,9 +263,8 @@ static void print_ref_status(char flag, const char *summary, struct ref *to, str\n \n static const char *status_abbrev(unsigned char sha1[20])\n {\n-\tconst char *abbrev;\n-\tabbrev = find_unique_abbrev(sha1, DEFAULT_ABBREV);\n-\treturn abbrev ? abbrev : sha1_to_hex(sha1);\n+\treturn find_unique_abbrev_flags(sha1, DEFAULT_ABBREV,\n+\t\t\tFIND_UNIQUE_ABBREV_MISSING);\n }\n \n static void print_ok_ref_status(struct ref *ref)\ndiff --git a/cache.h b/cache.h\nindex 7a38511..0bcbf54 100644\n--- a/cache.h\n+++ b/cache.h\n@@ -383,7 +383,14 @@ extern char *git_path(const char *fmt, ...) __attribute__((format (printf, 1, 2)\n extern char *sha1_file_name(const unsigned char *sha1);\n extern char *sha1_pack_name(const unsigned char *sha1);\n extern char *sha1_pack_index_name(const unsigned char *sha1);\n-extern const char *find_unique_abbrev(const unsigned char *sha1, int);\n+\n+#define FIND_UNIQUE_ABBREV_MISSING 1\n+extern const char *find_unique_abbrev_flags(const unsigned char *sha1, int len, int flags);\n+static inline const char *find_unique_abbrev(const unsigned char *sha1, int len)\n+{\n+\treturn find_unique_abbrev_flags(sha1, len, 0);\n+}\n+\n extern const unsigned char null_sha1[20];\n static inline int is_null_sha1(const unsigned char *sha1)\n {\ndiff --git a/sha1_name.c b/sha1_name.c\nindex a99aff3..666d36c 100644\n--- a/sha1_name.c\n+++ b/sha1_name.c\n@@ -190,12 +190,15 @@ static int get_short_sha1(const char *name, int len, unsigned char *sha1,\n \treturn status;\n }\n \n-const char *find_unique_abbrev(const unsigned char *sha1, int len)\n+const char *find_unique_abbrev_flags(const unsigned char *sha1, int len,\n+\t\tint flags)\n {\n \tint status, missing;\n \tstatic char hex[41];\n \n \tmissing = !has_sha1_file(sha1);\n+\tif (missing && !(flags & FIND_UNIQUE_ABBREV_MISSING))\n+\t\treturn NULL;\n \tmemcpy(hex, sha1_to_hex(sha1), 40);\n \tif (len == 40 || !len)\n \t\treturn hex;\n"},{"id":"66981","messageId":"20080131104107.GE25546@coredump.intra.peff.net","threadId":"11805","inReplyTo":"7vbq72cmaa.fsf@gitster.siamese.dyndns.org","subject":"Re: git-push: forced update of tag shows unabbreviated SHA1","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2008-01-31T10:41:07Z","receivedAt":"2008-01-31T10:41:07Z","isPatch":false,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Thu, Jan 31, 2008 at 02:38:37AM -0800, Junio C Hamano wrote:\n\n> > Actually, it looks like we already handle a similar case: the null sha1,\n> > so the change isn't that big (and the null sha1 case folds nicely into\n> > the \"missing\" case).\n> \n> Heh, \"Make sure to abbreviate something that does not match\" ---\n> I forgot about that trick I did looooooooong time ago ;-).\n> \n> I like the idea.  Does it not break anything?\n\nNo idea. :) I will hold onto it and submit a cleaned up version\npost-1.5.4.\n\n-Peff\n"}]}