{"thread":{"id":"54169","subject":"[PATCH] core.abbrev disables abbreviations","startedAt":"2020-09-01T07:43:57Z","lastAt":"2020-12-23T20:22:44Z","messageCount":13,"participants":["Eric Wong","Derrick Stolee","Junio C Hamano","brian m. carlson"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"404838","messageId":"20200901074355.GA4498@dcvr","threadId":"54169","inReplyTo":null,"subject":"[PATCH] core.abbrev disables abbreviations","fromName":"Eric Wong","fromEmail":"e@80x24.org","sentAt":"2020-09-01T07:43:55Z","receivedAt":"2020-09-01T07:43:57Z","isPatch":true,"sender":{"key":"e@80x24.org","avatar":null},"body":"These allows users to write hash-agnostic scripts and configs to\ndisable abbreviations. Using \"-c core.abbrev=40\" will be\ninsufficient with SHA-256, and \"-c core.abbrev=64\" won't work\nwith SHA-1 repos today.\n\nSigned-off-by: Eric Wong \n---\n I kinda wanted to allow a value of \"max\", but I figured the existing\n boolean falsiness words might make more sense with `--no-abbrev' in\n for some commands... Naming is hard :x\n\n config.c | 4 ++++\n 1 file changed, 4 insertions(+)\n\ndiff --git a/config.c b/config.c\nindex 2bdff4457b..f2e09c72ca 100644\n--- a/config.c\n+++ b/config.c\n@@ -1217,6 +1217,10 @@ static int git_default_core_config(const char *var, const char *value, void *cb)\n \t\t\treturn config_error_nonbool(var);\n \t\tif (!strcasecmp(value, \"auto\"))\n \t\t\tdefault_abbrev = -1;\n+\t\telse if (!strcasecmp(value, \"false\") ||\n+\t\t\t !strcasecmp(value, \"no\") ||\n+\t\t\t !strcasecmp(value, \"off\"))\n+\t\t\tdefault_abbrev = the_hash_algo->hexsz;\n \t\telse {\n \t\t\tint abbrev = git_config_int(var, value);\n \t\t\tif (abbrev < minimum_abbrev || abbrev > the_hash_algo->hexsz)\n"},{"id":"404856","messageId":"9c00f29b-45e4-ccdf-6d81-5eabd58c875b@gmail.com","threadId":"54169","inReplyTo":"20200901074355.GA4498@dcvr","subject":"Re: [PATCH] core.abbrev disables abbreviations","fromName":"Derrick Stolee","fromEmail":"stolee@gmail.com","sentAt":"2020-09-01T12:14:32Z","receivedAt":"2020-09-01T12:23:11Z","isPatch":true,"sender":{"key":"stolee@gmail.com","avatar":"https://avatars.githubusercontent.com/u/570044?v=4"},"body":"On 9/1/2020 3:43 AM, Eric Wong wrote:\n> These allows users to write hash-agnostic scripts and configs to\n> disable abbreviations. Using \"-c core.abbrev=40\" will be\n> insufficient with SHA-256, and \"-c core.abbrev=64\" won't work\n> with SHA-1 repos today.\n> \n> Signed-off-by: Eric Wong \n> ---\n> I kinda wanted to allow a value of \"max\", but I figured the existing\n> boolean falsiness words might make more sense with `--no-abbrev' in\n> for some commands... Naming is hard :x\n> \n> config.c | 4 ++++\n> 1 file changed, 4 insertions(+)\n> \n> diff --git a/config.c b/config.c\n> index 2bdff4457b..f2e09c72ca 100644\n> --- a/config.c\n> +++ b/config.c\n> @@ -1217,6 +1217,10 @@ static int git_default_core_config(const char *var, const char *value, void *cb)\n> \t\t\treturn config_error_nonbool(var);\n> \t\tif (!strcasecmp(value, \"auto\"))\n> \t\t\tdefault_abbrev = -1;\n> +\t\telse if (!strcasecmp(value, \"false\") ||\n> +\t\t\t !strcasecmp(value, \"no\") ||\n> +\t\t\t !strcasecmp(value, \"off\"))\n> +\t\t\tdefault_abbrev = the_hash_algo->hexsz;\n\nI'm not sure we need three synonyms for \"no-abbrev\" here.\n\n\"false\" would be natural, except I think in a few places\nthe config value \"0\" is also interpreted as \"false\", but\nas seen below a value of \"0\" snaps up to the minimum\nallowed abbreviation.\n\n> \t\telse {\n> \t\t\tint abbrev = git_config_int(var, value);\n> \t\t\tif (abbrev < minimum_abbrev || abbrev > the_hash_algo->hexsz)\n\nPerhaps \"core.abbrev = never\" would be a good option?\n\nAfter we decide on the word, this patch needs:\n\n* Updates to Documentation/config/core.txt\n* A test that works with both hash versions.\n\nThanks,\n-Stolee\n"},{"id":"404867","messageId":"20200901144323.GA14554@dcvr","threadId":"54169","inReplyTo":"9c00f29b-45e4-ccdf-6d81-5eabd58c875b@gmail.com","subject":"Re: [PATCH] core.abbrev disables abbreviations","fromName":"Eric Wong","fromEmail":"e@80x24.org","sentAt":"2020-09-01T14:43:23Z","receivedAt":"2020-09-01T14:43:35Z","isPatch":true,"sender":{"key":"e@80x24.org","avatar":null},"body":"Derrick Stolee wrote:\n> On 9/1/2020 3:43 AM, Eric Wong wrote:\n> > index 2bdff4457b..f2e09c72ca 100644\n> > --- a/config.c\n> > +++ b/config.c\n> > @@ -1217,6 +1217,10 @@ static int git_default_core_config(const char *var, const char *value, void *cb)\n> > \t\t\treturn config_error_nonbool(var);\n> > \t\tif (!strcasecmp(value, \"auto\"))\n> > \t\t\tdefault_abbrev = -1;\n> > +\t\telse if (!strcasecmp(value, \"false\") ||\n> > +\t\t\t !strcasecmp(value, \"no\") ||\n> > +\t\t\t !strcasecmp(value, \"off\"))\n> > +\t\t\tdefault_abbrev = the_hash_algo->hexsz;\n> \n> I'm not sure we need three synonyms for \"no-abbrev\" here.\n\nI just used the accepted synonyms since I figured users\nwould be used to them, already.\n\n> \"false\" would be natural, except I think in a few places\n> the config value \"0\" is also interpreted as \"false\", but\n> as seen below a value of \"0\" snaps up to the minimum\n> allowed abbreviation.\n> \n> > \t\telse {\n> > \t\t\tint abbrev = git_config_int(var, value);\n> > \t\t\tif (abbrev < minimum_abbrev || abbrev > the_hash_algo->hexsz)\n\nIt actually errors out on the next line, here.\n\nPerhaps adopting parse_opt_abbrev_cb behavior of clamping to\nthe minimum and maximum supported values is more consistent?\n\n> Perhaps \"core.abbrev = never\" would be a good option?\n\n*shrug*\n\n> After we decide on the word, this patch needs:\n> \n> * Updates to Documentation/config/core.txt\n\nWill do.\n\n> * A test that works with both hash versions.\n\nWill do, though not too sure where the tests for this should be.\n\nThanks for the comments, will wait a few days for comments\nbefore sending out v2.\n"},{"id":"404869","messageId":"cbbc6d40-7126-cf28-aeb5-46b72b7e6d08@gmail.com","threadId":"54169","inReplyTo":"20200901144323.GA14554@dcvr","subject":"Re: [PATCH] core.abbrev disables abbreviations","fromName":"Derrick Stolee","fromEmail":"stolee@gmail.com","sentAt":"2020-09-01T14:59:58Z","receivedAt":"2020-09-01T15:00:07Z","isPatch":true,"sender":{"key":"stolee@gmail.com","avatar":"https://avatars.githubusercontent.com/u/570044?v=4"},"body":"On 9/1/2020 10:43 AM, Eric Wong wrote:\n> Derrick Stolee wrote:\n>> After we decide on the word, this patch needs:\n>>\n>> * Updates to Documentation/config/core.txt\n> \n> Will do.\n\nThanks.\n\n>> * A test that works with both hash versions.\n> \n> Will do, though not too sure where the tests for this should be.\n\nt3404-rebase-interactive.sh seems to already test the\ncore.abbrev config value. Could be a good place to provide\nan extra check.\n\nt4205-log-pretty-formats.sh could also use some references\nto core.abbrev.\n\nThanks,\n-Stolee\n"},{"id":"404871","messageId":"xmqqblipebto.fsf@gitster.c.googlers.com","threadId":"54169","inReplyTo":"9c00f29b-45e4-ccdf-6d81-5eabd58c875b@gmail.com","subject":"Re: [PATCH] core.abbrev disables abbreviations","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2020-09-01T15:49:55Z","receivedAt":"2020-09-01T15:51:23Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Derrick Stolee writes:\n\n>> +\t\telse if (!strcasecmp(value, \"false\") ||\n>> +\t\t\t !strcasecmp(value, \"no\") ||\n>> +\t\t\t !strcasecmp(value, \"off\"))\n>> +\t\t\tdefault_abbrev = the_hash_algo->hexsz;\n>\n> I'm not sure we need three synonyms for \"no-abbrev\" here.\n\nI do not particularly mind, but if we imitate the variety of various\nboolean false, I'd prefer to see the code to parse them shared to\navoid them drifting apart over time.\n\n> \"false\" would be natural, except I think in a few places\n> the config value \"0\" is also interpreted as \"false\", but\n> as seen below a value of \"0\" snaps up to the minimum\n> allowed abbreviation.\n\nI was in the vicinity of this code recently for reviewing another\ntopic, but IIRC, 0 came from the UI level does get rounded up to the\nminimum accepted and never reach \"default_abbrev\", but if you manage\nto place 0 or -1 in default_abbrev here (e.g. with additional code,\nlike the above part with the right hand side of the assignment\nupdated), I think the value will propagate throughout the codepath\nand causes the downstream code to do the right thing. 0 will give\nyou no-abbreviation (i.e. full length depending on the length of the\nhash) and -1 will give you the \"scale as appropriate for the size of\nthe object store\".\n\nI have mild preference for using 0 over hardcoded single \"full\nlength\" here. Even though we currently do not plan to allow\nmultiple hashes in use simultaneously in a single invocation of Git,\nif that ever happens, we will regret hardcoding the_hash_algo->hexsz\non the right hand side of the assignment here, like this patch does.\n\nTelling the downstream code in the control flow that we want no\ntruncation by using 0 would keep both 40-hexdigit and 64-hexdigit\nhashes to their original length (as opposed to telling it to\ntruncate at 40 or 64 by using the_hash_algo->hexsz).\n\nThanks.\n\n\n\n"},{"id":"404875","messageId":"xmqqtuwhcnrl.fsf@gitster.c.googlers.com","threadId":"54169","inReplyTo":"xmqqblipebto.fsf@gitster.c.googlers.com","subject":"Re: [PATCH] core.abbrev disables abbreviations","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2020-09-01T19:14:54Z","receivedAt":"2020-09-01T19:15:00Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Junio C Hamano writes:\n\n> Derrick Stolee writes:\n>\n>>> +\t\telse if (!strcasecmp(value, \"false\") ||\n>>> +\t\t\t !strcasecmp(value, \"no\") ||\n>>> +\t\t\t !strcasecmp(value, \"off\"))\n>>> +\t\t\tdefault_abbrev = the_hash_algo->hexsz;\n>>\n>> I'm not sure we need three synonyms for \"no-abbrev\" here.\n>\n> I do not particularly mind, but if we imitate the variety of various\n> boolean false, I'd prefer to see the code to parse them shared to\n> avoid them drifting apart over time.\n\nJust a clarification. \n\n - I do not particularly mind having multiple synonyms. \n\n - I do mind these one-off strcasecmp that will cause them to drift\n away from what we do for the boolean 'false'.\n\nThanks.\n"},{"id":"404893","messageId":"20200901233752.GC241078@camp.crustytoothpaste.net","threadId":"54169","inReplyTo":"xmqqblipebto.fsf@gitster.c.googlers.com","subject":"Re: [PATCH] core.abbrev disables abbreviations","fromName":"brian m. carlson","fromEmail":"sandals@crustytoothpaste.net","sentAt":"2020-09-01T23:37:52Z","receivedAt":"2020-09-01T23:38:35Z","isPatch":true,"sender":{"key":"sandals@crustytoothpaste.net","avatar":"https://avatars.githubusercontent.com/u/497054?v=4"},"body":"On 2020-09-01 at 15:49:55, Junio C Hamano wrote:\n> I was in the vicinity of this code recently for reviewing another\n> topic, but IIRC, 0 came from the UI level does get rounded up to the\n> minimum accepted and never reach \"default_abbrev\", but if you manage\n> to place 0 or -1 in default_abbrev here (e.g. with additional code,\n> like the above part with the right hand side of the assignment\n> updated), I think the value will propagate throughout the codepath\n> and causes the downstream code to do the right thing. 0 will give\n> you no-abbreviation (i.e. full length depending on the length of the\n> hash) and -1 will give you the \"scale as appropriate for the size of\n> the object store\".\n> \n> I have mild preference for using 0 over hardcoded single \"full\n> length\" here. Even though we currently do not plan to allow\n> multiple hashes in use simultaneously in a single invocation of Git,\n> if that ever happens, we will regret hardcoding the_hash_algo->hexsz\n> on the right hand side of the assignment here, like this patch does.\n\nI think we have some commands that accept --abbrev=0 as a value meaning\n\"no abbreviation\", because I've touched that code as part of the SHA-256\nwork. So as far as the option value is concerned, I think it may make\nsense to use 0 for that meaning and just document it.\n-- \nbrian m. carlson: Houston, Texas, US\n"},{"id":"412852","messageId":"xmqqbleliq58.fsf@gitster.c.googlers.com","threadId":"54169","inReplyTo":"20200901144323.GA14554@dcvr","subject":"Re: [PATCH] core.abbrev disables abbreviations","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2020-12-22T19:42:27Z","receivedAt":"2020-12-22T19:43:15Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Eric Wong writes:\n\n> Thanks for the comments, will wait a few days for comments\n> before sending out v2.\n\nThis has seen some review suggestions and as far as I remember, can\nbe summarised as:\n\n - there was a rough consensus that this was a desirable feature.\n\n - a one-off hardcoded list of \"false\" would rather want to be\n consistent with config.c::git_parse_maybe_bool_text().\n\n - documentation is missing for the configuration variable.\n\nIt has been almost three months; has a v2 been posted that I missed?\n\nThanks.\n"},{"id":"412871","messageId":"xmqqk0t9fmq6.fsf@gitster.c.googlers.com","threadId":"54169","inReplyTo":"20201222231702.GA17661@dcvr","subject":"Re: [PATCH] core.abbrev disables abbreviations","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2020-12-22T23:24:33Z","receivedAt":"2020-12-22T23:25:33Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Eric Wong writes:\n\n> Can somebody else take this over? Sorry my brain isn't working\n> well these months and I'm behind on other things :<\n\nSurely and thanks for letting us know. Take care and happy holidays\nand new year to you.\n"},{"id":"412872","messageId":"20201222231702.GA17661@dcvr","threadId":"54169","inReplyTo":"xmqqbleliq58.fsf@gitster.c.googlers.com","subject":"Re: [PATCH] core.abbrev disables abbreviations","fromName":"Eric Wong","fromEmail":"e@80x24.org","sentAt":"2020-12-22T23:17:02Z","receivedAt":"2020-12-22T23:25:42Z","isPatch":true,"sender":{"key":"e@80x24.org","avatar":null},"body":"Junio C Hamano wrote:\n> Eric Wong writes:\n> \n> > Thanks for the comments, will wait a few days for comments\n> > before sending out v2.\n> \n> This has seen some review suggestions and as far as I remember, can\n> be summarised as:\n> \n> - there was a rough consensus that this was a desirable feature.\n> \n> - a one-off hardcoded list of \"false\" would rather want to be\n> consistent with config.c::git_parse_maybe_bool_text().\n> \n> - documentation is missing for the configuration variable.\n> \n> It has been almost three months; has a v2 been posted that I missed?\n\nCan somebody else take this over? Sorry my brain isn't working\nwell these months and I'm behind on other things :<\n"},{"id":"412874","messageId":"xmqq8s9pfkls.fsf@gitster.c.googlers.com","threadId":"54169","inReplyTo":"xmqqk0t9fmq6.fsf@gitster.c.googlers.com","subject":"Re: [PATCH] core.abbrev disables abbreviations","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2020-12-23T00:10:23Z","receivedAt":"2020-12-23T00:11:27Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"From: Eric Wong \nDate: Tue, 1 Sep 2020 07:43:55 +0000\nSubject: [PATCH] core.abbrev=no disables abbreviations\n\nThese allows users to write hash-agnostic scripts and configs to\ndisable abbreviations. Using \"-c core.abbrev=40\" will be\ninsufficient with SHA-256, and \"-c core.abbrev=64\" won't work\nwith SHA-1 repos today.\n\nSigned-off-by: Eric Wong \n[jc: tweaked implementation, added doc and a test]\nSigned-off-by: Junio C Hamano \n---\n Documentation/config/core.txt | 2 ++\n config.c | 2 ++\n t/t3200-branch.sh | 2 ++\n 3 files changed, 6 insertions(+)\n\ndiff --git a/Documentation/config/core.txt b/Documentation/config/core.txt\nindex 160aacad84..c04f62a54a 100644\n--- a/Documentation/config/core.txt\n+++ b/Documentation/config/core.txt\n@@ -625,4 +625,6 @@ core.abbrev::\n \tcomputed based on the approximate number of packed objects\n \tin your repository, which hopefully is enough for\n \tabbreviated object names to stay unique for some time.\n+\tIf set to \"no\", no abbreviation is made and the object names\n+\tare shown in their full length.\n \tThe minimum length is 4.\ndiff --git a/config.c b/config.c\nindex 1137bd73af..4c0cf3a1c1 100644\n--- a/config.c\n+++ b/config.c\n@@ -1217,6 +1217,8 @@ static int git_default_core_config(const char *var, const char *value, void *cb)\n \t\t\treturn config_error_nonbool(var);\n \t\tif (!strcasecmp(value, \"auto\"))\n \t\t\tdefault_abbrev = -1;\n+\t\telse if (!git_parse_maybe_bool_text(value))\n+\t\t\tdefault_abbrev = the_hash_algo->hexsz;\n \t\telse {\n \t\t\tint abbrev = git_config_int(var, value);\n \t\t\tif (abbrev < minimum_abbrev || abbrev > the_hash_algo->hexsz)\ndiff --git a/t/t3200-branch.sh b/t/t3200-branch.sh\nindex a0b832d59e..67db316911 100755\n--- a/t/t3200-branch.sh\n+++ b/t/t3200-branch.sh\n@@ -305,7 +305,9 @@ test_expect_success 'git branch --list -v with --abbrev' '\n \n \tgit branch -v --list --no-abbrev t >actual.noabbrev &&\n \tgit branch -v --list --abbrev=0 t >actual.0abbrev &&\n+\tgit -c core.abbrev=no branch -v --list t >actual.noabbrev-conf &&\n \ttest_cmp actual.noabbrev actual.0abbrev &&\n+\ttest_cmp actual.noabbrev actual.noabbrev-conf &&\n \n \tgit branch -v --list --abbrev=36 t >actual.36abbrev &&\n \t# how many hexdigits are used?\n-- \n2.30.0-rc1-197-gdf840da780\n\n"},{"id":"412929","messageId":"ab6e0536-19ed-99b3-a13d-873008b0f70e@gmail.com","threadId":"54169","inReplyTo":"xmqq8s9pfkls.fsf@gitster.c.googlers.com","subject":"Re: [PATCH] core.abbrev disables abbreviations","fromName":"Derrick Stolee","fromEmail":"stolee@gmail.com","sentAt":"2020-12-23T14:38:11Z","receivedAt":"2020-12-23T14:39:31Z","isPatch":true,"sender":{"key":"stolee@gmail.com","avatar":"https://avatars.githubusercontent.com/u/570044?v=4"},"body":"On 12/22/2020 7:10 PM, Junio C Hamano wrote:\n> From: Eric Wong \n> Date: Tue, 1 Sep 2020 07:43:55 +0000\n> Subject: [PATCH] core.abbrev=no disables abbreviations\n> \n> These allows users to write hash-agnostic scripts and configs to\n> disable abbreviations. Using \"-c core.abbrev=40\" will be\n> insufficient with SHA-256, and \"-c core.abbrev=64\" won't work\n> with SHA-1 repos today.\n> \n> Signed-off-by: Eric Wong \n> [jc: tweaked implementation, added doc and a test]\n> Signed-off-by: Junio C Hamano \n\nThanks for picking this back up. Your version LGTM.\n\n-Stolee\n"},{"id":"412946","messageId":"X+OmuBDBHhdzHjzn@camp.crustytoothpaste.net","threadId":"54169","inReplyTo":"xmqq8s9pfkls.fsf@gitster.c.googlers.com","subject":"Re: [PATCH] core.abbrev disables abbreviations","fromName":"brian m. carlson","fromEmail":"sandals@crustytoothpaste.net","sentAt":"2020-12-23T20:21:12Z","receivedAt":"2020-12-23T20:22:44Z","isPatch":true,"sender":{"key":"sandals@crustytoothpaste.net","avatar":"https://avatars.githubusercontent.com/u/497054?v=4"},"body":"On 2020-12-23 at 00:10:23, Junio C Hamano wrote:\n> From: Eric Wong \n> Date: Tue, 1 Sep 2020 07:43:55 +0000\n> Subject: [PATCH] core.abbrev=no disables abbreviations\n> \n> These allows users to write hash-agnostic scripts and configs to\n> disable abbreviations. Using \"-c core.abbrev=40\" will be\n> insufficient with SHA-256, and \"-c core.abbrev=64\" won't work\n> with SHA-1 repos today.\n> \n> Signed-off-by: Eric Wong \n> [jc: tweaked implementation, added doc and a test]\n> Signed-off-by: Junio C Hamano \n\nI agree this looks good, and it's a feature I think is valuable. Thanks\nto everyone involved.\n-- \nbrian m. carlson (he/him or they/them)\nHouston, Texas, US\n"}]}