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

[Jeff King <peff@peff.net>]

On Tue, Jan 22, 2019 at 03:35:21PM +0100, Johannes Schindelin wrote:

> 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)?

That's quite a while ago, but I'm pretty sure I was just following
existing practice. It would be fine to switch from stat() to access().
But...

> 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?

I think access() will always dereference. So it would not detect a
dangling symlink. Whether that matters or not is going to depend on each
caller.

I doubt it would matter much either way in this case. And I don't think
this is performance critical (it should be once per checkout, not once
per file).

If there are callers that care (and I assume there are due to the
existence of a50f9fcfeb0), and if we do care about performance on
platforms where stat() is slower, it might be reasonable to have a
platform-specific implementation of file_exists().

Likewise, anybody converting from access() should consider whether each
site cares about dangling symlinks (though in general, I'd expect most
of them to simply not have thought of it, and be happy to start
considering that as "exists").

-Peff