Re: [PATCH 12/19] bash prompt: use bash builtins to check whether inside git dir

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

 



Johannes Sixt <j.sixt@xxxxxxxxxxxxx> writes:

>> __git_ps1() runs the '$(git rev-parse --is-inside-git-dir)' command
>> substitution to check whether we are inside a .git directory and the
>> bash prompt needs to be adjusted accordingly (i.e. display 'BARE!' or
>> 'GIT_DIR!').  This imposes the overhead of fork()ing a subshell and
>> fork()+exec()ing a git process.
>> 
>> Perform this check by comparing the path to the repository and the
>> current directory using only bash builtins, thereby sparing all that
>> fork()+exec() overhead.
>
>> -	if [ "true" = "$(git rev-parse --is-inside-git-dir 2>/dev/null)" ]; then
>> +	local pwd_p
>> +	__git_pwd_p pwd_p
>> +	# inside .git dir?
>> +	if [ "$__git_dir" = "." -o \
>> +			"${pwd_p#$__git_dir}" != "$pwd_p" ]; then
>
> At this point, $__git_dir is c:/dir style, whereas $pwd_p is /c/dir style,
> and the intended prefix check does not trigger.
>
> As long as $__git_dir is only used to access files, it does not matter
> whether it is Windows style or POSIX style. But if $__git_dir is used in a
> comparison, then you must make 100% sure that the involved paths are of
> the same vintage.
>
> What would be lost if this patch were dropped?

One loss of fork/exec is what would be lost, I would guess, and that seems
to be the primary point of this entire series, so...

At the conceptual level, I think the optimization in this patch makes
sense, but if the assumed primary beneficiary (i.e. Windows) cannot
benefit from this particular optimization due to two different path
representations, it wouldn't help being conceptually sound X-<.
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[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]