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

From: Jens Lehmann <jens.lehmann@web.de>
Date: Feb 7, 2014, 21:32 UTC
Message-ID: <52F550EC.20202@web.de>
In-Reply-To: <CACsJy8CiAPnatithenDKBBKVGFHQZsu4mJLEjuWFD2GXqO56Lw@mail.gmail.com>

Am 04.02.2014 01:11, schrieb Duy Nguyen:

Show 29 quoted lines

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

Show 18 quoted lines

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

Show 26 quoted lines

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

Show 6 quoted lines

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