Re: [PATCH 3/4] set_work_tree: use chdir_notify

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

 



On Thu, Mar 29, 2018 at 7:50 PM, Jeff King <peff@xxxxxxxx> wrote:
> On Thu, Mar 29, 2018 at 07:02:21PM +0200, Duy Nguyen wrote:
>
>> On Wed, Mar 28, 2018 at 7:42 PM, Jeff King <peff@xxxxxxxx> wrote:
>> > When we change to the top of the working tree, we manually
>> > re-adjust $GIT_DIR and call set_git_dir() again, in order to
>> > update any relative git-dir we'd compute earlier.
>>
>> Another way to approach this problem is not delaying chdir() at all.
>> We have to delay calling setup_work_tree() and not do it in
>> setup_git_directory() because it can die() when chdir() fails. But in
>> many cases, the command does not actually need the worktree and does
>> not deserve to die. But what if we make setup_work_tree be gentle?
>>
>> If it successfully chdir() at the end of setup_git_directory() (and
>> perhaps before the first set_git_dir call), great! The problem we're
>> dealing here vanishes. If it fails, don't die, just set a flag. Later
>> on when a command requests a worktree, we can check this flag and now
>> can die().
>>
>> It's less code this way, but it uses up more of your (or my) time
>> because even though the first set_git_dir() call actually happens at 8
>> places. Is it worth trying ?
>
> I do kind of like that. I'm reasonably happy with the chdir_notify()
> interface, but it would be nicer still if we could get rid of it in the
> first place. It's true that we _could_ chdir from other places, but

There's another place we do, that I should mention and keep
forgetting. Our run-command.c code allows to switch cwd, and if
$GIT_DIR and stuff is relative then we should reparent them too just
like we do with setup_work_tree(). Your chdir-notify makes it super
easy to support that, we just need to move the prep_childenv() down
below chdir(). But since nobody has complaint, I suppose that feature
is not really popular (or at least not used to launch another git
process anyway)

> realistically speaking, we do our one big chdir as part of the setup
> process.
-- 
Duy



[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