Re: [PATCH 03/14] make is_submodule_populated gently
- From
Junio C Hamano <gitster@pobox.com>
- Date
- Feb 15, 2017, 18:10 UTC
- Message-ID
- <xmqqlgt7495z.fsf@gitster.mtv.corp.google.com>
- In-Reply-To
- <20170215003423.20245-4-sbeller@google.com>
Stefan Beller <sbeller@google.com> writes:
> We need the gentle version in a later patch. As we have just one caller, > migrate the caller.
Ordinarily, we keep the original helper implemented as a thin wrapper that passes NULL as retun_error_code, which causes it to die() on error for existing callers. But because we only have one caller (and topics in-flight do not add new ones), we do not bother with that.
The reasoning makes sense, at least to me.
We may want to add a comment about the behaviour upon error for the helper function? I see resolve_gitdir_gently() does not do so and the readers have to follow the callflow down to read_gitfile_gently() which does have the comment, so perhaps we are OK without any.
Looks good to me.
Show 52 quoted lines
> Signed-off-by: Stefan Beller <sbeller@google.com>
> ---
> builtin/grep.c | 2 +-
> submodule.c | 4 ++--
> submodule.h | 2 +-
> 3 files changed, 4 insertions(+), 4 deletions(-)
>
> diff --git a/builtin/grep.c b/builtin/grep.c
> index 2c727ef499..b17835aed6 100644
> --- a/builtin/grep.c
> +++ b/builtin/grep.c
> @@ -616,7 +616,7 @@ static int grep_submodule(struct grep_opt *opt, const unsigned char *sha1,
> {
> if (!is_submodule_initialized(path))
> return 0;
> - if (!is_submodule_populated(path)) {
> + if (!is_submodule_populated_gently(path, NULL)) {
> /*
> * If searching history, check for the presense of the
> * submodule's gitdir before skipping the submodule.
> diff --git a/submodule.c b/submodule.c
> index 3b98766a6b..9bbdd3ce7c 100644
> --- a/submodule.c
> +++ b/submodule.c
> @@ -237,12 +237,12 @@ int is_submodule_initialized(const char *path)
> /*
> * Determine if a submodule has been populated at a given 'path'
> */
> -int is_submodule_populated(const char *path)
> +int is_submodule_populated_gently(const char *path, int *return_error_code)
> {
> int ret = 0;
> char *gitdir = xstrfmt("%s/.git", path);
>
> - if (resolve_gitdir(gitdir))
> + if (resolve_gitdir_gently(gitdir, return_error_code))
> ret = 1;
>
> free(gitdir);
> diff --git a/submodule.h b/submodule.h
> index 05ab674f06..689033e538 100644
> --- a/submodule.h
> +++ b/submodule.h
> @@ -41,7 +41,7 @@ extern int submodule_config(const char *var, const char *value, void *cb);
> extern void gitmodules_config(void);
> extern void gitmodules_config_sha1(const unsigned char *commit_sha1);
> extern int is_submodule_initialized(const char *path);
> -extern int is_submodule_populated(const char *path);
> +extern int is_submodule_populated_gently(const char *path, int *return_error_code);
> extern int parse_submodule_update_strategy(const char *value,
> struct submodule_update_strategy *dst);
> extern const char *submodule_strategy_to_string(const struct submodule_update_strategy *s);