Re: [WIP/PATCH 9/9] submodule: teach unpack_trees() to update submodules

[Duy Nguyen <pclouds@gmail.com>]

On Tue, Feb 4, 2014 at 2:54 AM, Jens Lehmann <Jens.Lehmann@web.de> wrote:
> Implement the functionality needed to enable work tree manipulating
> commands so that an changed submodule does not only affect the index but
> it also updates the work tree of any initialized submodule according to
> the SHA-1 recorded in the superproject.
>
> Signed-off-by: Jens Lehmann <Jens.Lehmann@web.de>
> ---
>  entry.c        | 15 ++++++++--
>  submodule.c    | 86 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
>  submodule.h    |  3 ++
>  unpack-trees.c | 69 ++++++++++++++++++++++++++++++++++++----------
>  unpack-trees.h |  1 +
>  5 files changed, 157 insertions(+), 17 deletions(-)
>
> diff --git a/entry.c b/entry.c
> index d1bf6ec..61a2767 100644
> --- a/entry.c
> +++ b/entry.c
> @@ -265,7 +265,7 @@ int checkout_entry(struct cache_entry *ce,
>
>         if (!check_path(path, len, &st, state->base_dir_len)) {
>                 unsigned changed = ce_match_stat(ce, &st, CE_MATCH_IGNORE_VALID|CE_MATCH_IGNORE_SKIP_WORKTREE);
> -               if (!changed)
> +               if (!changed && (!S_ISDIR(st.st_mode) || !S_ISGITLINK(ce->ce_mode)))
>                         return 0;

Should we report something when ce is a gitlink, but path is not a
directory, instead of siliently exit?

> diff --git a/submodule.c b/submodule.c
> index 3907034..83e7595 100644
> --- a/submodule.c
> +++ b/submodule.c
> @@ -520,6 +520,42 @@ int depopulate_submodule(const char *path)
>         return 0;
>  }
>
> +int update_submodule(const char *path, const unsigned char sha1[20], int force)
> +{
> +       struct strbuf buf = STRBUF_INIT;
> +       struct child_process cp;
> +       const char *hex_sha1 = sha1_to_hex(sha1);
> +       const char *argv[] = {
> +               "checkout",
> +               force ? "-fq" : "-q",

respect "state->quiet" in checkout_entry() as well?

> +               hex_sha1,
> +               NULL,
> +       };
> +       const char *git_dir;
> +
> +       strbuf_addf(&buf, "%s/.git", path);
> +       git_dir = read_gitfile(buf.buf);
> +       if (!git_dir)
> +               git_dir = buf.buf;
> +       if (!is_directory(git_dir)) {
> +               strbuf_release(&buf);
> +               /* The submodule is not populated, so we can't check it out */
> +               return 0;
> +       }
> +       strbuf_release(&buf);
> +
> +       memset(&cp, 0, sizeof(cp));
> +       cp.argv = argv;
> +       cp.env = local_repo_env;
> +       cp.git_cmd = 1;
> +       cp.no_stdin = 1;
> +       cp.dir = path;   /* GIT_WORK_TREE doesn't work for git checkout */

And if we do respect --quiet and it's not specified, paths printed by
this process is relative to "dir", not to user cwd. Could be
confusing.

> +       if (run_command(&cp))
> +               return error("Could not checkout submodule %s", path);
> +
> +       return 0;
> +}
> +
-- 
Duy