Re: [PATCH v2 2/9] setup: introduce startup_info->original_cwd

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 




On 11/29/2021 12:18 PM, Elijah Newren wrote:
> On Mon, Nov 29, 2021 at 6:05 AM Derrick Stolee <stolee@xxxxxxxxx> wrote:
>>
>> On 11/25/2021 3:39 AM, Elijah Newren via GitGitGadget wrote:
>>> From: Elijah Newren <newren@xxxxxxxxx>
>>
>>> @@ -866,6 +866,8 @@ int cmd_main(int argc, const char **argv)
>>>
>>>       trace_command_performance(argv);
>>>
>>> +     startup_info->original_cwd = xgetcwd();
>>> +
>>
>> I see this initial assignment in cmd_main()...
> 
> It looks like you accidentally responded to v2 when there's a v3
> (something I occasionally do too).  v3 changes this to put it in
> common-main instead of here, as suggested by Ævar, but to answer the
> question...

Yes, sorry about that. My inbox was delayed in showing me that a v3
existed until I was halfway through reviewing v2. (It then only
showed me half of the patches from v3, so something was causing
a delay.)

>>> +static void setup_original_cwd(void)
>>> +{
>>> +     struct strbuf tmp = STRBUF_INIT;
>>> +     const char *worktree = NULL;
>>> +     int offset = -1;
>>> +
>>> +     /*
>>> +      * startup_info->original_cwd wass set early on in cmd_main(), unless
>>> +      * we're an auxiliary tool like git-remote-http or test-tool.
>>> +      */
>>> +     if (!startup_info->original_cwd)
>>> +             return;
>>
>> ...which is assumed to be run before this method was called...
>>
>>> @@ -1330,6 +1378,7 @@ const char *setup_git_directory_gently(int *nongit_ok)
>>>               setenv(GIT_PREFIX_ENVIRONMENT, "", 1);
>>>       }
>>>
>>> +     setup_original_cwd();
>>
>> ...here in setup_git_directory_gently().
>>
>> Why do we need that assignment in cmd_main()? Could we instead
>> let setup_original_cwd() do the initial assignment? Or is it
>> possible that a chdir has happened already before this point?
> 
> In v1, I made that mistake.  Then I realized that when users pass the
> -C option to git, there is a chdir() call immediately upon parsing of
> the -C option.  So I had to move the strbuf_getcwd() call earlier.

Ok. I wonder if we could setup_original_cwd() earlier than that
parsing, but I'm sure you've already explored that option.

Thanks,
-Stolee



[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]

  Powered by Linux