Hi,
Stefan Beller wrote:
> This patch tightens the check upfront, such that we do not need
> to spawn a child process to find out if the submodule is broken.
Sounds sensible.
[...]
> --- a/submodule.c
> +++ b/submodule.c
[...]
> @@ -1319,10 +1338,23 @@ static int get_next_submodule(struct child_process *cp,
> argv_array_push(&cp->args, default_argv);
> argv_array_push(&cp->args, "--submodule-prefix");
> argv_array_push(&cp->args, submodule_prefix.buf);
> +
> + repo_clear(repo);
> + free(repo);
> ret = 1;
> + } else {
> + /*
> + * An empty directory is normal,
> + * the submodule is not initialized
> + */
> + if (S_ISGITLINK(ce->ce_mode) &&
> + !is_empty_dir(ce->name)) {
What if the directory is nonempty (e.g. contains build artifacts)?
> + spf->result = 1;
> + strbuf_addf(err,
> + _("Could not access submodule '%s'"),
> + ce->name);
> + }
Should this exit the loop? Otherwise, multiple "Could not access"
messages can go in the same err string a big concatenated line.
Thanks,
Jonathan
