{"thread":{"id":"63943","subject":"[PATCH] abbrev: allow extending beyond 20 chars to disambiguate","startedAt":"2025-08-11T15:26:35Z","lastAt":"2025-08-14T15:09:20Z","messageCount":14,"participants":["Junio C Hamano","brian m. carlson","Derrick Stolee","René Scharfe","Jon Forrest"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"523951","messageId":"xmqqfrdx517b.fsf@gitster.g","threadId":"63943","inReplyTo":null,"subject":"[PATCH] abbrev: allow extending beyond 20 chars to disambiguate","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2025-08-11T15:26:32Z","receivedAt":"2025-08-11T15:26:35Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"When you have two or more objects with object names that share more\nthan half the length of the hash algorithm in use (e.g. 10 bytes for\nSHA-1 that produces 20-byte/160-bit hash), find_unique_abbrev()\nfails to show disambiguation.\n\nTo see how many leading letters of a given full object name is\nsufficiently unambiguous, the algorithm starts from a initial\nlength, guessed based on the estimated number of objects in the\nrepository, and see if another object that shares the prefix, and\nkeeps extending the abbreviation. The loop stops at GIT_MAX_RAWSZ,\nwhich is counted as the number of bytes, since 5b20ace6 (sha1_name:\nunroll len loop in find_unique_abbrev_r(), 2017-10-08); before that\nchange, it extended up to GIT_MAX_HEXSZ, which is the correct limit\nbecause the loop is adding one output letter per iteration.\n\nSigned-off-by: Junio C Hamano \n---\n\n * No tests added, since I do not think I want to find two valid\n objects with their object names sharing the same prefix that is\n more than 20 letters long. The current abbreviation code happens\n to ignore validity of the object and takes invalid objects into\n account when disambiguating, but I do not want to see a test rely\n on that.\n\n Git 2.15 (that predates 5b20ace6 which is in Git 2.16) does the\n right thing, even though it is a bit too old codebase to build\n these days (I had to omit curl and pcre, as I didn't need to get\n them working again only to see how its disambiguation code works)\n with the up-to-date libraries.\n\n object-name.c | 2 +-\n 1 file changed, 1 insertion(+), 1 deletion(-)\n\ndiff --git c/object-name.c w/object-name.c\nindex 11aa0e6afc..13e8a4e47d 100644\n--- c/object-name.c\n+++ w/object-name.c\n@@ -704,7 +704,7 @@ static int extend_abbrev_len(const struct object_id *oid, void *cb_data)\n \twhile (mad->hex[i] && mad->hex[i] == get_hex_char_from_oid(oid, i))\n \t\ti++;\n \n-\tif (i < GIT_MAX_RAWSZ && i >= mad->cur_len)\n+\tif (i < GIT_MAX_HEXSZ && i >= mad->cur_len)\n \t\tmad->cur_len = i + 1;\n \n \treturn 0;\n\n"},{"id":"523961","messageId":"xmqq5xet3d17.fsf@gitster.g","threadId":"63943","inReplyTo":"xmqqfrdx517b.fsf@gitster.g","subject":"Re: [PATCH] abbrev: allow extending beyond 20 chars to disambiguate","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2025-08-11T18:53:56Z","receivedAt":"2025-08-11T18:53:59Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Junio C Hamano writes:\n\n> keeps extending the abbreviation. The loop stops at GIT_MAX_RAWSZ,\n> which is counted as the number of bytes, since 5b20ace6 (sha1_name:\n> unroll len loop in find_unique_abbrev_r(), 2017-10-08); before that\n> change, it extended up to GIT_MAX_HEXSZ, which is the correct limit\n> because the loop is adding one output letter per iteration.\n\nThis is half a truth. It is correct that the loop used to terminate\nat GIT_SHA1_HEXSZ, and replacing it with GIT_MAX_HEXSZ is wrong, as\nMAX_HEXSZ can be much larger than the max hexsz for the hash function\nin use in the repository.\n\nI'll be sending a reworked version that takes the current hash\nfunction into account.\n\n"},{"id":"523962","messageId":"xmqqzfc51xvk.fsf@gitster.g","threadId":"63943","inReplyTo":"xmqqfrdx517b.fsf@gitster.g","subject":"[PATCH v2] abbrev: allow extending beyond 20 chars to disambiguate","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2025-08-11T19:06:39Z","receivedAt":"2025-08-11T19:06:41Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"When you have two or more objects with object names that share more\nthan half the length of the hash algorithm in use (e.g. 10 bytes for\nSHA-1 that produces 20-byte/160-bit hash), find_unique_abbrev()\nfails to show disambiguation.\n\nTo see how many leading letters of a given full object name is\nsufficiently unambiguous, the algorithm starts from a initial\nlength, guessed based on the estimated number of objects in the\nrepository, and see if another object that shares the prefix, and\nkeeps extending the abbreviation. The loop stops at GIT_MAX_RAWSZ,\nwhich is counted as the number of bytes, since 5b20ace6 (sha1_name:\nunroll len loop in find_unique_abbrev_r(), 2017-10-08); before that\nchange, it extended up to GIT_SHA1_HEXSZ, which was the correct\nlimit because the loop is adding one output letter per iteration\nand back then SHA256 was not in the picture.\n\nPass the max length of the hash being in use in the current\nrepository down the code path, and use it to compute the code to\nupdate the abbreviation length required to make it unique.\n\nSigned-off-by: Junio C Hamano \n---\n object-name.c | 6 ++++--\n 1 file changed, 4 insertions(+), 2 deletions(-)\n\ndiff --git a/object-name.c b/object-name.c\nindex 11aa0e6afc..8f9af57c0a 100644\n--- a/object-name.c\n+++ b/object-name.c\n@@ -680,6 +680,7 @@ static unsigned msb(unsigned long val)\n struct min_abbrev_data {\n \tunsigned int init_len;\n \tunsigned int cur_len;\n+\tunsigned int max_len;\n \tchar *hex;\n \tstruct repository *repo;\n \tconst struct object_id *oid;\n@@ -699,12 +700,12 @@ static inline char get_hex_char_from_oid(const struct object_id *oid,\n static int extend_abbrev_len(const struct object_id *oid, void *cb_data)\n {\n \tstruct min_abbrev_data *mad = cb_data;\n-\n \tunsigned int i = mad->init_len;\n+\n \twhile (mad->hex[i] && mad->hex[i] == get_hex_char_from_oid(oid, i))\n \t\ti++;\n \n-\tif (i < GIT_MAX_RAWSZ && i >= mad->cur_len)\n+\tif (mad->cur_len <= i && i < mad->max_len)\n \t\tmad->cur_len = i + 1;\n \n \treturn 0;\n@@ -864,6 +865,7 @@ int repo_find_unique_abbrev_r(struct repository *r, char *hex,\n \tmad.repo = r;\n \tmad.init_len = len;\n \tmad.cur_len = len;\n+\tmad.max_len = hexsz;\n \tmad.hex = hex;\n \tmad.oid = oid;\n \n\nRange-diff:\n1: 2e1d2b4ef6 ! 1: 5c67e57f14 abbrev: allow extending beyond 20 chars to disambiguate\n @@ Commit message\n keeps extending the abbreviation. The loop stops at GIT_MAX_RAWSZ,\n which is counted as the number of bytes, since 5b20ace6 (sha1_name:\n unroll len loop in find_unique_abbrev_r(), 2017-10-08); before that\n - change, it extended up to GIT_MAX_HEXSZ, which is the correct limit\n - because the loop is adding one output letter per iteration.\n + change, it extended up to GIT_SHA1_HEXSZ, which was the correct\n + limit because the loop is adding one output letter per iteration and\n + back then SHA256 was not in the picture.\n +\n + Pass the max length of the hash being in use in the current\n + repository down the code path, and use it to compute the code to\n + update the abbreviation length required to make it unique.\n \n Signed-off-by: Junio C Hamano \n \n ## object-name.c ##\n -@@ object-name.c: static int extend_abbrev_len(const struct object_id *oid, void *cb_data)\n +@@ object-name.c: static unsigned msb(unsigned long val)\n + struct min_abbrev_data {\n + \tunsigned int init_len;\n + \tunsigned int cur_len;\n ++\tunsigned int max_len;\n + \tchar *hex;\n + \tstruct repository *repo;\n + \tconst struct object_id *oid;\n +@@ object-name.c: static inline char get_hex_char_from_oid(const struct object_id *oid,\n + static int extend_abbrev_len(const struct object_id *oid, void *cb_data)\n + {\n + \tstruct min_abbrev_data *mad = cb_data;\n +-\n + \tunsigned int i = mad->init_len;\n ++\n \twhile (mad->hex[i] && mad->hex[i] == get_hex_char_from_oid(oid, i))\n \t\ti++;\n \n -\tif (i < GIT_MAX_RAWSZ && i >= mad->cur_len)\n -+\tif (i < GIT_MAX_HEXSZ && i >= mad->cur_len)\n ++\tif (mad->cur_len <= i && i < mad->max_len)\n \t\tmad->cur_len = i + 1;\n \n \treturn 0;\n +@@ object-name.c: int repo_find_unique_abbrev_r(struct repository *r, char *hex,\n + \tmad.repo = r;\n + \tmad.init_len = len;\n + \tmad.cur_len = len;\n ++\tmad.max_len = hexsz;\n + \tmad.hex = hex;\n + \tmad.oid = oid;\n + \n-- \n2.51.0-rc1-144-g869f44a1ca\n\n"},{"id":"523985","messageId":"aJpd2MYMWgEoxQWi@fruit.crustytoothpaste.net","threadId":"63943","inReplyTo":"xmqqfrdx517b.fsf@gitster.g","subject":"Re: [PATCH] abbrev: allow extending beyond 20 chars to disambiguate","fromName":"brian m. carlson","fromEmail":"sandals@crustytoothpaste.net","sentAt":"2025-08-11T21:17:12Z","receivedAt":"2025-08-11T21:22:33Z","isPatch":true,"sender":{"key":"sandals@crustytoothpaste.net","avatar":"https://avatars.githubusercontent.com/u/497054?v=4"},"body":"On 2025-08-11 at 15:26:32, Junio C Hamano wrote:\n> When you have two or more objects with object names that share more\n> than half the length of the hash algorithm in use (e.g. 10 bytes for\n> SHA-1 that produces 20-byte/160-bit hash), find_unique_abbrev()\n> fails to show disambiguation.\n\nIs this really the case? If the restriction is due to using\nGIT_MAX_RAWSZ instead of GIT_MAX_HEXSZ, then that's 32 vs. 64 in our\nmodern codebase.\n\n> To see how many leading letters of a given full object name is\n> sufficiently unambiguous, the algorithm starts from a initial\n> length, guessed based on the estimated number of objects in the\n> repository, and see if another object that shares the prefix, and\n> keeps extending the abbreviation. The loop stops at GIT_MAX_RAWSZ,\n> which is counted as the number of bytes, since 5b20ace6 (sha1_name:\n> unroll len loop in find_unique_abbrev_r(), 2017-10-08); before that\n> change, it extended up to GIT_MAX_HEXSZ, which is the correct limit\n> because the loop is adding one output letter per iteration.\n\nNicely explained.\n\n> * No tests added, since I do not think I want to find two valid\n> objects with their object names sharing the same prefix that is\n> more than 20 letters long. The current abbreviation code happens\n> to ignore validity of the object and takes invalid objects into\n> account when disambiguating, but I do not want to see a test rely\n> on that.\n\nYes, even if we could efficiently create such a collision with SHA-1\nusing the best known attacks on it, that would still be 2^63.5, which\nwas estimated to cost about USD 10,000 in 2025. I don't think doing\nthat just to produce a test would be a good use of the project's (or\nreally, anyone else's) funds. Using SHA-256, of course, would require\nat least 2^80 work.\n-- \nbrian m. carlson (they/them)\nToronto, Ontario, CA\n"},{"id":"523986","messageId":"xmqq4iud1rfm.fsf@gitster.g","threadId":"63943","inReplyTo":"aJpd2MYMWgEoxQWi@fruit.crustytoothpaste.net","subject":"Re: [PATCH] abbrev: allow extending beyond 20 chars to disambiguate","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2025-08-11T21:25:49Z","receivedAt":"2025-08-11T21:25:52Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"\"brian m. carlson\" writes:\n\n> On 2025-08-11 at 15:26:32, Junio C Hamano wrote:\n>> When you have two or more objects with object names that share more\n>> than half the length of the hash algorithm in use (e.g. 10 bytes for\n>> SHA-1 that produces 20-byte/160-bit hash), find_unique_abbrev()\n>> fails to show disambiguation.\n>\n> Is this really the case? \n\nWhat I wrote in the above is correct.\n\n> If the restriction is due to using\n> GIT_MAX_RAWSZ instead of GIT_MAX_HEXSZ, then that's 32 vs. 64 in our\n> modern codebase.\n\nThe above numbers are correct but irrelevant ;-).\n\nThe thing is, the offending commit changed from 40-bytes\n(GIT_SHA1_HEXSZ) to 32-bytes (GIT_MAX_RAWSZ). Plase see v2 patch.\n\nThanks.\n"},{"id":"523987","messageId":"xmqqzfc5zgxt.fsf@gitster.g","threadId":"63943","inReplyTo":"xmqq4iud1rfm.fsf@gitster.g","subject":"Re: [PATCH] abbrev: allow extending beyond 20 chars to disambiguate","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2025-08-11T21:28:30Z","receivedAt":"2025-08-11T21:28:33Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Junio C Hamano writes:\n\n> \"brian m. carlson\" writes:\n>\n>> On 2025-08-11 at 15:26:32, Junio C Hamano wrote:\n>>> When you have two or more objects with object names that share more\n>>> than half the length of the hash algorithm in use (e.g. 10 bytes for\n>>> SHA-1 that produces 20-byte/160-bit hash), find_unique_abbrev()\n>>> fails to show disambiguation.\n>>\n>> Is this really the case? \n>\n> What I wrote in the above is correct.\n\nSorry, I meant \"incorrect\". And using GIT_MAX_HEXSZ will break the\nabbreviation system rather badly. It has to be the length of the\nhash that is being used.\n\n>\n>> If the restriction is due to using\n>> GIT_MAX_RAWSZ instead of GIT_MAX_HEXSZ, then that's 32 vs. 64 in our\n>> modern codebase.\n>\n> The above numbers are correct but irrelevant ;-).\n>\n> The thing is, the offending commit changed from 40-bytes\n> (GIT_SHA1_HEXSZ) to 32-bytes (GIT_MAX_RAWSZ). Plase see v2 patch.\n>\n> Thanks.\n"},{"id":"524003","messageId":"aJptcgwQmwIbnrqE@fruit.crustytoothpaste.net","threadId":"63943","inReplyTo":"xmqqzfc51xvk.fsf@gitster.g","subject":"Re: [PATCH v2] abbrev: allow extending beyond 20 chars to disambiguate","fromName":"brian m. carlson","fromEmail":"sandals@crustytoothpaste.net","sentAt":"2025-08-11T22:23:46Z","receivedAt":"2025-08-11T22:23:48Z","isPatch":true,"sender":{"key":"sandals@crustytoothpaste.net","avatar":"https://avatars.githubusercontent.com/u/497054?v=4"},"body":"On 2025-08-11 at 19:06:39, Junio C Hamano wrote:\n> diff --git a/object-name.c b/object-name.c\n> index 11aa0e6afc..8f9af57c0a 100644\n> --- a/object-name.c\n> +++ b/object-name.c\n> @@ -680,6 +680,7 @@ static unsigned msb(unsigned long val)\n> struct min_abbrev_data {\n> \tunsigned int init_len;\n> \tunsigned int cur_len;\n> +\tunsigned int max_len;\n> \tchar *hex;\n> \tstruct repository *repo;\n> \tconst struct object_id *oid;\n> @@ -699,12 +700,12 @@ static inline char get_hex_char_from_oid(const struct object_id *oid,\n> static int extend_abbrev_len(const struct object_id *oid, void *cb_data)\n> {\n> \tstruct min_abbrev_data *mad = cb_data;\n> -\n> \tunsigned int i = mad->init_len;\n> +\n> \twhile (mad->hex[i] && mad->hex[i] == get_hex_char_from_oid(oid, i))\n> \t\ti++;\n> \n> -\tif (i < GIT_MAX_RAWSZ && i >= mad->cur_len)\n> +\tif (mad->cur_len <= i && i < mad->max_len)\n> \t\tmad->cur_len = i + 1;\n> \n> \treturn 0;\n> @@ -864,6 +865,7 @@ int repo_find_unique_abbrev_r(struct repository *r, char *hex,\n> \tmad.repo = r;\n> \tmad.init_len = len;\n> \tmad.cur_len = len;\n> +\tmad.max_len = hexsz;\n> \tmad.hex = hex;\n> \tmad.oid = oid;\n\nThis definitely looks more sensible, since we're using the algorithm\nspecified in the passed in `oid` variable in\n`repo_find_unique_abbrev_r` to determine the length.\n\nI will admit that despite having touched this code recently in my\nSHA-1/SHA-256 interoperability work, I'm definitely not an expert in\nthis area, so while I don't see anything that stands out to me as wrong,\nyou probably will want someone else to verify here.\n-- \nbrian m. carlson (they/them)\nToronto, Ontario, CA\n"},{"id":"524036","messageId":"55cbc80e-2585-49dd-80e9-33f200a9fac1@gmail.com","threadId":"63943","inReplyTo":"xmqqzfc51xvk.fsf@gitster.g","subject":"Re: [PATCH v2] abbrev: allow extending beyond 20 chars to disambiguate","fromName":"Derrick Stolee","fromEmail":"stolee@gmail.com","sentAt":"2025-08-12T13:28:43Z","receivedAt":"2025-08-12T13:29:07Z","isPatch":true,"sender":{"key":"stolee@gmail.com","avatar":"https://avatars.githubusercontent.com/u/570044?v=4"},"body":"On 8/11/2025 3:06 PM, Junio C Hamano wrote:\n> When you have two or more objects with object names that share more\n> than half the length of the hash algorithm in use (e.g. 10 bytes for\n> SHA-1 that produces 20-byte/160-bit hash), find_unique_abbrev()\n> fails to show disambiguation.\n> \n> To see how many leading letters of a given full object name is\n> sufficiently unambiguous, the algorithm starts from a initial\n> length, guessed based on the estimated number of objects in the\n> repository, and see if another object that shares the prefix, and\n> keeps extending the abbreviation. The loop stops at GIT_MAX_RAWSZ,\n> which is counted as the number of bytes, since 5b20ace6 (sha1_name:\n> unroll len loop in find_unique_abbrev_r(), 2017-10-08);\n\nWow. My first Git contribution.\n\nSome style issues that you're opportunistically cleaning up are\ndue to my newness, for sure.\n\n> before that\n> change, it extended up to GIT_SHA1_HEXSZ, which was the correct\n> limit because the loop is adding one output letter per iteration\n> and back then SHA256 was not in the picture.\n> \n> Pass the max length of the hash being in use in the current\n> repository down the code path, and use it to compute the code to\n> update the abbreviation length required to make it unique.\n> \n> Signed-off-by: Junio C Hamano \n> ---\n> object-name.c | 6 ++++--\n> 1 file changed, 4 insertions(+), 2 deletions(-)\n> \n> diff --git a/object-name.c b/object-name.c\n> index 11aa0e6afc..8f9af57c0a 100644\n> --- a/object-name.c\n> +++ b/object-name.c\n> @@ -680,6 +680,7 @@ static unsigned msb(unsigned long val)\n> struct min_abbrev_data {\n> \tunsigned int init_len;\n> \tunsigned int cur_len;\n> +\tunsigned int max_len;\n> \tchar *hex;\n> \tstruct repository *repo;\n> \tconst struct object_id *oid;\n> @@ -699,12 +700,12 @@ static inline char get_hex_char_from_oid(const struct object_id *oid,\n> static int extend_abbrev_len(const struct object_id *oid, void *cb_data)\n> {\n> \tstruct min_abbrev_data *mad = cb_data;\n> -\n> \tunsigned int i = mad->init_len;\n> +\n> \twhile (mad->hex[i] && mad->hex[i] == get_hex_char_from_oid(oid, i))\n> \t\ti++;\n> \n> -\tif (i < GIT_MAX_RAWSZ && i >= mad->cur_len)\n> +\tif (mad->cur_len <= i && i < mad->max_len)\n> \t\tmad->cur_len = i + 1;\n\nThis logic is all about not extending the abbreviation\nlength to beyond the length of the hex array, so your\nlimits make sense. Moving the comparisons here helps\nwith readability. \n> \treturn 0;\n> @@ -864,6 +865,7 @@ int repo_find_unique_abbrev_r(struct repository *r, char *hex,\n> \tmad.repo = r;\n> \tmad.init_len = len;\n> \tmad.cur_len = len;\n> +\tmad.max_len = hexsz;\n\nThis new parameter required for allowing SHA256 is valuable.\n\nI agree that we shouldn't need a test case to guarantee this\nin the future. Good to clean up unnecessary uses of the\nmacro limits.\n\nThanks,\n-Stolee\n\n"},{"id":"524052","messageId":"9ae4a718-b00d-4435-8739-cf87b2c9df7d@web.de","threadId":"63943","inReplyTo":"xmqqzfc51xvk.fsf@gitster.g","subject":"Re: [PATCH v2] abbrev: allow extending beyond 20 chars to disambiguate","fromName":"René Scharfe","fromEmail":"l.s.r@web.de","sentAt":"2025-08-12T14:58:14Z","receivedAt":"2025-08-12T14:58:16Z","isPatch":true,"sender":{"key":"l.s.r@web.de","avatar":"https://avatars.githubusercontent.com/u/26122331?v=4"},"body":"On 8/11/25 9:06 PM, Junio C Hamano wrote:\n> When you have two or more objects with object names that share more\n> than half the length of the hash algorithm in use (e.g. 10 bytes for\n> SHA-1 that produces 20-byte/160-bit hash), find_unique_abbrev()\n> fails to show disambiguation.\n> \n> To see how many leading letters of a given full object name is\n> sufficiently unambiguous, the algorithm starts from a initial\n> length, guessed based on the estimated number of objects in the\n> repository, and see if another object that shares the prefix, and\n> keeps extending the abbreviation. The loop stops at GIT_MAX_RAWSZ,\n> which is counted as the number of bytes, since 5b20ace6 (sha1_name:\n> unroll len loop in find_unique_abbrev_r(), 2017-10-08); before that\n> change, it extended up to GIT_SHA1_HEXSZ, which was the correct\n> limit because the loop is adding one output letter per iteration\n> and back then SHA256 was not in the picture.\n> \n> Pass the max length of the hash being in use in the current\n> repository down the code path, and use it to compute the code to\n> update the abbreviation length required to make it unique.\n> \n> Signed-off-by: Junio C Hamano \n> ---\n> object-name.c | 6 ++++--\n> 1 file changed, 4 insertions(+), 2 deletions(-)\n> \n> diff --git a/object-name.c b/object-name.c\n> index 11aa0e6afc..8f9af57c0a 100644\n> --- a/object-name.c\n> +++ b/object-name.c\n> @@ -680,6 +680,7 @@ static unsigned msb(unsigned long val)\n> struct min_abbrev_data {\n> \tunsigned int init_len;\n> \tunsigned int cur_len;\n> +\tunsigned int max_len;\n> \tchar *hex;\n> \tstruct repository *repo;\n> \tconst struct object_id *oid;\n> @@ -699,12 +700,12 @@ static inline char get_hex_char_from_oid(const struct object_id *oid,\n> static int extend_abbrev_len(const struct object_id *oid, void *cb_data)\n> {\n> \tstruct min_abbrev_data *mad = cb_data;\n> -\n> \tunsigned int i = mad->init_len;\n> +\n> \twhile (mad->hex[i] && mad->hex[i] == get_hex_char_from_oid(oid, i))\n> \t\ti++;\n> \n> -\tif (i < GIT_MAX_RAWSZ && i >= mad->cur_len)\n> +\tif (mad->cur_len <= i && i < mad->max_len)\n> \t\tmad->cur_len = i + 1;\n\nThis combines two checks: Whether we can increment and whether the new\nlength is greater than the old one. Only if both are true we take the\nnew length. Shouldn't they be separate, though? Why reject a new\nlength that happens to be the maximum? And max_len is not explicitly\nneeded for the first check:\n\n\t/* One more to disambiguate, if possible. */\n\tif (mad->hex[i])\n\t\ti++;\n\n\t/* New record? */\n\tif (i > mad->cur_len)\n\t\tmad->cur_len = i;\n\nRené\n\n"},{"id":"524055","messageId":"xmqqwm78vaah.fsf@gitster.g","threadId":"63943","inReplyTo":"9ae4a718-b00d-4435-8739-cf87b2c9df7d@web.de","subject":"Re: [PATCH v2] abbrev: allow extending beyond 20 chars to disambiguate","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2025-08-12T15:17:58Z","receivedAt":"2025-08-12T15:18:00Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"René Scharfe writes:\n\n>> \twhile (mad->hex[i] && mad->hex[i] == get_hex_char_from_oid(oid, i))\n>> \t\ti++;\n>> \n>> -\tif (i < GIT_MAX_RAWSZ && i >= mad->cur_len)\n>> +\tif (mad->cur_len <= i && i < mad->max_len)\n>> \t\tmad->cur_len = i + 1;\n>\n> This combines two checks: Whether we can increment and whether the new\n> length is greater than the old one. Only if both are true we take the\n> new length. Shouldn't they be separate, though? Why reject a new\n> length that happens to be the maximum? And max_len is not explicitly\n> needed for the first check:\n>\n> \t/* One more to disambiguate, if possible. */\n> \tif (mad->hex[i])\n> \t\ti++;\n>\n> \t/* New record? */\n> \tif (i > mad->cur_len)\n> \t\tmad->cur_len = i;\n>\n> René\n\nGreat.\n\nYour observation resolves my puzzlement about the first while() loop\nthat has been bugging me ever since I started looking at this code.\nThe mad->hex[] array is NUL terminated, and the loop can terminate\ncorrectly without being told about hexsz at all, and we ought to be\nable to use the same information to make sure we stop incrementing\nthe .cur_len member without running beyond the end of the string.\n\nIn other words, wouldn't this be what we want, without any of the\nmax_len crap?\n\ndiff --git c/object-name.c w/object-name.c\nindex 11aa0e6afc..4cd1d38778 100644\n--- c/object-name.c\n+++ w/object-name.c\n@@ -704,7 +704,7 @@ static int extend_abbrev_len(const struct object_id *oid, void *cb_data)\n \twhile (mad->hex[i] && mad->hex[i] == get_hex_char_from_oid(oid, i))\n \t\ti++;\n \n-\tif (i < GIT_MAX_RAWSZ && i >= mad->cur_len)\n+\tif (mad->hex[i] && i >= mad->cur_len)\n \t\tmad->cur_len = i + 1;\n \n \treturn 0;\n"},{"id":"524057","messageId":"e463481d-b6a6-4928-86e1-63677f5a88b2@gmail.com","threadId":"63943","inReplyTo":"aJpd2MYMWgEoxQWi@fruit.crustytoothpaste.net","subject":"Re: [PATCH] abbrev: allow extending beyond 20 chars to disambiguate","fromName":"Jon Forrest","fromEmail":"nobozo@gmail.com","sentAt":"2025-08-12T15:26:32Z","receivedAt":"2025-08-12T15:26:34Z","isPatch":true,"sender":{"key":"nobozo@gmail.com","avatar":"https://gravatar.com/avatar/37c6a7b57f29b3f35c6a9016537907c52f15bbf98575617dc046bcec6bf06372?d=mp&s=160"},"body":"\n\nOn 8/11/25 2:17 PM, brian m. carlson wrote:\n\n>> To see how many leading letters of a given full object name is\n>> sufficiently unambiguous, the algorithm starts from a initial\n>> length, guessed based on the estimated number of objects in the\n>> repository, and see if another object that shares the prefix, and\n>> keeps extending the abbreviation. The loop stops at GIT_MAX_RAWSZ,\n>> which is counted as the number of bytes, since 5b20ace6 (sha1_name:\n>> unroll len loop in find_unique_abbrev_r(), 2017-10-08); before that\n>> change, it extended up to GIT_MAX_HEXSZ, which is the correct limit\n>> because the loop is adding one output letter per iteration.\n\nI'm new to all this but the way I did it is much simpler.\nWhat I did was to check all the files in the appropriate\nobject store directory (e.g. .git/objects/XX, where XX are\nthe first 2 letters of the object given on the command line.\nIf any of the filenames in that directory start with the\nstring given on the command line, minus the first 2 letters,\nthen that's a match. If more than one filename matches then\nthat's ambiguous.\n\nWhat's wrong with this approach?\n\nJon\n\n"},{"id":"524060","messageId":"6d0631ed-9a28-4107-8c07-298a9424769b@web.de","threadId":"63943","inReplyTo":"xmqqwm78vaah.fsf@gitster.g","subject":"Re: [PATCH v2] abbrev: allow extending beyond 20 chars to disambiguate","fromName":"René Scharfe","fromEmail":"l.s.r@web.de","sentAt":"2025-08-12T15:59:56Z","receivedAt":"2025-08-12T15:59:59Z","isPatch":true,"sender":{"key":"l.s.r@web.de","avatar":"https://avatars.githubusercontent.com/u/26122331?v=4"},"body":"\n\nOn 8/12/25 5:17 PM, Junio C Hamano wrote:\n> René Scharfe writes:\n> \n>>> \twhile (mad->hex[i] && mad->hex[i] == get_hex_char_from_oid(oid, i))\n>>> \t\ti++;\n>>> \n>>> -\tif (i < GIT_MAX_RAWSZ && i >= mad->cur_len)\n>>> +\tif (mad->cur_len <= i && i < mad->max_len)\n>>> \t\tmad->cur_len = i + 1;\n>>\n>> This combines two checks: Whether we can increment and whether the new\n>> length is greater than the old one. Only if both are true we take the\n>> new length. Shouldn't they be separate, though? Why reject a new\n>> length that happens to be the maximum? And max_len is not explicitly\n>> needed for the first check:\n>>\n>> \t/* One more to disambiguate, if possible. */\n>> \tif (mad->hex[i])\n>> \t\ti++;\n>>\n>> \t/* New record? */\n>> \tif (i > mad->cur_len)\n>> \t\tmad->cur_len = i;\n>>\n>> René\n> \n> Great.\n> \n> Your observation resolves my puzzlement about the first while() loop\n> that has been bugging me ever since I started looking at this code.\n> The mad->hex[] array is NUL terminated, and the loop can terminate\n> correctly without being told about hexsz at all, and we ought to be\n> able to use the same information to make sure we stop incrementing\n> the .cur_len member without running beyond the end of the string.\n> \n> In other words, wouldn't this be what we want, without any of the\n> max_len crap?\n> \n> diff --git c/object-name.c w/object-name.c\n> index 11aa0e6afc..4cd1d38778 100644\n> --- c/object-name.c\n> +++ w/object-name.c\n> @@ -704,7 +704,7 @@ static int extend_abbrev_len(const struct object_id *oid, void *cb_data)\n> \twhile (mad->hex[i] && mad->hex[i] == get_hex_char_from_oid(oid, i))\n> \t\ti++;\n> \n> -\tif (i < GIT_MAX_RAWSZ && i >= mad->cur_len)\n> +\tif (mad->hex[i] && i >= mad->cur_len)\n> \t\tmad->cur_len = i + 1;\n> \n> \treturn 0;\n\nThis combines the two checks and I don't see why. Why not update\ncur_len when oid and mad->oid are the same (mad->hex[i] == '\\0')? Ah,\nto _ignore_ duplicates on purpose, when we have the same object loose\nand packed or in multiple packs, right? We wouldn't want to let that\naffect abbreviations, makes sense.\n\nRené\n\n"},{"id":"524063","messageId":"5c7942a3-bccf-44e5-94c2-39fc50f9bab3@web.de","threadId":"63943","inReplyTo":"e463481d-b6a6-4928-86e1-63677f5a88b2@gmail.com","subject":"Re: [PATCH] abbrev: allow extending beyond 20 chars to disambiguate","fromName":"René Scharfe","fromEmail":"l.s.r@web.de","sentAt":"2025-08-12T16:21:41Z","receivedAt":"2025-08-12T16:21:49Z","isPatch":true,"sender":{"key":"l.s.r@web.de","avatar":"https://avatars.githubusercontent.com/u/26122331?v=4"},"body":"On 8/12/25 5:26 PM, Jon Forrest wrote:\n> \n> \n> On 8/11/25 2:17 PM, brian m. carlson wrote:\n> \n>>> To see how many leading letters of a given full object name is\n>>> sufficiently unambiguous, the algorithm starts from a initial\n>>> length, guessed based on the estimated number of objects in the\n>>> repository, and see if another object that shares the prefix, and\n>>> keeps extending the abbreviation.  The loop stops at GIT_MAX_RAWSZ,\n>>> which is counted as the number of bytes, since 5b20ace6 (sha1_name:\n>>> unroll len loop in find_unique_abbrev_r(), 2017-10-08); before that\n>>> change, it extended up to GIT_MAX_HEXSZ, which is the correct limit\n>>> because the loop is adding one output letter per iteration.\n> \n> I'm new to all this but the way I did it is much simpler.\n> What I did was to check all the files in the appropriate\n> object store directory (e.g. .git/objects/XX, where XX are\n> the first 2 letters of the object given on the command line.\n> If any of the filenames in that directory start with the\n> string given on the command line, minus the first 2 letters,\n> then that's a match. If more than one filename matches then\n> that's ambiguous.\n> \n> What's wrong with this approach?\nYou also need to check all pack files and alternate object databases\nthat you might have.\n\nRené\n\n"},{"id":"524178","messageId":"xmqqh5ya6iua.fsf_-_@gitster.g","threadId":"63943","inReplyTo":"xmqqzfc51xvk.fsf@gitster.g","subject":"[PATCH v3] abbrev: allow extending beyond 32 chars to disambiguate","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2025-08-14T15:09:17Z","receivedAt":"2025-08-14T15:09:20Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"When you have two or more objects with object names that share more\nthan 32 letters in an SHA-1 repository, find_unique_abbrev() fails\nto show disambiguation.\n\nTo see how many leading letters of a given full object name is\nsufficiently unambiguous, the algorithm starts from a initial\nlength, guessed based on the estimated number of objects in the\nrepository, and see if another object that shares the prefix, and\nkeeps extending the abbreviation. The loop stops at GIT_MAX_RAWSZ,\nwhich is counted as the number of bytes, since 5b20ace6 (sha1_name:\nunroll len loop in find_unique_abbrev_r(), 2017-10-08); before that\nchange, it extended up to GIT_SHA1_HEXSZ, which meant to stop at the\nend of hexadecimal SHA-1 object name.\n\nBecause the hexadecimal object name passed to the function is\nNUL-terminated, and this fact is used to correctly terminate the\nloop that scans for the first difference earlier in the function,\nuse it to make sure we do not increment the .cur_len member beyond\nthe end of the string.\n\nNoticed-by: Jon Forrest \nHelped-by: René Scharfe \nSigned-off-by: Junio C Hamano \n---\n\n * To tie the loose ends, here is what is in 'seen'. We may want to\n merge it down once 2.51 final gets tagged.\n\n object-name.c | 2 +-\n 1 file changed, 1 insertion(+), 1 deletion(-)\n\ndiff --git a/object-name.c b/object-name.c\nindex 11aa0e6afc..4cd1d38778 100644\n--- a/object-name.c\n+++ b/object-name.c\n@@ -704,7 +704,7 @@ static int extend_abbrev_len(const struct object_id *oid, void *cb_data)\n \twhile (mad->hex[i] && mad->hex[i] == get_hex_char_from_oid(oid, i))\n \t\ti++;\n \n-\tif (i < GIT_MAX_RAWSZ && i >= mad->cur_len)\n+\tif (mad->hex[i] && i >= mad->cur_len)\n \t\tmad->cur_len = i + 1;\n \n \treturn 0;\n-- \n2.51.0-rc2-158-gf97fc618fa\n\n"}]}