Re: [PATCH] Make locked paths absolute when current directory is changed

[Duy Nguyen <pclouds@gmail.com>]

On Sat, Jul 19, 2014 at 12:47 AM, Junio C Hamano <gitster@pobox.com> wrote:
>> +             abspath = absolute_path(lk->filename);
>> +             if (strlen(abspath) >= sizeof(lk->filename))
>> +                     warning("locked path %s is relative when current directory "
>> +                             "is changed", lk->filename);
>
> Shouldn't this be a die() or an error return (which will kill the
> caller anyway)?

We don't know for sure there will be a die() or something to trigger
the roll back (or commit). If the chdir() is temporary, absolute path
not fitting in PATH_MAX chars is not fatal because cwd will be
reverted before commit/rollback. A better solution is probably avoid
PATH_MAX in lk->filename. But yeah, changing it to die() is safer
(especially when cwd is moved permanently for some options in
update-index and read-tree)

>> @@ -636,6 +636,7 @@ static const char *setup_git_directory_gently_1(int *nongit_ok)
>>               die_errno("Unable to read current working directory");
>>       offset = len = strlen(cwd);
>>
>> +     make_locked_paths_absolute();
>
> Just being curious, but this early in the start-up sequence, what
> files do we have locks on?

We don't know. For most builtin commands, the setup is done early and
we can be sure of no locks. Some commands (especially non-builtin) can
still delay calling setup_git_directory() until later and they might
do something in between, so better be safe than sorry.
-- 
Duy