{"thread":{"id":"47704","subject":"[PATCH v2 0/1] setup: recognise extensions.objectFormat","startedAt":"2018-01-28T00:36:25Z","lastAt":"2018-01-30T20:53:56Z","messageCount":9,"participants":["Patryk Obara","brian m. carlson","Jeff King","Junio C Hamano"],"isPatch":true,"patchVersion":2,"patchTotal":1},"messages":[{"id":"337595","messageId":"cover.1517098675.git.patryk.obara@gmail.com","threadId":"47704","inReplyTo":"af22d7feb8a8448fa3953c66e69a8257460bff07.1516800711.git.patryk.obara@gmail.com","subject":"[PATCH v2 0/1] setup: recognise extensions.objectFormat","fromName":"Patryk Obara","fromEmail":"patryk.obara@gmail.com","sentAt":"2018-01-28T00:36:16Z","receivedAt":"2018-01-28T00:36:25Z","isPatch":true,"sender":{"key":"patryk.obara@gmail.com","avatar":"https://avatars.githubusercontent.com/u/3967?v=4"},"body":"Compared to v1:\n\nImplemented code suggestions from Duy Nguyễn (string for translation and\nstrbuf instead of char array). I also added an annotation in\nrepository-version.txt, clarifying, that this option is useful only for\ndevelopment purpose for now.\n\nPatryk Obara (1):\n setup: recognise extensions.objectFormat\n\n Documentation/technical/repository-version.txt | 12 ++++++++++++\n setup.c | 27 ++++++++++++++++++++++++++\n t/t1302-repo-version.sh | 15 ++++++++++++++\n 3 files changed, 54 insertions(+)\n\n\nbase-commit: 5be1f00a9a701532232f57958efab4be8c959a29\n-- \n2.14.3\n\n"},{"id":"337596","messageId":"e430ad029facdd6209927d352f0e7545cdd0e435.1517098675.git.patryk.obara@gmail.com","threadId":"47704","inReplyTo":"cover.1517098675.git.patryk.obara@gmail.com","subject":"[PATCH v2 1/1] setup: recognise extensions.objectFormat","fromName":"Patryk Obara","fromEmail":"patryk.obara@gmail.com","sentAt":"2018-01-28T00:36:17Z","receivedAt":"2018-01-28T00:36:29Z","isPatch":true,"sender":{"key":"patryk.obara@gmail.com","avatar":"https://avatars.githubusercontent.com/u/3967?v=4"},"body":"This extension selects which hashing algorithm from vtable should be\nused for reading and writing objects in the object store. At the moment\nsupports only single value (sha-1).\n\nIn case value of objectFormat is an unknown hashing algorithm, Git\ncommand will fail with following message:\n\n fatal: unknown repository extensions found:\n\t objectformat = \n\nTo indicate, that this specific objectFormat value is not recognised.\n\nThe objectFormat extension is not allowed in repository marked as\nversion 0 to prevent any possibility of accidentally writing a NewHash\nobject in the sha-1 object store. This extension behaviour is different\nthan preciousObjects extension (which is allowed in repo version 0).\n\nAdd tests and documentation note about new extension.\n\nSigned-off-by: Patryk Obara \n---\n Documentation/technical/repository-version.txt | 12 ++++++++++++\n setup.c | 27 ++++++++++++++++++++++++++\n t/t1302-repo-version.sh | 15 ++++++++++++++\n 3 files changed, 54 insertions(+)\n\ndiff --git a/Documentation/technical/repository-version.txt b/Documentation/technical/repository-version.txt\nindex 00ad37986e..7e2b832603 100644\n--- a/Documentation/technical/repository-version.txt\n+++ b/Documentation/technical/repository-version.txt\n@@ -86,3 +86,15 @@ for testing format-1 compatibility.\n When the config key `extensions.preciousObjects` is set to `true`,\n objects in the repository MUST NOT be deleted (e.g., by `git-prune` or\n `git repack -d`).\n+\n+`objectFormat`\n+~~~~~~~~~~~~~~\n+\n+This extension instructs Git to use a specific algorithm for addressing\n+and interpreting objects in the object store. Currently, the only\n+supported object format is `sha-1`. At the moment, the primary purpose\n+of this option is to enable Git developers to experiment with different\n+hashing algorithms without re-compilation of git client.\n+\n+See `hash-function-transition.txt` document for more detailed explanation.\n+\ndiff --git a/setup.c b/setup.c\nindex 8cc34186ce..9b9993a14e 100644\n--- a/setup.c\n+++ b/setup.c\n@@ -405,6 +405,31 @@ void setup_work_tree(void)\n \tinitialized = 1;\n }\n \n+static int find_object_format(const char *value)\n+{\n+\tint i;\n+\tfor (i = GIT_HASH_SHA1; i < GIT_HASH_NALGOS; ++i) {\n+\t\tif (strcmp(value, hash_algos[i].name) == 0)\n+\t\t\treturn i;\n+\t}\n+\treturn GIT_HASH_UNKNOWN;\n+}\n+\n+static void detect_object_format(struct repository_format *data,\n+\t\t\t\t const char *value)\n+{\n+\tif (data->version == 0)\n+\t\tdie(_(\"invalid repository format version '%d'\"), data->version);\n+\n+\tdata->hash_algo = find_object_format(value);\n+\tif (data->hash_algo == GIT_HASH_UNKNOWN) {\n+\t\tstruct strbuf object_format = STRBUF_INIT;\n+\t\tstrbuf_addf(&object_format, \"objectformat = %s\", value);\n+\t\tstring_list_append(&data->unknown_extensions, object_format.buf);\n+\t\tstrbuf_release(&object_format);\n+\t}\n+}\n+\n static int check_repo_format(const char *var, const char *value, void *vdata)\n {\n \tstruct repository_format *data = vdata;\n@@ -422,6 +447,8 @@ static int check_repo_format(const char *var, const char *value, void *vdata)\n \t\t\t;\n \t\telse if (!strcmp(ext, \"preciousobjects\"))\n \t\t\tdata->precious_objects = git_config_bool(var, value);\n+\t\telse if (!strcmp(ext, \"objectformat\"))\n+\t\t\tdetect_object_format(data, value);\n \t\telse\n \t\t\tstring_list_append(&data->unknown_extensions, ext);\n \t} else if (strcmp(var, \"core.bare\") == 0) {\ndiff --git a/t/t1302-repo-version.sh b/t/t1302-repo-version.sh\nindex ce4cff13bb..227b397ff2 100755\n--- a/t/t1302-repo-version.sh\n+++ b/t/t1302-repo-version.sh\n@@ -107,4 +107,19 @@ test_expect_success 'gc runs without complaint' '\n \tgit gc\n '\n \n+test_expect_success 'object-format not allowed in repo version=0' '\n+\tmkconfig 0 \"objectFormat = sha-1\" >.git/config &&\n+\tcheck_abort\n+'\n+\n+test_expect_success 'object-format=sha-1 allowed' '\n+\tmkconfig 1 \"objectFormat = sha-1\" >.git/config &&\n+\tcheck_allow\n+'\n+\n+test_expect_success 'object-format=foo unsupported' '\n+\tmkconfig 1 \"objectFormat = foo\" >.git/config &&\n+\tcheck_abort\n+'\n+\n test_done\n-- \n2.14.3\n\n"},{"id":"337608","messageId":"20180128154022.GG431130@genre.crustytoothpaste.net","threadId":"47704","inReplyTo":"e430ad029facdd6209927d352f0e7545cdd0e435.1517098675.git.patryk.obara@gmail.com","subject":"Re: [PATCH v2 1/1] setup: recognise extensions.objectFormat","fromName":"brian m. carlson","fromEmail":"sandals@crustytoothpaste.net","sentAt":"2018-01-28T15:40:22Z","receivedAt":"2018-01-28T15:40:33Z","isPatch":true,"sender":{"key":"sandals@crustytoothpaste.net","avatar":"https://avatars.githubusercontent.com/u/497054?v=4"},"body":"On Sun, Jan 28, 2018 at 01:36:17AM +0100, Patryk Obara wrote:\n> This extension selects which hashing algorithm from vtable should be\n> used for reading and writing objects in the object store. At the moment\n> supports only single value (sha-1).\n\nI think you want an \"it\" here: \"At the moment *it* supports\".\n\n> In case value of objectFormat is an unknown hashing algorithm, Git\n> command will fail with following message:\n> \n> fatal: unknown repository extensions found:\n> \t objectformat = \n> \n> To indicate, that this specific objectFormat value is not recognised.\n> \n> The objectFormat extension is not allowed in repository marked as\n> version 0 to prevent any possibility of accidentally writing a NewHash\n> object in the sha-1 object store. This extension behaviour is different\n> than preciousObjects extension (which is allowed in repo version 0).\n> \n> Add tests and documentation note about new extension.\n> \n> Signed-off-by: Patryk Obara \n\nOther than that, the patch looks good to me. I like that we reject\ninvalid values immediately. Adding documentation is good, too.\n-- \nbrian m. carlson / brian with sandals: Houston, Texas, US\nhttps://www.crustytoothpaste.net/~bmc | My opinion only\nOpenPGP: https://keybase.io/bk2204\n"},{"id":"337664","messageId":"cover.1517241235.git.patryk.obara@gmail.com","threadId":"47704","inReplyTo":"cover.1517098675.git.patryk.obara@gmail.com","subject":"[PATCH v3 0/1] setup: recognise extensions.objectFormat","fromName":"Patryk Obara","fromEmail":"patryk.obara@gmail.com","sentAt":"2018-01-29T15:59:14Z","receivedAt":"2018-01-29T15:59:25Z","isPatch":true,"sender":{"key":"patryk.obara@gmail.com","avatar":"https://avatars.githubusercontent.com/u/3967?v=4"},"body":"Compred to v2:\n\nFixed commit message.\n\nPatryk Obara (1):\n setup: recognise extensions.objectFormat\n\n Documentation/technical/repository-version.txt | 12 ++++++++++++\n setup.c | 27 ++++++++++++++++++++++++++\n t/t1302-repo-version.sh | 15 ++++++++++++++\n 3 files changed, 54 insertions(+)\n\n\nbase-commit: 5be1f00a9a701532232f57958efab4be8c959a29\n-- \n2.14.3\n\n"},{"id":"337665","messageId":"ef3dacc7f0669a05987cd0a018926b9bdbf10aed.1517241235.git.patryk.obara@gmail.com","threadId":"47704","inReplyTo":"cover.1517241235.git.patryk.obara@gmail.com","subject":"[PATCH v3 1/1] setup: recognise extensions.objectFormat","fromName":"Patryk Obara","fromEmail":"patryk.obara@gmail.com","sentAt":"2018-01-29T15:59:15Z","receivedAt":"2018-01-29T15:59:29Z","isPatch":true,"sender":{"key":"patryk.obara@gmail.com","avatar":"https://avatars.githubusercontent.com/u/3967?v=4"},"body":"This extension selects which hashing algorithm from vtable should be\nused for reading and writing objects in the object store. At the moment\nit supports only single value (sha-1).\n\nIn case value of objectFormat is an unknown hashing algorithm, Git\ncommand will fail with the following message:\n\n fatal: unknown repository extensions found:\n\t objectformat = \n\nTo indicate, that this specific objectFormat value is not recognised.\n\nThe objectFormat extension is not allowed in repository marked as\nversion 0 to prevent any possibility of accidentally writing a NewHash\nobject in the sha-1 object store. This extension behaviour is different\nthan preciousObjects extension (which is allowed in repo version 0).\n\nAdd tests and documentation note about new extension.\n\nSigned-off-by: Patryk Obara \n---\n Documentation/technical/repository-version.txt | 12 ++++++++++++\n setup.c | 27 ++++++++++++++++++++++++++\n t/t1302-repo-version.sh | 15 ++++++++++++++\n 3 files changed, 54 insertions(+)\n\ndiff --git a/Documentation/technical/repository-version.txt b/Documentation/technical/repository-version.txt\nindex 00ad37986e..7e2b832603 100644\n--- a/Documentation/technical/repository-version.txt\n+++ b/Documentation/technical/repository-version.txt\n@@ -86,3 +86,15 @@ for testing format-1 compatibility.\n When the config key `extensions.preciousObjects` is set to `true`,\n objects in the repository MUST NOT be deleted (e.g., by `git-prune` or\n `git repack -d`).\n+\n+`objectFormat`\n+~~~~~~~~~~~~~~\n+\n+This extension instructs Git to use a specific algorithm for addressing\n+and interpreting objects in the object store. Currently, the only\n+supported object format is `sha-1`. At the moment, the primary purpose\n+of this option is to enable Git developers to experiment with different\n+hashing algorithms without re-compilation of git client.\n+\n+See `hash-function-transition.txt` document for more detailed explanation.\n+\ndiff --git a/setup.c b/setup.c\nindex 8cc34186ce..9b9993a14e 100644\n--- a/setup.c\n+++ b/setup.c\n@@ -405,6 +405,31 @@ void setup_work_tree(void)\n \tinitialized = 1;\n }\n \n+static int find_object_format(const char *value)\n+{\n+\tint i;\n+\tfor (i = GIT_HASH_SHA1; i < GIT_HASH_NALGOS; ++i) {\n+\t\tif (strcmp(value, hash_algos[i].name) == 0)\n+\t\t\treturn i;\n+\t}\n+\treturn GIT_HASH_UNKNOWN;\n+}\n+\n+static void detect_object_format(struct repository_format *data,\n+\t\t\t\t const char *value)\n+{\n+\tif (data->version == 0)\n+\t\tdie(_(\"invalid repository format version '%d'\"), data->version);\n+\n+\tdata->hash_algo = find_object_format(value);\n+\tif (data->hash_algo == GIT_HASH_UNKNOWN) {\n+\t\tstruct strbuf object_format = STRBUF_INIT;\n+\t\tstrbuf_addf(&object_format, \"objectformat = %s\", value);\n+\t\tstring_list_append(&data->unknown_extensions, object_format.buf);\n+\t\tstrbuf_release(&object_format);\n+\t}\n+}\n+\n static int check_repo_format(const char *var, const char *value, void *vdata)\n {\n \tstruct repository_format *data = vdata;\n@@ -422,6 +447,8 @@ static int check_repo_format(const char *var, const char *value, void *vdata)\n \t\t\t;\n \t\telse if (!strcmp(ext, \"preciousobjects\"))\n \t\t\tdata->precious_objects = git_config_bool(var, value);\n+\t\telse if (!strcmp(ext, \"objectformat\"))\n+\t\t\tdetect_object_format(data, value);\n \t\telse\n \t\t\tstring_list_append(&data->unknown_extensions, ext);\n \t} else if (strcmp(var, \"core.bare\") == 0) {\ndiff --git a/t/t1302-repo-version.sh b/t/t1302-repo-version.sh\nindex ce4cff13bb..227b397ff2 100755\n--- a/t/t1302-repo-version.sh\n+++ b/t/t1302-repo-version.sh\n@@ -107,4 +107,19 @@ test_expect_success 'gc runs without complaint' '\n \tgit gc\n '\n \n+test_expect_success 'object-format not allowed in repo version=0' '\n+\tmkconfig 0 \"objectFormat = sha-1\" >.git/config &&\n+\tcheck_abort\n+'\n+\n+test_expect_success 'object-format=sha-1 allowed' '\n+\tmkconfig 1 \"objectFormat = sha-1\" >.git/config &&\n+\tcheck_allow\n+'\n+\n+test_expect_success 'object-format=foo unsupported' '\n+\tmkconfig 1 \"objectFormat = foo\" >.git/config &&\n+\tcheck_abort\n+'\n+\n test_done\n-- \n2.14.3\n\n"},{"id":"337772","messageId":"20180130013759.GA27694@sigill.intra.peff.net","threadId":"47704","inReplyTo":"e430ad029facdd6209927d352f0e7545cdd0e435.1517098675.git.patryk.obara@gmail.com","subject":"Re: [PATCH v2 1/1] setup: recognise extensions.objectFormat","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2018-01-30T01:38:00Z","receivedAt":"2018-01-30T01:38:07Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Sun, Jan 28, 2018 at 01:36:17AM +0100, Patryk Obara wrote:\n\n> This extension selects which hashing algorithm from vtable should be\n> used for reading and writing objects in the object store. At the moment\n> supports only single value (sha-1).\n> \n> In case value of objectFormat is an unknown hashing algorithm, Git\n> command will fail with following message:\n> \n> fatal: unknown repository extensions found:\n> \t objectformat = \n> \n> To indicate, that this specific objectFormat value is not recognised.\n\nI don't have a strong opinion on this, but it does feel a little funny\nto add this extension now, before we quite know what the code that uses\nit is going to look like (or maybe we're farther along there than I\nrealize).\n\nWhat do we gain by doing this now as opposed to later? By the design of\nthe extension code, we should complain on older versions anyway. And by\ndoing it now we carry a small risk that it might not give us the\ninterface we want, and it will be slightly harder to paper over this\nfailed direction.\n\nAll that said, if people like brian, who are thinking more about this\ntransition than I am, are onboard, I'm OK with it.\n\n> The objectFormat extension is not allowed in repository marked as\n> version 0 to prevent any possibility of accidentally writing a NewHash\n> object in the sha-1 object store. This extension behaviour is different\n> than preciousObjects extension (which is allowed in repo version 0).\n\nIt wasn't intended that anyone would specify preciousObjects with repo\nversion 0. It's a dangerous misconfiguration (because versions which\npredate the extensions mechanism won't actually respect it at all!).\n\nSo we probably ought to complain loudly on having anything in\nextensions.* when the repositoryformat is less than 1.\n\nI originally wrote it the other way out of an abundance of\nbackward-compatibility. After all \"extension.*\" doesn't mean anything in\nformat 0, and somebody _could_ have added such a config key for their\nown purposes. But that's a pretty weak argument, and if we are going to\nstart marking some extensions as forbidden there, we might as well do\nthem all.\n\nSomething like this:\n\ndiff --git a/cache.h b/cache.h\nindex d8b975a571..259c4a5361 100644\n--- a/cache.h\n+++ b/cache.h\n@@ -921,6 +921,7 @@ struct repository_format {\n \tint is_bare;\n \tint hash_algo;\n \tchar *work_tree;\n+\tint extensions_seen;\n \tstruct string_list unknown_extensions;\n };\n \ndiff --git a/setup.c b/setup.c\nindex 8cc34186ce..85dfcf330b 100644\n--- a/setup.c\n+++ b/setup.c\n@@ -413,6 +413,7 @@ static int check_repo_format(const char *var, const char *value, void *vdata)\n \tif (strcmp(var, \"core.repositoryformatversion\") == 0)\n \t\tdata->version = git_config_int(var, value);\n \telse if (skip_prefix(var, \"extensions.\", &ext)) {\n+\t\tdata->extensions_seen = 1;\n \t\t/*\n \t\t * record any known extensions here; otherwise,\n \t\t * we fall through to recording it as unknown, and\n@@ -503,6 +504,11 @@ int verify_repository_format(const struct repository_format *format,\n \t\treturn -1;\n \t}\n \n+\tif (format->version < 1 && format->extensions_seen) {\n+\t\tstrbuf_addstr(err, _(\"extensions found but repo version is < 1\"));\n+\t\treturn -1;\n+\t}\n+\n \tif (format->version >= 1 && format->unknown_extensions.nr) {\n \t\tint i;\n \ndiff --git a/t/t1302-repo-version.sh b/t/t1302-repo-version.sh\nindex ce4cff13bb..9e9f67d756 100755\n--- a/t/t1302-repo-version.sh\n+++ b/t/t1302-repo-version.sh\n@@ -80,9 +80,10 @@ while read outcome version extensions; do\n done <<\\EOF\n allow 0\n allow 1\n+abort 0 noop\n allow 1 noop\n+abort 0 no-such-extension\n abort 1 no-such-extension\n-allow 0 no-such-extension\n EOF\n \n test_expect_success 'precious-objects allowed' '\n"},{"id":"337791","messageId":"e3c203f8-7971-40ce-8d9e-2dfe35f51a8a@gmail.com","threadId":"47704","inReplyTo":"20180130013759.GA27694@sigill.intra.peff.net","subject":"Re: [PATCH v2 1/1] setup: recognise extensions.objectFormat","fromName":"Patryk Obara","fromEmail":"patryk.obara@gmail.com","sentAt":"2018-01-30T16:30:04Z","receivedAt":"2018-01-30T16:30:20Z","isPatch":true,"sender":{"key":"patryk.obara@gmail.com","avatar":"https://avatars.githubusercontent.com/u/3967?v=4"},"body":"On 30/01/2018 02:38, Jeff King wrote:\n> On Sun, Jan 28, 2018 at 01:36:17AM +0100, Patryk Obara wrote:\n> \n>> This extension selects which hashing algorithm from vtable should be\n>> used for reading and writing objects in the object store. At the moment\n>> supports only single value (sha-1).\n>>\n>> In case value of objectFormat is an unknown hashing algorithm, Git\n>> command will fail with following message:\n>>\n>> fatal: unknown repository extensions found:\n>> \t objectformat = \n>>\n>> To indicate, that this specific objectFormat value is not recognised.\n> \n> I don't have a strong opinion on this, but it does feel a little funny\n> to add this extension now, before we quite know what the code that uses\n> it is going to look like (or maybe we're farther along there than I\n> realize).\n\nCode using this is already in master - in the result of overwriting\ndata->hash_algo, every piece of code, that was modernised starts using\nthe selected hash algorithm (through the_hash_algo) instead of hardcoded\nsha-1.\n\nAs far as I can tell status is following:\n\nOnce following topics will land:\n- po/object-id (in pu)\n- brian's object-id-part-11 (in review now)\n- upcoming brian's object-id-part-12 (not sent to mailing list yet)\n- few more object-id conversions and uses of the_hash_algo\n\nwe'll be in state, where just dropping new entry in hash_algos table\nwill enable experimental switching of object format.\n\nWith changes listed above \"git hash-object -w -t blob\" and\n\"git cat-file\" work with NewHash (whatever it may be - brian is using \nblake2 in his experiments, I am using openssl sha3-256).\n\nRight now I am looking at updating index structures and functions - \nafter which git commit should work. In the transition plan it's \ndescribed as \"introducing index v3\" (are there any new requirements, \nthat constitute \"v3\" besides longer hash?).\n\n> What do we gain by doing this now as opposed to later? By the design of\n> the extension code, we should complain on older versions anyway. And by\n> doing it now we carry a small risk that it might not give us the\n> interface we want, and it will be slightly harder to paper over this\n> failed direction.\n\nMostly convenience for developers, who want to work on transition. \nThere's no need to re-compile only for changing default hashing \nalgorithm (which is useful for testing and debugging). I could carry \nthis patch around to every NewHash-related branch, that I work on but \nit's annoying me already ;)\n\nAs long as hash_algos table contains only sha-1, users effectively see\nthis extension as noop.\n\n> It wasn't intended that anyone would specify preciousObjects with repo\n> version 0. It's a dangerous misconfiguration (because versions which\n> predate the extensions mechanism won't actually respect it at all!).\n> \n> So we probably ought to complain loudly on having anything in\n> extensions.* when the repositoryformat is less than 1.\n >\n> I originally wrote it the other way out of an abundance of\n> backward-compatibility. After all \"extension.*\" doesn't mean anything in\n> format 0, and somebody _could_ have added such a config key for their\n> own purposes. But that's a pretty weak argument, and if we are going to\n> start marking some extensions as forbidden there, we might as well do\n> them all.\n\nWhat about users, who are using new version of Git, but have it \nmisconfigured with preciousObjects and repo format 0? That's why I \ndecided to make repo format check specific to objectFormat extension \n(initially I made it generic to all extensions).\n\nAt the same time... there's extension.partialclone in pu and it does not \nhave check on repo format.\n\n-- \n| ← Ceci n'est pas une pipe\nPatryk Obara\n"},{"id":"337792","messageId":"20180130164148.GA5053@sigill.intra.peff.net","threadId":"47704","inReplyTo":"e3c203f8-7971-40ce-8d9e-2dfe35f51a8a@gmail.com","subject":"Re: [PATCH v2 1/1] setup: recognise extensions.objectFormat","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2018-01-30T16:41:48Z","receivedAt":"2018-01-30T16:41:56Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Tue, Jan 30, 2018 at 05:30:04PM +0100, Patryk Obara wrote:\n\n> > I don't have a strong opinion on this, but it does feel a little funny\n> > to add this extension now, before we quite know what the code that uses\n> > it is going to look like (or maybe we're farther along there than I\n> > realize).\n> \n> Code using this is already in master - in the result of overwriting\n> data->hash_algo, every piece of code, that was modernised starts using\n> the selected hash algorithm (through the_hash_algo) instead of hardcoded\n> sha-1.\n\nRight, that part seems pretty simple. But in the long run, is that going\nto be enough for the hash transition? My impression is that the\ntransition document is going to require a more nuanced view than \"this\nis the hash algorithm for this repo\".\n\nPutting code in master is OK; we can always refactor it. But once we\nadd and document a user-facing config option like this, we have to\nsupport it forever. So that's really the step I was wondering about: are\nwe sure this is what the user-facing config is going to look like?\n\n> > What do we gain by doing this now as opposed to later? By the design of\n> > the extension code, we should complain on older versions anyway. And by\n> > doing it now we carry a small risk that it might not give us the\n> > interface we want, and it will be slightly harder to paper over this\n> > failed direction.\n> \n> Mostly convenience for developers, who want to work on transition. There's\n> no need to re-compile only for changing default hashing algorithm (which is\n> useful for testing and debugging). I could carry this patch around to every\n> NewHash-related branch, that I work on but it's annoying me already ;)\n\nOK, that makes some sense to me. Even if we may end up with a more\nnuanced config later, this is useful for getting the first step done:\njust making a standalone NewHash repo without worrying about\ninteroperation with existing history.\n\n> > I originally wrote it the other way out of an abundance of\n> > backward-compatibility. After all \"extension.*\" doesn't mean anything in\n> > format 0, and somebody _could_ have added such a config key for their\n> > own purposes. But that's a pretty weak argument, and if we are going to\n> > start marking some extensions as forbidden there, we might as well do\n> > them all.\n> \n> What about users, who are using new version of Git, but have it\n> misconfigured with preciousObjects and repo format 0? That's why I decided\n> to make repo format check specific to objectFormat extension (initially I\n> made it generic to all extensions).\n\nBut that's sort of my point. It appears to be working, but the\nprior-version safety they think they have is not there. I think we're\nbetter off erring on the side of caution here, and letting them know\nforcefully that their config is bogus.\n\n> At the same time... there's extension.partialclone in pu and it does not\n> have check on repo format.\n\nIMHO it should (and we should just do it by enforcing it for all\nextensions automatically).\n\n-Peff\n"},{"id":"337810","messageId":"xmqq8tcft6ec.fsf@gitster-ct.c.googlers.com","threadId":"47704","inReplyTo":"20180130164148.GA5053@sigill.intra.peff.net","subject":"Re: [PATCH v2 1/1] setup: recognise extensions.objectFormat","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2018-01-30T20:53:47Z","receivedAt":"2018-01-30T20:53:56Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jeff King writes:\n\n> Putting code in master is OK; we can always refactor it. But once we\n> add and document a user-facing config option like this, we have to\n> support it forever. So that's really the step I was wondering about: are\n> we sure this is what the user-facing config is going to look like?\n\nYup, that is an important distinction.\n\n> But that's sort of my point. It appears to be working, but the\n> prior-version safety they think they have is not there. I think we're\n> better off erring on the side of caution here, and letting them know\n> forcefully that their config is bogus.\n>\n>> At the same time... there's extension.partialclone in pu and it does not\n>> have check on repo format.\n>\n> IMHO it should (and we should just do it by enforcing it for all\n> extensions automatically).\n\nSounds good.\n"}]}