{"thread":{"id":"26691","subject":"[PATCH 1/2] sha1_name: try to use same abbrev length when core.abbrevguard is specified","startedAt":"2011-03-08T10:59:10Z","lastAt":"2011-03-10T19:11:39Z","messageCount":13,"participants":["Namhyung Kim","Junio C Hamano"],"isPatch":true,"patchVersion":1,"patchTotal":2},"messages":[{"id":"162982","messageId":"1299581951-4670-1-git-send-email-namhyung@gmail.com","threadId":"26691","inReplyTo":null,"subject":"[PATCH 1/2] sha1_name: try to use same abbrev length when core.abbrevguard is specified","fromName":"Namhyung Kim","fromEmail":"namhyung@gmail.com","sentAt":"2011-03-08T10:59:10Z","receivedAt":"2011-03-08T10:59:10Z","isPatch":true,"sender":{"key":"namhyung@gmail.com","avatar":"https://avatars.githubusercontent.com/u/48503?v=4"},"body":"If find_unique_abbrev() finds a ambiguous SHA1 name, it tries\nto find again with increased length. In this case, result hex\nstrings could have different lengths even though the\ncore.abbrevguard config option is specified. But if the option\nis specified and increased length (delta) is less than its\nvalue, the result could be adjusted to the same length.\n\nSigned-off-by: Namhyung Kim <namhyung@gmail.com>\n---\n sha1_name.c |    5 ++++-\n 1 files changed, 4 insertions(+), 1 deletions(-)\n\ndiff --git a/sha1_name.c b/sha1_name.c\nindex 709ff2e..6bb8942 100644\n--- a/sha1_name.c\n+++ b/sha1_name.c\n@@ -197,6 +197,7 @@ const char *find_unique_abbrev(const unsigned char *sha1, int len)\n {\n \tint status, exists;\n \tstatic char hex[41];\n+\tint extra_len = unique_abbrev_extra_length;\n \n \texists = has_sha1_file(sha1);\n \tmemcpy(hex, sha1_to_hex(sha1), 40);\n@@ -208,12 +209,14 @@ const char *find_unique_abbrev(const unsigned char *sha1, int len)\n \t\tif (exists\n \t\t    ? !status\n \t\t    : status == SHORT_NAME_NOT_FOUND) {\n-\t\t\tint cut_at = len + unique_abbrev_extra_length;\n+\t\t\tint cut_at = len + extra_len;\n \t\t\tcut_at = (cut_at < 40) ? cut_at : 40;\n \t\t\thex[cut_at] = 0;\n \t\t\treturn hex;\n \t\t}\n \t\tlen++;\n+\t\tif (extra_len > 0)\n+\t\t\textra_len--;\n \t}\n \treturn hex;\n }\n-- \n1.7.4\n"},{"id":"162983","messageId":"1299581951-4670-2-git-send-email-namhyung@gmail.com","threadId":"26691","inReplyTo":"1299581951-4670-1-git-send-email-namhyung@gmail.com","subject":"[PATCH 2/2] blame: introduce -u/--unique option","fromName":"Namhyung Kim","fromEmail":"namhyung@gmail.com","sentAt":"2011-03-08T10:59:11Z","receivedAt":"2011-03-08T10:59:11Z","isPatch":true,"sender":{"key":"namhyung@gmail.com","avatar":"https://avatars.githubusercontent.com/u/48503?v=4"},"body":"-u/--unique option will find and use minimum length of unique\nSHA-1 name. If -l option is specified also, it will have higher\npriority, IOW git blame will use full 40-length SHA-1 name.\n\nSigned-off-by: Namhyung Kim <namhyung@gmail.com>\n---\n builtin/blame.c |   18 +++++++++++++++++-\n 1 files changed, 17 insertions(+), 1 deletions(-)\n\ndiff --git a/builtin/blame.c b/builtin/blame.c\nindex aa30ec5..9ea41bc 100644\n--- a/builtin/blame.c\n+++ b/builtin/blame.c\n@@ -41,6 +41,7 @@ static int reverse;\n static int blank_boundary;\n static int incremental;\n static int xdl_opts;\n+static int longest_uniq_sha1;\n \n static enum date_mode blame_date_mode = DATE_ISO8601;\n static size_t blame_date_width;\n@@ -1618,6 +1619,7 @@ static const char *format_time(unsigned long time, const char *tz_str,\n #define OUTPUT_SHOW_SCORE      0100\n #define OUTPUT_NO_AUTHOR       0200\n #define OUTPUT_SHOW_EMAIL\t0400\n+#define OUTPUT_UNIQ_OBJECT_NAME\t01000\n \n static void emit_porcelain(struct scoreboard *sb, struct blame_entry *ent)\n {\n@@ -1664,14 +1666,22 @@ static void emit_other(struct scoreboard *sb, struct blame_entry *ent, int opt)\n \tstruct commit_info ci;\n \tchar hex[41];\n \tint show_raw_time = !!(opt & OUTPUT_RAW_TIMESTAMP);\n+\tint sha1_len;\n \n \tget_commit_info(suspect->commit, &ci, 1);\n \tstrcpy(hex, sha1_to_hex(suspect->commit->object.sha1));\n \n+\tif (opt & OUTPUT_LONG_OBJECT_NAME)\n+\t\tsha1_len = 40;\n+\telse if (opt & OUTPUT_UNIQ_OBJECT_NAME)\n+\t\tsha1_len = longest_uniq_sha1;\n+\telse\n+\t\tsha1_len = 8;\n+\n \tcp = nth_line(sb, ent->lno);\n \tfor (cnt = 0; cnt < ent->num_lines; cnt++) {\n \t\tchar ch;\n-\t\tint length = (opt & OUTPUT_LONG_OBJECT_NAME) ? 40 : 8;\n+\t\tint length = sha1_len;\n \n \t\tif (suspect->commit->object.flags & UNINTERESTING) {\n \t\t\tif (blank_boundary)\n@@ -1842,6 +1852,7 @@ static void find_alignment(struct scoreboard *sb, int *option)\n \tfor (e = sb->ent; e; e = e->next) {\n \t\tstruct origin *suspect = e->suspect;\n \t\tstruct commit_info ci;\n+\t\tconst char *sha1;\n \t\tint num;\n \n \t\tif (strcmp(suspect->path, sb->path))\n@@ -1867,6 +1878,10 @@ static void find_alignment(struct scoreboard *sb, int *option)\n \t\t\tlongest_dst_lines = num;\n \t\tif (largest_score < ent_score(sb, e))\n \t\t\tlargest_score = ent_score(sb, e);\n+\t\tsha1 = find_unique_abbrev(suspect->commit->object.sha1,\n+\t\t\t\t\t  MINIMUM_ABBREV);\n+\t\tif (longest_uniq_sha1 < strlen(sha1))\n+\t\t\tlongest_uniq_sha1 = strlen(sha1);\n \t}\n \tmax_orig_digits = lineno_width(longest_src_lines);\n \tmax_digits = lineno_width(longest_dst_lines);\n@@ -2306,6 +2321,7 @@ int cmd_blame(int argc, const char **argv, const char *prefix)\n \t\tOPT_BIT('s', NULL, &output_option, \"Suppress author name and timestamp (Default: off)\", OUTPUT_NO_AUTHOR),\n \t\tOPT_BIT('e', \"show-email\", &output_option, \"Show author email instead of name (Default: off)\", OUTPUT_SHOW_EMAIL),\n \t\tOPT_BIT('w', NULL, &xdl_opts, \"Ignore whitespace differences\", XDF_IGNORE_WHITESPACE),\n+\t\tOPT_BIT('u', \"unique\", &output_option, \"Show minimum unique SHA-1 (Default: off)\", OUTPUT_UNIQ_OBJECT_NAME),\n \t\tOPT_STRING('S', NULL, &revs_file, \"file\", \"Use revisions from <file> instead of calling git-rev-list\"),\n \t\tOPT_STRING(0, \"contents\", &contents_from, \"file\", \"Use <file>'s contents as the final image\"),\n \t\t{ OPTION_CALLBACK, 'C', NULL, &opt, \"score\", \"Find line copies within and across files\", PARSE_OPT_OPTARG, blame_copy_callback },\n-- \n1.7.4\n"},{"id":"163068","messageId":"7vipvsaz27.fsf@alter.siamese.dyndns.org","threadId":"26691","inReplyTo":"1299581951-4670-1-git-send-email-namhyung@gmail.com","subject":"Re: [PATCH 1/2] sha1_name: try to use same abbrev length when core.abbrevguard is specified","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2011-03-09T19:12:00Z","receivedAt":"2011-03-09T19:12:00Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Namhyung Kim <namhyung@gmail.com> writes:\n\n> If find_unique_abbrev() finds a ambiguous SHA1 name, it tries\n> to find again with increased length. In this case, result hex\n> strings could have different lengths even though the\n> core.abbrevguard config option is specified. But if the option\n> is specified and increased length (delta) is less than its\n> value, the result could be adjusted to the same length.\n\nI am not sure if I can understand what problem you are trying to solve\nfrom the above description.\n\nThe function is given \"len\" from the caller to specify the minimum length\nof the output the caller expects (i.e. even if 4 hexdigits is enough to\nidentify the given commit in a small project, the caller can say it wants\nto see at least 7 hexdigits).  The loop without your patch finds the\nshortest prefix whose length is at least that given length that uniquely\nidentifies the given object (or the shortest prefix that doesn't identify\nany existing object if the given sha1 does not exist in the repository).\nAnd then ensures the returned value is longer by the guard as an extra\nsafety measure, so that later when the project grows, the disambiguation\nwe find today has a better chance to survive.\n\nWith this patch, the loop decreases the length of the guard when \"len\"\ngiven by the caller is insufficient to ensure uniqueness, which does not\nsound right.\n\nSuppose the given object has ambiguous other objects and you need 8\nhexdigits at least to make it unique in today's history.  The caller gives\nyou len of 7, and the guard is set to 3.\n\nWith the original code, the loop starts with 7, finds that it is not\nlong enough to disambiguate, increments and retries, finds that 8 is the\nshortest prefix, and then adds the guard and returns 11 hexdigits.\n\nWith your patch, the loop starts with 7 with extra set to 3, finds that 7\nis not long enough and decrements extra to 2, finds that 8 is the shortest\nprefix, and then returns only 10 hexdigits.\n\nWhich feels like totally going against the reason why we added the guard.\n\nWhat am I missing?\n\n>  sha1_name.c |    5 ++++-\n>  1 files changed, 4 insertions(+), 1 deletions(-)\n>\n> diff --git a/sha1_name.c b/sha1_name.c\n> index 709ff2e..6bb8942 100644\n> --- a/sha1_name.c\n> +++ b/sha1_name.c\n> @@ -197,6 +197,7 @@ const char *find_unique_abbrev(const unsigned char *sha1, int len)\n>  {\n>  \tint status, exists;\n>  \tstatic char hex[41];\n> +\tint extra_len = unique_abbrev_extra_length;\n>  \n>  \texists = has_sha1_file(sha1);\n>  \tmemcpy(hex, sha1_to_hex(sha1), 40);\n> @@ -208,12 +209,14 @@ const char *find_unique_abbrev(const unsigned char *sha1, int len)\n>  \t\tif (exists\n>  \t\t    ? !status\n>  \t\t    : status == SHORT_NAME_NOT_FOUND) {\n> -\t\t\tint cut_at = len + unique_abbrev_extra_length;\n> +\t\t\tint cut_at = len + extra_len;\n>  \t\t\tcut_at = (cut_at < 40) ? cut_at : 40;\n>  \t\t\thex[cut_at] = 0;\n>  \t\t\treturn hex;\n>  \t\t}\n>  \t\tlen++;\n> +\t\tif (extra_len > 0)\n> +\t\t\textra_len--;\n>  \t}\n>  \treturn hex;\n>  }\n"},{"id":"163069","messageId":"7vbp1kayoe.fsf@alter.siamese.dyndns.org","threadId":"26691","inReplyTo":"7vipvsaz27.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH 1/2] sha1_name: try to use same abbrev length when core.abbrevguard is specified","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2011-03-09T19:20:17Z","receivedAt":"2011-03-09T19:20:17Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Junio C Hamano <gitster@pobox.com> writes:\n\n> The function is given \"len\" from the caller to specify the minimum length\n> of the output the caller expects (i.e. even if 4 hexdigits is enough to\n> identify the given commit in a small project, the caller can say it wants\n> to see at least 7 hexdigits).  The loop without your patch finds the\n> shortest prefix whose length is at least that given length that uniquely\n> identifies the given object (or the shortest prefix that doesn't identify\n> any existing object if the given sha1 does not exist in the repository).\n> And then ensures the returned value is longer by the guard as an extra\n> safety measure, so that later when the project grows, the disambiguation\n> we find today has a better chance to survive.\n> ...\n> What am I missing?\n\nI think what may be desirable is to honor the caller-supplied \"len\" a bit\nbetter.  If an object is uniquely identifiable with only 4-hexdigit today,\nand if the caller gives 7 as len and the guard is set to 3, we return 10\nhexdigits with the current code.  We should instead return 7 hexdigits in\nsuch a case, as that is in line with the \"use 3 extra to give the\ndisambiguation we find today a better chance to survive\".\n\nPerhaps that was what you wanted to do, but I don't think that is what\nyour code actually does.\n\n>\n>>  sha1_name.c |    5 ++++-\n>>  1 files changed, 4 insertions(+), 1 deletions(-)\n>>\n>> diff --git a/sha1_name.c b/sha1_name.c\n>> index 709ff2e..6bb8942 100644\n>> --- a/sha1_name.c\n>> +++ b/sha1_name.c\n>> @@ -197,6 +197,7 @@ const char *find_unique_abbrev(const unsigned char *sha1, int len)\n>>  {\n>>  \tint status, exists;\n>>  \tstatic char hex[41];\n>> +\tint extra_len = unique_abbrev_extra_length;\n>>  \n>>  \texists = has_sha1_file(sha1);\n>>  \tmemcpy(hex, sha1_to_hex(sha1), 40);\n>> @@ -208,12 +209,14 @@ const char *find_unique_abbrev(const unsigned char *sha1, int len)\n>>  \t\tif (exists\n>>  \t\t    ? !status\n>>  \t\t    : status == SHORT_NAME_NOT_FOUND) {\n>> -\t\t\tint cut_at = len + unique_abbrev_extra_length;\n>> +\t\t\tint cut_at = len + extra_len;\n>>  \t\t\tcut_at = (cut_at < 40) ? cut_at : 40;\n>>  \t\t\thex[cut_at] = 0;\n>>  \t\t\treturn hex;\n>>  \t\t}\n>>  \t\tlen++;\n>> +\t\tif (extra_len > 0)\n>> +\t\t\textra_len--;\n>>  \t}\n>>  \treturn hex;\n>>  }\n"},{"id":"163072","messageId":"7vy64o9ixz.fsf@alter.siamese.dyndns.org","threadId":"26691","inReplyTo":"1299581951-4670-2-git-send-email-namhyung@gmail.com","subject":"Re: [PATCH 2/2] blame: introduce -u/--unique option","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2011-03-09T19:45:28Z","receivedAt":"2011-03-09T19:45:28Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Namhyung Kim <namhyung@gmail.com> writes:\n\n> -u/--unique option will find and use minimum length of unique\n> SHA-1 name. If -l option is specified also, it will have higher\n> priority, IOW git blame will use full 40-length SHA-1 name.\n\n> @@ -1867,6 +1878,10 @@ static void find_alignment(struct scoreboard *sb, int *option)\n>  \t\t\tlongest_dst_lines = num;\n>  \t\tif (largest_score < ent_score(sb, e))\n>  \t\t\tlargest_score = ent_score(sb, e);\n> +\t\tsha1 = find_unique_abbrev(suspect->commit->object.sha1,\n> +\t\t\t\t\t  MINIMUM_ABBREV);\n> +\t\tif (longest_uniq_sha1 < strlen(sha1))\n> +\t\t\tlongest_uniq_sha1 = strlen(sha1);\n\nThe logic to determine and keep track of the longuest-unique looks\ncorrect, but I was hoping that we already have an easy optimization\ncodepath to do this only once per commit, not for every blame-entry in the\nresult.  Doesn't the code have a similar optimization to figure out the\nnecessary number of columns to show author names (I haven't read the code\nrecently, though)?\n\nAlso we might find that the performance impact of doing this may be so\nminiscule that it is not worth wasting a short option name.  If we were to\nuse an option, I was actually hoping that the option would let the users\nspecify a value different from the hardcoded 8 at the same time.  E.g.\n\n    git blame --abbrev=8 ;# current default with uniquefy applied\n    git blame --abbrev=4 ;# equivalent to your \"blame -u\"\n\nCan we have a benchmark of this feature in a largish and busy file in a\nproject with a deep history?\n"},{"id":"163110","messageId":"1299734855.1496.9.camel@leonhard","threadId":"26691","inReplyTo":"7vipvsaz27.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH 1/2] sha1_name: try to use same abbrev length when core.abbrevguard is specified","fromName":"Namhyung Kim","fromEmail":"namhyung@gmail.com","sentAt":"2011-03-10T05:27:35Z","receivedAt":"2011-03-10T05:27:35Z","isPatch":true,"sender":{"key":"namhyung@gmail.com","avatar":"https://avatars.githubusercontent.com/u/48503?v=4"},"body":"2011-03-09 (수), 11:12 -0800, Junio C Hamano: \n> Namhyung Kim <namhyung@gmail.com> writes:\n> \n> > If find_unique_abbrev() finds a ambiguous SHA1 name, it tries\n> > to find again with increased length. In this case, result hex\n> > strings could have different lengths even though the\n> > core.abbrevguard config option is specified. But if the option\n> > is specified and increased length (delta) is less than its\n> > value, the result could be adjusted to the same length.\n> \n> I am not sure if I can understand what problem you are trying to solve\n> from the above description.\n> \n\nThat's probably because of my poor English. :(\n\n\n> The function is given \"len\" from the caller to specify the minimum length\n> of the output the caller expects (i.e. even if 4 hexdigits is enough to\n> identify the given commit in a small project, the caller can say it wants\n> to see at least 7 hexdigits).  The loop without your patch finds the\n> shortest prefix whose length is at least that given length that uniquely\n> identifies the given object (or the shortest prefix that doesn't identify\n> any existing object if the given sha1 does not exist in the repository).\n> And then ensures the returned value is longer by the guard as an extra\n> safety measure, so that later when the project grows, the disambiguation\n> we find today has a better chance to survive.\n> \n> With this patch, the loop decreases the length of the guard when \"len\"\n> given by the caller is insufficient to ensure uniqueness, which does not\n> sound right.\n> \n> Suppose the given object has ambiguous other objects and you need 8\n> hexdigits at least to make it unique in today's history.  The caller gives\n> you len of 7, and the guard is set to 3.\n> \n> With the original code, the loop starts with 7, finds that it is not\n> long enough to disambiguate, increments and retries, finds that 8 is the\n> shortest prefix, and then adds the guard and returns 11 hexdigits.\n> \n> With your patch, the loop starts with 7 with extra set to 3, finds that 7\n> is not long enough and decrements extra to 2, finds that 8 is the shortest\n> prefix, and then returns only 10 hexdigits.\n> \n> Which feels like totally going against the reason why we added the guard.\n> \n> What am I missing?\n> \n\nWhat I was thinking is like below (from a user's point of view):\n\nI knew git use 7 hexdigit to represent a commit object and it was not\nenough for my project. So I gave it 5 for the guard and expected to see\n12 in everywhere - if 12 always guarantees uniqueness for the project\nnow and probably, the near future. But sometimes git prints 13 even\nthough 12 is long enough to represent the commit uniquely. Because 7 is\nnot enough but 8 is, and simply add 5 (the guard) to it.\n\nSo I thought it would be natural to print 12 always in this case and\nthus, submitted the patch.\n\nThanks.\n\n-- \nRegards,\nNamhyung Kim\n"},{"id":"163112","messageId":"1299737621.1496.55.camel@leonhard","threadId":"26691","inReplyTo":"7vy64o9ixz.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH 2/2] blame: introduce -u/--unique option","fromName":"Namhyung Kim","fromEmail":"namhyung@gmail.com","sentAt":"2011-03-10T06:13:41Z","receivedAt":"2011-03-10T06:13:41Z","isPatch":true,"sender":{"key":"namhyung@gmail.com","avatar":"https://avatars.githubusercontent.com/u/48503?v=4"},"body":"2011-03-09 (수), 11:45 -0800, Junio C Hamano:\n> Namhyung Kim <namhyung@gmail.com> writes:\n> \n> > -u/--unique option will find and use minimum length of unique\n> > SHA-1 name. If -l option is specified also, it will have higher\n> > priority, IOW git blame will use full 40-length SHA-1 name.\n> \n> > @@ -1867,6 +1878,10 @@ static void find_alignment(struct scoreboard *sb, int *option)\n> >  \t\t\tlongest_dst_lines = num;\n> >  \t\tif (largest_score < ent_score(sb, e))\n> >  \t\t\tlargest_score = ent_score(sb, e);\n> > +\t\tsha1 = find_unique_abbrev(suspect->commit->object.sha1,\n> > +\t\t\t\t\t  MINIMUM_ABBREV);\n> > +\t\tif (longest_uniq_sha1 < strlen(sha1))\n> > +\t\t\tlongest_uniq_sha1 = strlen(sha1);\n> \n> The logic to determine and keep track of the longuest-unique looks\n> correct, but I was hoping that we already have an easy optimization\n> codepath to do this only once per commit, not for every blame-entry in the\n> result.  Doesn't the code have a similar optimization to figure out the\n> necessary number of columns to show author names (I haven't read the code\n> recently, though)?\n> \n\nRight. METAINFO_SHOWN flag does that. I'll move the code under the block\nin next version.\n\n\n> Also we might find that the performance impact of doing this may be so\n> miniscule that it is not worth wasting a short option name.  If we were to\n> use an option, I was actually hoping that the option would let the users\n> specify a value different from the hardcoded 8 at the same time.  E.g.\n> \n>     git blame --abbrev=8 ;# current default with uniquefy applied\n>     git blame --abbrev=4 ;# equivalent to your \"blame -u\"\n> \n\nHmm... I thought about that too. But you mean that you only want\n--abbrev instead of -u, right?\n\nMy original intention was if user specified --abbrev explicitly, it\nshould be honored regardless of the uniqueness. The guard will not be\nused in this situation because the user gave the exact length [s]he\nwants to see.\n\n\n> Can we have a benchmark of this feature in a largish and busy file in a\n> project with a deep history?\n> \n\nI gave it a try on a file in Linux kernel (with METAINFO_SHOWN\noptimization applied):\n\n$ wc -l mm/page_alloc.c\n5629 mm/page_alloc.c\n$ git log --oneline mm/page_alloc.c | wc -l\n566\n$ perf record ../git/git blame -u mm/page_alloc.c > /dev/null\n[ perf record: Woken up 1 times to write data ]\n[ perf record: Captured and wrote 0.229 MB perf.data (~10014 samples) ]\n$ perf report\n# Events: 5K cycles\n#\n# Overhead  Command         Shared Object                         Symbol\n# ........  .......  ....................  .............................\n#\n    21.11%      git  git                   [.] cmd_blame\n    18.34%      git  git                   [.] get_origin\n    18.32%      git  git                   [.] pass_blame\n     3.99%      git  libz.so.1.2.3.3       [.] inflate\n     3.14%      git  git                   [.] xdl_hash_record\n     2.84%      git  libz.so.1.2.3.3       [.] inflate_table\n     2.60%      git  libz.so.1.2.3.3       [.] inflate_fast\n     2.23%      git  git                   [.] find_pack_entry_one\n     1.84%      git  git                   [.] xdl_recmatch\n     1.51%      git  libc-2.11.1.so        [.] memcpy\n     1.49%      git  git                   [.] xdl_prepare_env\n     1.36%      git  git                   [.] xdl_prepare_ctx\n     1.13%      git  [kernel.kallsyms]     [k] __lock_acquire\n     1.12%      git  git                   [.] xdi_diff\n     1.09%      git  git                   [.] blame_chunk\n     0.99%      git  libc-2.11.1.so        [.] _int_malloc\n     0.79%      git  git                   [.] tree_entry_interesting\n     0.77%      git  git                   [.] origin_decref\n     0.67%      git  libz.so.1.2.3.3       [.] adler32\n     0.42%      git  [kernel.kallsyms]     [k] sched_clock_local\n     0.40%      git  git                   [.] decode_tree_entry\n     0.38%      git  [kernel.kallsyms]     [k] clear_page_c\n     0.35%      git  libc-2.11.1.so        [.] __GI___strncmp_ssse3\n     0.32%      git  git                   [.] lookup_object\n     0.31%      git  [kernel.kallsyms]     [k] lock_release\n     0.29%      git  [kernel.kallsyms]     [k] hlock_class\n     0.29%      git  [kernel.kallsyms]     [k] page_fault\n     0.29%      git  git                   [.] patch_delta\n     0.28%      git  [kernel.kallsyms]     [k] local_clock\n     0.27%      git  [kernel.kallsyms]     [k] sched_clock\n     0.25%      git  [kernel.kallsyms]     [k] delay_tsc\n     0.25%      git  git                   [.] parse_commit_buffer\n     0.25%      git  [kernel.kallsyms]     [k] look_up_lock_class\n     0.23%      git  git                   [.] xdl_cha_alloc\n     0.22%      git  libc-2.11.1.so        [.] _int_free\n     0.22%      git  libc-2.11.1.so        [.] malloc_consolidate\n     0.22%      git  [kernel.kallsyms]     [k] mark_lock\n     0.21%      git  libc-2.11.1.so        [.] __malloc\n     0.20%      git  git                   [.] nth_packed_object_offset\n\nLooks like it doesn't have a performance impact IMHO.\nHope this helps.\n\nThank you.\n\n\n-- \nRegards,\nNamhyung Kim\n"},{"id":"163121","messageId":"7vlj0n5o3n.fsf_-_@alter.siamese.dyndns.org","threadId":"26691","inReplyTo":"7vbp1kayoe.fsf@alter.siamese.dyndns.org","subject":"Re* [PATCH 1/2] sha1_name: try to use same abbrev length when core.abbrevguard is specified","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2011-03-10T09:19:56Z","receivedAt":"2011-03-10T09:19:56Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Junio C Hamano <gitster@pobox.com> writes:\n\n> I think what may be desirable is to honor the caller-supplied \"len\" a bit\n> better.  If an object is uniquely identifiable with only 4-hexdigit today,\n> and if the caller gives 7 as len and the guard is set to 3, we return 10\n> hexdigits with the current code.  We should instead return 7 hexdigits in\n> such a case, as that is in line with the \"use 3 extra to give the\n> disambiguation we find today a better chance to survive\".\n\nAnd here is an attempt to do so.  I have to admit that I didn't give it\ntoo much thought, though, so please be careful when reviewing the logic.\n\n-- >8 --\nSubject: find_unique_abbrev(): honor caller-supplied \"len\" better\n\nThe caller of this function wants to ensure that the returned string is a\nunique abbreviation of the object name, and at least len characters long.\nWhen \"len\" is sufficiently short and we cannot ensure uniqueness with only\n\"len\" bytes, we returned minimally unique prefix (i.e. if you dropped the\nlast character, there would be two or more objects that share that same\nprefix as their names in the repository).\n\nAn earlier change introduced core.abbrevguard configuration with a\nrealization that a short prefix that is unique today may not stay unique\nforever, as new objects are added to the repository. When \"len\" is shorter\nthan the length necessary to ensure uniqueness today, instead of returning\na string that is only one character longer than the longest ambiguous\nprefix, we wanted to add that many extra characters to ensure uniqueness\nfor longer time.\n\nHowever, the code forgot that \"len\" may be sufficiently long.  If an\nobject is uniquely identifiable with only 4-hexdigit today, and if the\ncaller gives 7 as len and the guard is set to 3, we returned 10 hexdigits,\nwhich was 3 characters longer than necessary.  We should instead return 7\nhexdigits in such a case, as that is in line with the original intention\nof using 3 extra hexdigits to give the disambiguation we find today a\nbetter chance to survive.\n\nSigned-off-by: Junio C Hamano <gitster@pobox.com>\n---\n sha1_name.c |   24 ++++++++++++++++++++++++\n 1 files changed, 24 insertions(+), 0 deletions(-)\n\ndiff --git a/sha1_name.c b/sha1_name.c\nindex 709ff2e..0f81581 100644\n--- a/sha1_name.c\n+++ b/sha1_name.c\n@@ -193,6 +193,23 @@ static int get_short_sha1(const char *name, int len, unsigned char *sha1,\n \treturn status;\n }\n \n+/*\n+ * The caller wants a unique abbreviation of the full object name in\n+ * \"sha1\" that is at least \"len\" characters long.\n+ *\n+ * (1) If sha1 identifies an existing object, there must be no other\n+ *     object that shares the returned string as the prefix of its\n+ *     name;\n+ *\n+ * (2) If no object with the given object name exists, there must be\n+ *     no object that has the returned string as the prefix of its\n+ *     name.\n+ *\n+ * Usually we try to return as short a string as possible, but the\n+ * core.abbrevguard configuration may tell us to use at least that\n+ * many characters more than necessary to make the result unique,\n+ * in order to keep it unique a bit longer.\n+ */\n const char *find_unique_abbrev(const unsigned char *sha1, int len)\n {\n \tint status, exists;\n@@ -202,6 +219,13 @@ const char *find_unique_abbrev(const unsigned char *sha1, int len)\n \tmemcpy(hex, sha1_to_hex(sha1), 40);\n \tif (len == 40 || !len)\n \t\treturn hex;\n+\tlen -= unique_abbrev_extra_length;\n+\tif (len <= 0)\n+\t\tlen = 1;\n+\t/*\n+\t * Try to see how short a prefix we can feed to get\n+\t * the desired unique hit\n+\t */\n \twhile (len < 40) {\n \t\tunsigned char sha1_ret[20];\n \t\tstatus = get_short_sha1(hex, len, sha1_ret, 1);\n"},{"id":"163124","messageId":"7vhbbb5mzo.fsf@alter.siamese.dyndns.org","threadId":"26691","inReplyTo":"1299737621.1496.55.camel@leonhard","subject":"Re: [PATCH 2/2] blame: introduce -u/--unique option","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2011-03-10T09:43:55Z","receivedAt":"2011-03-10T09:43:55Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Namhyung Kim <namhyung@gmail.com> writes:\n\n> Hmm... I thought about that too. But you mean that you only want\n> --abbrev instead of -u, right?\n\nYeah, if it can be shown that ensuring the uniqueness in addition to\nuniform length with negligible cost, I don't think there is any reason to\nhave this as an option.  So perhaps the minimum change would be to keep\nthe current \"8\" as a hardwired constant to feed find-unique-abbrev with to\nfind the longest unique prefix as your patch did and use that length when\nshowing the output.  Making \"8\" configurable would become a separate\ntopic.\n\n> My original intention was if user specified --abbrev explicitly, it\n> should be honored regardless of the uniqueness. The guard will not be\n> used in this situation because the user gave the exact length [s]he\n> wants to see.\n\nHmm, like giving a ridiculously short --abbrev and forcing the output to\nbe ambiguous yet hopefully they can be differenciated by other clues like\nauthor and date?\n\nThe reason I originally suggested to make the uniqueness an optional\nfeature was primarily output performance, but that is linear with the\nnumber of lines in the file and is dwarfed by the history traversal cost,\nso it is a non-issue, so I then suggested that it doesn't need to be an\noption.  But I think you may have a point--an option to decline uniqueness\nwould also make sense, especially when you want a very short prefix that\nyou know won't be unique and when you don't care about uniqueness.\n\nAs all the normal commands that use --abbrev would call f-u-a to ensure\nuniqueness, the current \"cut at fixed length\" behaviour is an oddball.  It\nshould become an optional behaviour.\n\nThe current behaviour, when we eventually have both options, would be:\n\n\t$ git blame --abbrev=8 --no-uniq $rev -- $path\n\nand should be the default when neither --abbrev=$n nor --no-uniq is given\nfor backward compatibility.  Porcelains are _not_ supposed to be reading\nfrom blame output without giving it --porcelain option, but we know we\ncannot expect sanity from people who script around git.\n\nWhen you want to give a very short width that wouldn't guarantee\nuniqueness, and you want to force that width, you would say:\n\n\t$ git blame --abbrev=4 --no-uniq $rev -- $path\n\nWithout --no-uniq, \n\n\t$ git blame --abbrev=4 $rev -- $path\n\nthe command may use more hexdigits than 4 to ensure uniqueness, but all\ncommit names are shown with the same width to ensure alignment as well.\n\nI kind of like that.  While I don't think \"--no-uniq\" is the best name for\nthat option, I think --abbrev everywhere else implies uniqueness so it is\nnot a good idea to require \"-u\" option for unique output and give\nambiguous output without one.  It would hurt consistency in the UI.\n"},{"id":"163133","messageId":"1299757966.1499.34.camel@leonhard","threadId":"26691","inReplyTo":"7vlj0n5o3n.fsf_-_@alter.siamese.dyndns.org","subject":"Re: Re* [PATCH 1/2] sha1_name: try to use same abbrev length when core.abbrevguard is specified","fromName":"Namhyung Kim","fromEmail":"namhyung@gmail.com","sentAt":"2011-03-10T11:52:46Z","receivedAt":"2011-03-10T11:52:46Z","isPatch":true,"sender":{"key":"namhyung@gmail.com","avatar":"https://avatars.githubusercontent.com/u/48503?v=4"},"body":"2011-03-10 (목), 01:19 -0800, Junio C Hamano:\n> Junio C Hamano <gitster@pobox.com> writes:\n> \n> > I think what may be desirable is to honor the caller-supplied \"len\" a bit\n> > better.  If an object is uniquely identifiable with only 4-hexdigit today,\n> > and if the caller gives 7 as len and the guard is set to 3, we return 10\n> > hexdigits with the current code.  We should instead return 7 hexdigits in\n> > such a case, as that is in line with the \"use 3 extra to give the\n> > disambiguation we find today a better chance to survive\".\n> \n> And here is an attempt to do so.  I have to admit that I didn't give it\n> too much thought, though, so please be careful when reviewing the logic.\n> \n\nWhat if the unique length is greater than or equal to the given length?\nFor instance the unique length is 7 and the caller gives 7 and the guard\nis 3. What do you want to return, 7 or 10? How about the unique length\nof 8?\n\nI think the meaning of the guard is somewhat vague. When this feature\nwas considered in LKML at first, Linus just wanted to change the default\nlength of commit abbreviation to 12 by making it user-configurable. [1]\nAnd this is the same situation what I tried to tell you in the previous\nemail.\n\nSo I think it would be better to choose the output length using only\ncaller-given length and the guard length if it guarantees the uniqueness\ntoday. It'll be simple, consistent and expected behavior IMHO.\n\nThanks.\n\n\n> -- >8 --\n> Subject: find_unique_abbrev(): honor caller-supplied \"len\" better\n> \n> The caller of this function wants to ensure that the returned string is a\n> unique abbreviation of the object name, and at least len characters long.\n> When \"len\" is sufficiently short and we cannot ensure uniqueness with only\n> \"len\" bytes, we returned minimally unique prefix (i.e. if you dropped the\n> last character, there would be two or more objects that share that same\n> prefix as their names in the repository).\n> \n> An earlier change introduced core.abbrevguard configuration with a\n> realization that a short prefix that is unique today may not stay unique\n> forever, as new objects are added to the repository. When \"len\" is shorter\n> than the length necessary to ensure uniqueness today, instead of returning\n> a string that is only one character longer than the longest ambiguous\n> prefix, we wanted to add that many extra characters to ensure uniqueness\n> for longer time.\n> \n> However, the code forgot that \"len\" may be sufficiently long.  If an\n> object is uniquely identifiable with only 4-hexdigit today, and if the\n> caller gives 7 as len and the guard is set to 3, we returned 10 hexdigits,\n> which was 3 characters longer than necessary.  We should instead return 7\n> hexdigits in such a case, as that is in line with the original intention\n> of using 3 extra hexdigits to give the disambiguation we find today a\n> better chance to survive.\n> \n> Signed-off-by: Junio C Hamano <gitster@pobox.com>\n> ---\n>  sha1_name.c |   24 ++++++++++++++++++++++++\n>  1 files changed, 24 insertions(+), 0 deletions(-)\n> \n> diff --git a/sha1_name.c b/sha1_name.c\n> index 709ff2e..0f81581 100644\n> --- a/sha1_name.c\n> +++ b/sha1_name.c\n> @@ -193,6 +193,23 @@ static int get_short_sha1(const char *name, int len, unsigned char *sha1,\n>  \treturn status;\n>  }\n>  \n> +/*\n> + * The caller wants a unique abbreviation of the full object name in\n> + * \"sha1\" that is at least \"len\" characters long.\n> + *\n> + * (1) If sha1 identifies an existing object, there must be no other\n> + *     object that shares the returned string as the prefix of its\n> + *     name;\n> + *\n> + * (2) If no object with the given object name exists, there must be\n> + *     no object that has the returned string as the prefix of its\n> + *     name.\n> + *\n> + * Usually we try to return as short a string as possible, but the\n> + * core.abbrevguard configuration may tell us to use at least that\n> + * many characters more than necessary to make the result unique,\n> + * in order to keep it unique a bit longer.\n> + */\n>  const char *find_unique_abbrev(const unsigned char *sha1, int len)\n>  {\n>  \tint status, exists;\n> @@ -202,6 +219,13 @@ const char *find_unique_abbrev(const unsigned char *sha1, int len)\n>  \tmemcpy(hex, sha1_to_hex(sha1), 40);\n>  \tif (len == 40 || !len)\n>  \t\treturn hex;\n> +\tlen -= unique_abbrev_extra_length;\n> +\tif (len <= 0)\n> +\t\tlen = 1;\n\nAnyway, how about\n\n\t  if (len < MINIMUM_ABBREV)\n\t\t  len = MINIMUM_ABBREV;\n\n\n> +\t/*\n> +\t * Try to see how short a prefix we can feed to get\n> +\t * the desired unique hit\n> +\t */\n>  \twhile (len < 40) {\n>  \t\tunsigned char sha1_ret[20];\n>  \t\tstatus = get_short_sha1(hex, len, sha1_ret, 1);\n\n\n-- \nRegards,\nNamhyung Kim\n"},{"id":"163134","messageId":"1299758084.1499.36.camel@leonhard","threadId":"26691","inReplyTo":"1299757966.1499.34.camel@leonhard","subject":"Re: Re* [PATCH 1/2] sha1_name: try to use same abbrev length when core.abbrevguard is specified","fromName":"Namhyung Kim","fromEmail":"namhyung@gmail.com","sentAt":"2011-03-10T11:54:44Z","receivedAt":"2011-03-10T11:54:44Z","isPatch":true,"sender":{"key":"namhyung@gmail.com","avatar":"https://avatars.githubusercontent.com/u/48503?v=4"},"body":"2011-03-10 (목), 20:52 +0900, Namhyung Kim:\n> 2011-03-10 (목), 01:19 -0800, Junio C Hamano:\n> > And here is an attempt to do so.  I have to admit that I didn't give it\n> > too much thought, though, so please be careful when reviewing the logic.\n> > \n> \n> What if the unique length is greater than or equal to the given length?\n> For instance the unique length is 7 and the caller gives 7 and the guard\n> is 3. What do you want to return, 7 or 10? How about the unique length\n> of 8?\n> \n> I think the meaning of the guard is somewhat vague. When this feature\n> was considered in LKML at first, Linus just wanted to change the default\n> length of commit abbreviation to 12 by making it user-configurable. [1]\n> And this is the same situation what I tried to tell you in the previous\n> email.\n> \n\nOh, I missed the link. :)\n\n[1] https://lkml.org/lkml/2010/10/28/264\n\n\n-- \nRegards,\nNamhyung Kim\n"},{"id":"163137","messageId":"1299765516.1499.150.camel@leonhard","threadId":"26691","inReplyTo":"7vhbbb5mzo.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH 2/2] blame: introduce -u/--unique option","fromName":"Namhyung Kim","fromEmail":"namhyung@gmail.com","sentAt":"2011-03-10T13:58:36Z","receivedAt":"2011-03-10T13:58:36Z","isPatch":true,"sender":{"key":"namhyung@gmail.com","avatar":"https://avatars.githubusercontent.com/u/48503?v=4"},"body":"2011-03-10 (목), 01:43 -0800, Junio C Hamano:\n> Namhyung Kim <namhyung@gmail.com> writes:\n> \n> > Hmm... I thought about that too. But you mean that you only want\n> > --abbrev instead of -u, right?\n> \n> Yeah, if it can be shown that ensuring the uniqueness in addition to\n> uniform length with negligible cost, I don't think there is any reason to\n> have this as an option.  So perhaps the minimum change would be to keep\n> the current \"8\" as a hardwired constant to feed find-unique-abbrev with to\n> find the longest unique prefix as your patch did and use that length when\n> showing the output.  Making \"8\" configurable would become a separate\n> topic.\n> \n\nOK. But I'm still wonder why the value is \"8\" instead of\n\"7\" (DEFAULT_ABBREV).\n\n\n> > My original intention was if user specified --abbrev explicitly, it\n> > should be honored regardless of the uniqueness. The guard will not be\n> > used in this situation because the user gave the exact length [s]he\n> > wants to see.\n> \n> Hmm, like giving a ridiculously short --abbrev and forcing the output to\n> be ambiguous yet hopefully they can be differenciated by other clues like\n> author and date?\n> \n> The reason I originally suggested to make the uniqueness an optional\n> feature was primarily output performance, but that is linear with the\n> number of lines in the file and is dwarfed by the history traversal cost,\n> so it is a non-issue, so I then suggested that it doesn't need to be an\n> option.  But I think you may have a point--an option to decline uniqueness\n> would also make sense, especially when you want a very short prefix that\n> you know won't be unique and when you don't care about uniqueness.\n> \n> As all the normal commands that use --abbrev would call f-u-a to ensure\n> uniqueness, the current \"cut at fixed length\" behaviour is an oddball.  It\n> should become an optional behaviour.\n> \n> The current behaviour, when we eventually have both options, would be:\n> \n> \t$ git blame --abbrev=8 --no-uniq $rev -- $path\n> \n> and should be the default when neither --abbrev=$n nor --no-uniq is given\n> for backward compatibility.  Porcelains are _not_ supposed to be reading\n> from blame output without giving it --porcelain option, but we know we\n> cannot expect sanity from people who script around git.\n> \n> When you want to give a very short width that wouldn't guarantee\n> uniqueness, and you want to force that width, you would say:\n> \n> \t$ git blame --abbrev=4 --no-uniq $rev -- $path\n> \n> Without --no-uniq, \n> \n> \t$ git blame --abbrev=4 $rev -- $path\n> \n> the command may use more hexdigits than 4 to ensure uniqueness, but all\n> commit names are shown with the same width to ensure alignment as well.\n> \n\nRight. But I'm not sure what the end result should be if the guard is\nset and/or --no-uniq is given. It might be related to the patch 1/2.\n\nSay the unique length is 4 and the guard is 3. What do you expect from\nthe following command?\n\n\t$ git blame --abbrev=5 -- $path\n\t$ git blame --abbrev=5 --no-uniq -- $path\n\nI think it should be 8 and 5. But possible alternatives are (7, 5),\n(5, 5), (8, 8) and (7, 8).\n\nLet me think of another example. In this case the unique length is 8,\nthe guard is 4 and the commands are same.\n\nI think it should be (9, 5). And corresponding alternatives are (12, 5),\n(8, 5), (12, 9) and (12, 9).\n\n\nMy logic can be described like:\n\n\tdefault output \t= max((c + g), u)\n\tno-uniq output \t= c\n\nwhere 'l' is the caller-given length, 'g' is the guard length, and 'u'\nis the unique length.\n\nAlternatives are:\n\n(a)\tdefault output = max((u + g), c)\n\tno-uniq output = c\n\n(b)\tdefault output = max(c, u)\n\tno-uniq output = c\n\n(c)\tdefault output = (c > u) ? (c + g) : (u + g)\n\tno-uniq output = c + g\n\n(d)\tdefault output = max((u + g), c)\n\tno-uniq output = c + g\n\n\nMaybe we can make more combination if you want. What do you think?\n\n\n\n> I kind of like that.  While I don't think \"--no-uniq\" is the best name for\n> that option, I think --abbrev everywhere else implies uniqueness so it is\n> not a good idea to require \"-u\" option for unique output and give\n> ambiguous output without one.  It would hurt consistency in the UI.\n> \n\nHow about \"--fixed\" ?\n\nThanks.\n\n\n-- \nRegards,\nNamhyung Kim\n"},{"id":"163165","messageId":"7vwrk64wpg.fsf@alter.siamese.dyndns.org","threadId":"26691","inReplyTo":"1299757966.1499.34.camel@leonhard","subject":"Re: Re* [PATCH 1/2] sha1_name: try to use same abbrev length when core.abbrevguard is specified","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2011-03-10T19:11:39Z","receivedAt":"2011-03-10T19:11:39Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Namhyung Kim <namhyung@gmail.com> writes:\n\n> 2011-03-10 (목), 01:19 -0800, Junio C Hamano:\n>> Junio C Hamano <gitster@pobox.com> writes:\n>> \n>> > I think what may be desirable is to honor the caller-supplied \"len\" a bit\n>> > better.  If an object is uniquely identifiable with only 4-hexdigit today,\n>> > and if the caller gives 7 as len and the guard is set to 3, we return 10\n>> > hexdigits with the current code.  We should instead return 7 hexdigits in\n>> > such a case, as that is in line with the \"use 3 extra to give the\n>> > disambiguation we find today a better chance to survive\".\n>> \n>> And here is an attempt to do so.  I have to admit that I didn't give it\n>> too much thought, though, so please be careful when reviewing the logic.\n>> \n>\n> What if the unique length is greater than or equal to the given length?\n> For instance the unique length is 7 and the caller gives 7 and the guard\n> is 3. What do you want to return, 7 or 10? How about the unique length\n> of 8?\n\nPerhaps I didn't state it clearly enough.\n\nConceptually, f-u-a works in two stages.  First find out what the shortest\nprefix that can be used to uniquely identify the given object _now_.\nThen make sure that the result it returns is at least \"len\" characters\nlong.  So if you code it naively, it would have looked like this:\n\n\tfor (try = 0; try < 40; try++)\n\t\tif (\"do first try characters name an object uniquely?\")\n\t\t\tbreak; /* happy */\n\tif (try < len)\n\t\treturn the first \"len\" characters;\n\telse\n        \treturn the first \"try\" characters;\n\nThe original (before the abbrevguard patch) uses an obvious optimization\nto start the trial from \"len\" because it didn't matter if a prefix shorter\nthan \"len\" is already unique---it will return \"len\" anyway.\n\nThe \"guard\" change was buggy.  At the conceptual level, it should have\nchanged the first phase of f-u-a, by dropping the \"_now_\" part. Given that\nwe will be adding more and more objects, only one extra character to make\nthings unique is not sufficient and having \"guard\" extra characters would\nkeep the result unique hopefully a little longer.  It should have done\nthis:\n\n\tfor (try = 0; try < 40; try++)\n\t\tif (\"do first try characters name an object uniquely?\")\n\t\t\tbreak; /* happy */\n       +/* try has only one extra character to make it unique */\n       +if (guard)\n       +\ttry += (guard - 1); /* could be try += guard */\n\tif (try < len)\n\t\treturn the first \"len\" characters;\n\telse\n        \treturn the first \"try\" characters;\n\nBack to your original question, if the object needs 7 characters (iow, its\n6-character prefix is shared with another object) to make it unique, and\nif the caller gives you 7 as length, without guard, you would get 7\nbecause you are saying 1 extra character is sufficiently unique by not\nsetting the guard.  If you set the guard to 3, you are saying that 1 extra\ncharacter is not sufficient to guard it against future collisions and you\nwant 3 extra instead, so you should be getting either 9 (if you define\nthat \"guard\" overrides the default \"1 more characters to make it minimally\nunique\") or 10 (if you define that \"guard\" specifies how many extra\ncharacters to use to make it safer).\n\nIf the object needs 8 characters to make it minimally unique and you give\n7 as \"len\", you should never get a 7-character string back.  f-u-a is not\n\"truncate to 'len'\" function but its primary job is to ensure uniqueness.\nWithout guard, you should get 8 (minimally unique), and with guard set to\n3, you should get either 10 or 11 (again, depending on the definition of\n\"guard\").\n\n> I think the meaning of the guard is somewhat vague. When this feature\n> was considered in LKML at first, Linus just wanted to change the default\n> length of commit abbreviation to 12 by making it user-configurable. [1]\n\nLinus's \"Let's make the default 12 for our project by adding a way to\ncustomize it\" merely states \"right now we know 7 is not enough and 12 is\nplenty for some time, and likely to be enough to give the same length\".\nIt does not mean he suddenly start favoring uniform length over\nuniqueness.  Uniqueness requirement is a given.\n\nAnd that discussion is all about what \"len\" to be given when calling\nf-u-a; it doesn't have anything to do with what the guard is about.\n\nSetting \"len\" to 12 would make sure your output you get today would have\nat least 12 characters, but it doesn't have any effect on how long an\nabbreviated unique object name will stay unique in the future.\n\nWhen you give the name of an object you have today to somebody else by\nshortening it with f-u-a, you can ensure that the object name has a better\nchance of staying unique a bit longer by setting the guard to a non-zero\nvalue even if you use shorter \"len\".\n\nThe \"len\" and \"guard\" are different concepts, have different purposes, and\nimplemented as two different variables, of course.\n\n> So I think it would be better to choose the output length using only\n> caller-given length and the guard length if it guarantees the uniqueness\n> today. It'll be simple, consistent and expected behavior IMHO.\n\nSorry, but I see nothing consistent nor expected about what you are\nsaying. Perhaps it is \"expected\" if you don't understand what the guard is\nabout. It is about the future viability of the output you produce today.\n\nIf you don't care about uniqueness tomorrow, you won't even need any\nguard.  You just give an interface to specify \"len\" and be done with it.\n"}]}