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

[Jens Lehmann <Jens.Lehmann@web.de>]

Am 04.02.2014 01:11, schrieb Duy Nguyen:
> 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?

Good point.

>> 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?

See below.

>> +               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.

That's the reason I'm currently always passing -q to checkout. While
checkout would have to learn a "--prefix=" option to be able to print
the path relative to the superproject, some (most?) users don't want
to see this detailed information from inside the submodule. After all
git status and diff currently also only show a condensed view of the
submodule state and don't print any detailed information about files
inside the submodule. We might want to add means to enable that later,
and then we'd have to conditionally provide --quiet (and --prefix)
here.

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