Am 03.08.2014 um 19:19 schrieb Junio C Hamano:
René Scharfe <l.s.r@xxxxxx> writes:
How about the patch below? Before it checks if an index entry exists
in the work tree, it checks if its path includes a symlink.
Honestly, I didn't expect the fix to be in the refresh-index code
path, but doing this there sort of makes sense.
I found it through observation, not understanding. Just looked for
stat/lstat calls executed by git status for b/different and b/equal
using strace; debugging printfs told me where they came from.
And do we need to use the threaded_ variant of the function here?
Hmmm, this is a tangent, but you comment made me wonder if we also
need to adjust preload_thread() in preload-index.c somehow, but we
do not touch CE_UPTODATE there, so it probably is not necessary.
The function calls ce_mark_uptodate(), which does set CE_UPTODATE. It
calls threaded_has_symlink_leading_path() before lstat() already,
however. (Since f62ce3de: Make index preloading check the whole path to
the file.)
The caller of refresh_cache_ent() is walking an array of sorted
pathnames aka istate->cache[] in a single-threaded fashion, possibly
with a pathspec to limit the scan.
There are two direct callers (refresh_index(), refresh_cache_entry())
and several indirect ones. Do we have a way to detect unsynchronized
parallel access to the has_symlink_leading_path()-cache? Checking the
full callers-of-callers tree manually looks a bit scary to me.
Do you mean that we may want to
make istate->cache[] scanned by multiple threads? I am not sure.
No, I didn't want to suggest any performance improvements. I'm only
interested in correctness for now.
René
--
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