Re: [PATCH v2 2/2] checkout: fix regression in checkout -b on intitial checkout

From: Junio C Hamano <gitster@pobox.com>
Date: Jan 22, 2019, 18:42 UTC
Message-ID: <xmqq8szch6u4.fsf@gitster-ct.c.googlers.com>
In-Reply-To: <nycvar.QRO.7.76.6.1901221529210.41@tvgsbejvaqbjf.bet>

Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:

Show 7 quoted lines

> I also looked at the implementation of `file_exists()` and found that it
> uses `lstat()`. Peff, you introduced this (using `stat()`) in c91f0d92efb3
> (git-commit.sh: convert run_status to a C builtin, 2006-09-08), could you
> enlighten me why you chose `stat()` over `access()` (the latter seems more
> light-weight to me)? Also, Junio, you changed it to use `lstat()` in
> a50f9fc5feb0 (file_exists(): dangling symlinks do exist, 2007-11-18), do
> you think we can/should use `access()` instead?

Given that the whole point of a50f9fc5 ("file_exists(): dangling symlinks do exist", 2007-11-18) is to make sure that we say "it exists" for a symbolic link that does not point anywhere, and that access(2) dereferences a symbolic link, changing it to use access(2) would change the meaning of the file_exists() function. So I do not think we _should_ use access(2).

I however do not know if we _can_ use access(2). It takes auditing the current callers and see if they truly care about knowing that a dangling symbolic link exists to determine if it is safe to do so. If none of them does, and if we do not have an immediate plan to add a new one that does, then we obviously can use it, but even in that case we'd need a comment in *.h to warn about it.