| Message ID | 20200723010943.2329634-33-sandals@crustytoothpaste.net (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | SHA-256, part 3/3 | expand |
"brian m. carlson" <sandals@crustytoothpaste.net> writes: > @@ -613,6 +622,11 @@ int verify_repository_format(const struct repository_format *format, > return -1; > } > > + if (format->version <= 0 && format->hash_algo != GIT_HASH_SHA1) { > + strbuf_addstr(err, _("extensions.objectFormat is not valid in repo v0")); > + return -1; > + } > + > return 0; > } > By declaring that the repository is invalid if its version is less than 1 and objectFormat extension defined, we prevent unwanted upgrading from happening by mistake. OK.
On 2020-07-23 at 02:04:52, Junio C Hamano wrote: > "brian m. carlson" <sandals@crustytoothpaste.net> writes: > > > @@ -613,6 +622,11 @@ int verify_repository_format(const struct repository_format *format, > > return -1; > > } > > > > + if (format->version <= 0 && format->hash_algo != GIT_HASH_SHA1) { > > + strbuf_addstr(err, _("extensions.objectFormat is not valid in repo v0")); > > + return -1; > > + } > > + > > return 0; > > } > > > > By declaring that the repository is invalid if its version is less > than 1 and objectFormat extension defined, we prevent unwanted > upgrading from happening by mistake. Yes, and more specifically: * If the repository is v0 and has an objectFormat set, we fail in newer versions of Git (i.e., after this series). Older versions which do not support the extension will see breakage (because unknown extensions are not fatal in v0), but we hope by adding this check that nobody will ever configure a repo this way, since it will be totally nonfunctional in this state, regardless of version. * If the repository is v1 and has an objectFormat set, we work with newer Git and everything is great. Older Git versions fail hard here, and the user gets a moderately helpful error message. v2 of the series just ignored the setting in v0, which would make it equally broken in older and newer versions, but would provide a less useful error message (probably about a corrupt index).
"brian m. carlson" <sandals@crustytoothpaste.net> writes: >> By declaring that the repository is invalid if its version is less >> than 1 and objectFormat extension defined, we prevent unwanted >> upgrading from happening by mistake. > > Yes, and more specifically: > > * If the repository is v0 and has an objectFormat set, we fail in newer > versions of Git (i.e., after this series). Older versions which do > not support the extension will see breakage (because unknown > extensions are not fatal in v0), but we hope by adding this check that > nobody will ever configure a repo this way, since it will be totally > nonfunctional in this state, regardless of version. > * If the repository is v1 and has an objectFormat set, we work with > newer Git and everything is great. Older Git versions fail hard here, > and the user gets a moderately helpful error message. > > v2 of the series just ignored the setting in v0, which would make it > equally broken in older and newer versions, but would provide a less > useful error message (probably about a corrupt index). Peff's 'jk/reject-newer-extensions-in-v0' uses a bit refactored code to make it easier to add only-v1-and-later extensions while rejecting them, even if the code knows about them, in v0 repository. Even though the mechanism is a bit different, the spirit is quite the same as this step. Please double check origin/seen:setup.c to see if I resolved textual conflicts in a sensible way. Thanks.
On 2020-07-23 at 04:15:37, Junio C Hamano wrote: > "brian m. carlson" <sandals@crustytoothpaste.net> writes: > > >> By declaring that the repository is invalid if its version is less > >> than 1 and objectFormat extension defined, we prevent unwanted > >> upgrading from happening by mistake. > > > > Yes, and more specifically: > > > > * If the repository is v0 and has an objectFormat set, we fail in newer > > versions of Git (i.e., after this series). Older versions which do > > not support the extension will see breakage (because unknown > > extensions are not fatal in v0), but we hope by adding this check that > > nobody will ever configure a repo this way, since it will be totally > > nonfunctional in this state, regardless of version. > > * If the repository is v1 and has an objectFormat set, we work with > > newer Git and everything is great. Older Git versions fail hard here, > > and the user gets a moderately helpful error message. > > > > v2 of the series just ignored the setting in v0, which would make it > > equally broken in older and newer versions, but would provide a less > > useful error message (probably about a corrupt index). > > Peff's 'jk/reject-newer-extensions-in-v0' uses a bit refactored code > to make it easier to add only-v1-and-later extensions while rejecting > them, even if the code knows about them, in v0 repository. Even > though the mechanism is a bit different, the spirit is quite the > same as this step. > > Please double check origin/seen:setup.c to see if I resolved textual > conflicts in a sensible way. Yes, this seems quite sensible, and exactly the resolution I was hoping for.
diff --git a/setup.c b/setup.c index 3a81307602..94e68bb4f4 100644 --- a/setup.c +++ b/setup.c @@ -470,7 +470,16 @@ static int check_repo_format(const char *var, const char *value, void *vdata) data->partial_clone = xstrdup(value); } else if (!strcmp(ext, "worktreeconfig")) data->worktree_config = git_config_bool(var, value); - else + else if (!strcmp(ext, "objectformat")) { + int format; + + if (!value) + return config_error_nonbool(var); + format = hash_algo_by_name(value); + if (format == GIT_HASH_UNKNOWN) + return error("invalid value for 'extensions.objectformat'"); + data->hash_algo = format; + } else string_list_append(&data->unknown_extensions, ext); } @@ -613,6 +622,11 @@ int verify_repository_format(const struct repository_format *format, return -1; } + if (format->version <= 0 && format->hash_algo != GIT_HASH_SHA1) { + strbuf_addstr(err, _("extensions.objectFormat is not valid in repo v0")); + return -1; + } + return 0; }
The transition plan specifies extensions.objectFormat as the indication that we're using a given hash in a certain repo. Read this as one of the extensions we support. If the user has specified an invalid value, fail. Ensure that we reject the extension if the repository format version is 0. Signed-off-by: brian m. carlson <sandals@crustytoothpaste.net> --- setup.c | 16 +++++++++++++++- 1 file changed, 15 insertions(+), 1 deletion(-)