"Victoria Dye via GitGitGadget" <gitgitgadget@xxxxxxxxx> writes: > Unlike other existing usage of 'get_dtype', the 'follow_symlinks' arg is set > to 1 to replicate the existing handling of symlink dirents. This > unfortunately requires calling 'stat' on the associated entry regardless of > platform, but symlinks in the loose ref store are highly unlikely since > they'd need to be created manually by a user. Yeek. I wonder what breaks if we do not do this follow_symlinks() part, i.e., either just replace stat() with lstat() in the original without any of these four patches (which would be simple to figure out what breaks), or omit [3/4] and let get_dtype() yield DT_LNK. It seems that it comes from a7e66ae3 ([PATCH] Make do_each_ref() follow symlinks., 2005-08-16), and just like I commented on there in its log message back then, I still doubt that following a symbolic link is a great idea here in this codepath. But optimization without behaviour change is a good way to ensure that optimization does not introduce new bugs, and because keeping the historical behaviour like the patches [3/4] and this patch does is more work (meaning: if it proves unnecessary to dereference symbolic links, we can remove code instead of having to write new code to support the new behaviour), let's take the series as-is, and defer it to future developers to further clean-up the semantics. > Note that this patch also changes the condition for skipping creation of a > ref entry from "when 'stat' fails" to "when the d_type is anything other > than DT_REG or DT_DIR". If a dirent's d_type is DT_UNKNOWN (either because > the platform doesn't support d_type in dirents or some other reason) or > DT_LNK, 'get_dtype' will try to derive the underlying type with 'stat'. If > the 'stat' fails, the d_type will remain 'DT_UNKNOWN' and dirent will be > skipped. However, it will also be skipped if it is any other valid d_type > (e.g. DT_FIFO for named pipes, DT_LNK for a nested symlink). Git does not > handle these properly anyway, so we can safely constrain accepted types to > directories and regular files. Sounds good. > Signed-off-by: Victoria Dye <vdye@xxxxxxxxxx> > --- > refs/files-backend.c | 14 +++++--------- > 1 file changed, 5 insertions(+), 9 deletions(-) Thanks.
