On 11/2/2017 6:24 PM, Jonathan Tan wrote:
On Thu, 2 Nov 2017 20:20:44 +0000 Jeff Hostetler <git@xxxxxxxxxxxxxxxxx> wrote:From: Jeff Hostetler <jeffhost@xxxxxxxxxxxxx> Introduce the ability to have missing objects in a repo. This functionality is guarded by new repository extension options: `extensions.partialcloneremote` and `extensions.partialclonefilter`.With this, it is unclear what happens if extensions.partialcloneremote is not set but extensions.partialclonefilter is set. For something as significant as a repository extension (which Git uses to determine if it will even attempt to interact with a repo), I think - I would prefer just extensions.partialclone (or extensions.partialcloneremote, though I prefer the former) which determines the remote (the important part, which controls the dynamic object fetching), and have another option "core.partialclonefilter" which is only useful if "extensions.partialclone" is set.
Yes, that is a point I wanted to ask about. I renamed the extensions.partialclone that you created and then I moved your remote.<name>.blob-max-bytes setting to be in extensions too. Moving it to core.partialclonefilter is fine.
I also don't think extensions.partialclonefilter (or core.partialclonefilter, if we use my suggestion) needs to be introduced so early in the patch set when it will only be used once we start fetching/cloning.+void partial_clone_utils_register( + const struct list_objects_filter_options *filter_options, + const char *remote, + const char *cmd_name) +{This function is useful once we have fetch/clone, but probably not before that. Since the fetch/clone patches are several patches ahead, could this be moved there?
Sure.
@@ -420,6 +420,19 @@ static int check_repo_format(const char *var, const char *value, void *vdata) ; else if (!strcmp(ext, "preciousobjects")) data->precious_objects = git_config_bool(var, value); + + else if (!strcmp(ext, KEY_PARTIALCLONEREMOTE)) + if (!value) + return config_error_nonbool(var); + else + data->partial_clone_remote = xstrdup(value); + + else if (!strcmp(ext, KEY_PARTIALCLONEFILTER)) + if (!value) + return config_error_nonbool(var); + else + data->partial_clone_filter = xstrdup(value); + else string_list_append(&data->unknown_extensions, ext); } else if (strcmp(var, "core.bare") == 0) {With a complicated block, probably better to use braces in these clauses.
Good point. Thanks, Jeff
