Re: [PATCH v2 2/3] init: do not set core.worktree more often than necessary

From: Duy Nguyen <pclouds@gmail.com>
Date: Sep 22, 2016, 10:06 UTC
Message-ID: <CACsJy8BQvo_XSXZ3m8=A=0tik3qPd8k9cyC4G408JK+AhtYStQ@mail.gmail.com>
In-Reply-To: <xmqqd1jx854z.fsf@gitster.mtv.corp.google.com>

On Thu, Sep 22, 2016 at 1:44 AM, Junio C Hamano <gitster@pobox.com> wrote:

Show 20 quoted lines

>> @@ -314,6 +315,8 @@ static void create_object_directory(void)
>>  int set_git_dir_init(const char *git_dir, const char *real_git_dir,
>>                    int exist_ok)
>>  {
>> +     original_git_dir = xstrdup(real_path(git_dir));
>> +
>>       if (real_git_dir) {
>>               struct stat st;
>
> The function being extern bothers me.  The create_default_files()
> function, which is the only thing consumes this variable, is called
> only from init_db(), and I'd prefer to see some way to guarantee
> that everybody who calls init_db() calls set_git_dir_init()
> beforehand.  Right now, cmd_init_db() and cmd_clone() are the only
> ones that call init_db() and they both call set_dir_git_init(); if a
> new caller starts calling init_db() and forgets to call the other
> one, that caller will be buggy.
>
> Perhaps a comment before init_db() to tell callers to always call
> the other one is the least thing necessary?

Good thinking. We could go a step further, baking it as assert() to catch new/incorrect call sequences automatically.

Or we could combine the two functions init_db() and set_git_dir_init() into one. I prefer this one, but having problem with finding a good name for it because the new function would prepare $GIT_DIR for the entire process and init the repository. Maybe enter_and_init_db(), enter_and_init_repo()? If no good name is found, I'll go back to either adding comment or assert().

-- 
Duy