Re: What's cooking in git.git (Oct 2013, #03; Wed, 16)

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

Am 18.10.2013 21:09, schrieb Junio C Hamano:
> Karsten Blees <karsten.blees@gmail.com> writes:
>> Can't we just use add_file_to_cache here (which replaces
>> cache_entries by creating a copy)?
>>
>> diff --git a/submodule.c b/submodule.c
>> index 1905d75..e388487 100644
>> --- a/submodule.c
>> +++ b/submodule.c
>> @@ -116,30 +116,7 @@ int remove_path_from_gitmodules(const char *path)
>>  
>>  void stage_updated_gitmodules(void)
>>  {
>> -       struct strbuf buf = STRBUF_INIT;
>> -       struct stat st;
>> -       int pos;
>> -       struct cache_entry *ce;
>> -       int namelen = strlen(".gitmodules");
>> -
>> -       pos = cache_name_pos(".gitmodules", namelen);
>> -       if (pos < 0) {
>> -               warning(_("could not find .gitmodules in index"));
>> -               return;
>> -       }
> 
> I think the remainder is (morally) equivalent between the original
> and a single "add-file-to-cache" call, and the version after your
> "how about this" patch in the message I am responding to looks more
> correct (e.g. why does the original lstat after it has read the
> file?).

Cargo cult programming. I was looking at other code manipulating
the index (as Documentation/technical/api-in-core-index.txt is
rather terse ;-) and concluded I would need to read the possibly
updated st.st_mode, in case updating the config file would have
changed that.

> But this warning may want to stay, no?

Of course you are right on this one. All test ran successfully
with this patch, so I think adding one for that warning makes
sense too. And as that is submodule related stuff I volunteer
for fixing all this ;-)

>> -       ce = active_cache[pos];
>> -       ce->ce_flags = namelen;
>> -       if (strbuf_read_file(&buf, ".gitmodules", 0) < 0)
>> -               die(_("reading updated .gitmodules failed"));
>> -       if (lstat(".gitmodules", &st) < 0)
>> -               die_errno(_("unable to stat updated .gitmodules"));
>> -       fill_stat_cache_info(ce, &st);
>> -       ce->ce_mode = ce_mode_from_stat(ce, st.st_mode);
>> -       if (remove_cache_entry_at(pos) < 0)
>> -               die(_("unable to remove .gitmodules from index"));
>> -       if (write_sha1_file(buf.buf, buf.len, blob_type, ce->sha1))
>> -               die(_("adding updated .gitmodules failed"));
>> -       if (add_cache_entry(ce, ADD_CACHE_OK_TO_ADD|ADD_CACHE_OK_TO_REPLACE))
>> +       if (add_file_to_cache(".gitmodules", 0))
>>                 die(_("staging updated .gitmodules failed"));