Re: [PATCH v2 2/6] submodules: load gitmodules file from commit sha1

From: Stefan Beller <sbeller@google.com>
Date: Nov 1, 2016, 16:39 UTC
Message-ID: <CAGZ79kYtZXqN5gd2LDdZAik4WxMU66oKRZOtvuAQ-Nwv7SDH+Q@mail.gmail.com>
In-Reply-To: <1477953496-103596-3-git-send-email-bmwill@google.com>

On Mon, Oct 31, 2016 at 3:38 PM, Brandon Williams <bmwill@google.com> wrote:

Show 42 quoted lines

> Teach submodules to load a '.gitmodules' file from a commit sha1.  This
> enables the population of the submodule_cache to be based on the state
> of the '.gitmodules' file from a particular commit.
>
> Signed-off-by: Brandon Williams <bmwill@google.com>
> ---
>  cache.h            |  2 ++
>  config.c           |  8 ++++----
>  submodule-config.c |  6 +++---
>  submodule-config.h |  3 +++
>  submodule.c        | 12 ++++++++++++
>  submodule.h        |  1 +
>  6 files changed, 25 insertions(+), 7 deletions(-)
>
> diff --git a/cache.h b/cache.h
> index 1be6526..559a461 100644
> --- a/cache.h
> +++ b/cache.h
> @@ -1690,6 +1690,8 @@ extern int git_default_config(const char *, const char *, void *);
>  extern int git_config_from_file(config_fn_t fn, const char *, void *);
>  extern int git_config_from_mem(config_fn_t fn, const enum config_origin_type,
>                                         const char *name, const char *buf, size_t len, void *data);
> +extern int git_config_from_blob_sha1(config_fn_t fn, const char *name,
> +                                    const unsigned char *sha1, void *data);
>  extern void git_config_push_parameter(const char *text);
>  extern int git_config_from_parameters(config_fn_t fn, void *data);
>  extern void git_config(config_fn_t fn, void *);
> diff --git a/config.c b/config.c
> index 83fdecb..4d78e72 100644
> --- a/config.c
> +++ b/config.c
> @@ -1214,10 +1214,10 @@ int git_config_from_mem(config_fn_t fn, const enum config_origin_type origin_typ
>         return do_config_from(&top, fn, data);
>  }
>
> -static int git_config_from_blob_sha1(config_fn_t fn,
> -                                    const char *name,
> -                                    const unsigned char *sha1,
> -                                    void *data)
> +int git_config_from_blob_sha1(config_fn_t fn,
> +                             const char *name,
> +                             const unsigned char *sha1,

While looking at this code, we may want to investigate if a conversion to struct object_id (instead of const char * for sha1) is feasible.

Show 32 quoted lines

> +                             void *data)
>  {
>         enum object_type type;
>         char *buf;
> diff --git a/submodule-config.c b/submodule-config.c
> index 098085b..8b9a2ef 100644
> --- a/submodule-config.c
> +++ b/submodule-config.c
> @@ -379,9 +379,9 @@ static int parse_config(const char *var, const char *value, void *data)
>         return ret;
>  }
>
> -static int gitmodule_sha1_from_commit(const unsigned char *commit_sha1,
> -                                     unsigned char *gitmodules_sha1,
> -                                     struct strbuf *rev)
> +int gitmodule_sha1_from_commit(const unsigned char *commit_sha1,
> +                              unsigned char *gitmodules_sha1,
> +                              struct strbuf *rev)
>  {
>         int ret = 0;
>
> diff --git a/submodule-config.h b/submodule-config.h
> index d05c542..78584ba 100644
> --- a/submodule-config.h
> +++ b/submodule-config.h
> @@ -29,6 +29,9 @@ const struct submodule *submodule_from_name(const unsigned char *commit_sha1,
>                 const char *name);
>  const struct submodule *submodule_from_path(const unsigned char *commit_sha1,
>                 const char *path);
> +extern int gitmodule_sha1_from_commit(const unsigned char *commit_sha1,
> +                                     unsigned char *gitmodules_sha1,
> +                                     struct strbuf *rev);

no need for extern here as it is a function declaration, not a variable declaration; (as said on patch 1, I think consistency to the surrounding is important here)

Show 35 quoted lines

>  void submodule_free(void);
>
>  #endif /* SUBMODULE_CONFIG_H */
> diff --git a/submodule.c b/submodule.c
> index ff4e7b2..19dfbd4 100644
> --- a/submodule.c
> +++ b/submodule.c
> @@ -198,6 +198,18 @@ void gitmodules_config(void)
>         }
>  }
>
> +void gitmodules_config_sha1(const unsigned char *commit_sha1)
> +{
> +       struct strbuf rev = STRBUF_INIT;
> +       unsigned char sha1[20];
> +
> +       if (gitmodule_sha1_from_commit(commit_sha1, sha1, &rev)) {
> +               git_config_from_blob_sha1(submodule_config, rev.buf,
> +                                         sha1, NULL);
> +       }
> +       strbuf_release(&rev);
> +}
> +
>  /*
>   * Determine if a submodule has been initialized at a given 'path'
>   */
> diff --git a/submodule.h b/submodule.h
> index bd039ca..9a24ac8 100644
> --- a/submodule.h
> +++ b/submodule.h
> @@ -37,6 +37,7 @@ void set_diffopt_flags_from_submodule_config(struct diff_options *diffopt,
>                 const char *path);
>  int submodule_config(const char *var, const char *value, void *cb);
>  void gitmodules_config(void);
> +extern void gitmodules_config_sha1(const unsigned char *commit_sha1);

same.

Show 6 quoted lines

>  extern int is_submodule_initialized(const char *path);
>  extern int is_submodule_checked_out(const char *path);
>  int parse_submodule_update_strategy(const char *value,
> --
> 2.8.0.rc3.226.g39d4020
>