Re: [PATCHv9 4/6] git submodule update: have a dedicated helper for cloning
- From
Junio C Hamano <gitster@pobox.com>
- Date
- Feb 9, 2016, 21:24 UTC
- Message-ID
- <xmqq8u2t37bt.fsf@gitster.mtv.corp.google.com>
- In-Reply-To
- <1455051274-15256-5-git-send-email-sbeller@google.com>
Stefan Beller <sbeller@google.com> writes:
Show 14 quoted lines
> + for (; pp->count < pp->list.nr; pp->count++) {
> + const struct submodule *sub = NULL;
> + const struct cache_entry *ce = pp->list.entries[pp->count];
> + struct strbuf displaypath_sb = STRBUF_INIT;
> + struct strbuf sb = STRBUF_INIT;
> + const char *displaypath = NULL;
> + char *url = NULL;
> + int needs_cloning = 0;
> +
> + if (ce_stage(ce)) {
> + if (pp->recursive_prefix)
> + strbuf_addf(err,
> + "Skipping unmerged submodule %s/%s\n",
> + pp->recursive_prefix, ce->name);The funny indentation of the string is a workaround for overly deep nesting, but is the overly deep nesting telling us that perhaps one iteration of this loop can be an invocation of a helper function, I wonder?
Show 6 quoted lines
> + else > + strbuf_addf(err, > + "Skipping unmerged submodule %s\n", > + ce->name); > + goto cleanup_and_continue; > + }
Show 11 quoted lines
> +
> + sub = submodule_from_path(null_sha1, ce->name);
> +
> + if (pp->recursive_prefix)
> + displaypath = relative_path(pp->recursive_prefix,
> + ce->name, &displaypath_sb);
> + else
> + displaypath = ce->name;
> +
> + if ((pp->update && !strcmp(pp->update, "none")) ||
> + (!pp->update && sub->update == SM_UPDATE_NONE)) {This looks a bit strange. I wonder pp->update should also become enum for the same reason why sub->update has become enum. That way, we need to be worried about parsing these tokens in one place where a textual string "none" is translated to SM_UPDATE_NONE. If we started allowing "None" in the sub->update parse_config() in submodule-config.c, we would want that new parsing rule propagated to pp->update, right?
> + strbuf_addf(err, "Skipping submodule '%s'\n", > + displaypath); > + goto cleanup_and_continue; > + }