David Aguilar <davvid@xxxxxxxxx> writes: > Right. At first I thought I could revise the commit message to > make it clearer that we simply want to skip all symlinks, since > it never makes sense to reuse a worktree symlinks, but looking > at the tests and implementation makes me realize that it's not > that simple. > > This is going to take a bit more time to get right. John, I was > hoping you'd be able to take a look -- I'm playing catch-up too. > When it was first reported I let it sit for a while in hopes > that the original author would pickup the issue, but months > passed and I figured I'd take a stab at helping the user out. > > Anyways, it'll take me a bit more time to understand the code > and work out a sensible solution. My gut feeling is that we > should adjust the dir-diff feature so that it ignores all > symlinks. That seems like a simple answer since we're deciding > to skip that chunk of complexity. What dir-diff wants to do is to prepare two directory hierarchies on the filesystem and run "diff -r" (or an equivalent command of user's choice) on them, e.g. "diff -r left/ right/". "left/" tree is typically what you want to compare your working tree files agaist (e.g. a clean checkout of "the other version"), and "right/" tree is populated with either copies of the working tree or symbolic links. The copying to "right/" feels wasteful, but your working tree may be littered with build artifacts, and making a clean copy with only tracked files is one way to make sure that "diff -r" with a clean checout of "the other version" will not show them. In the loop that walks the @rawdiff array, there are a lot of "if we see a symbolic link on the left, do this" before the last one that says "if the working tree side is not $null (i.e. not missing), ask ut_wt_file()". That code remembers which path on either side had symbolic links. Later in the same function, there is this comment "Symbolic links require special treatment." The intent of the code here is that any path that involves a symbolic link should be tweaked there. The loop over %symlink expects left/ and right/ to be populated normally by the loop over @working_tree, and then for any path that is a symbolic link is replaced with a phony regular file (not a symbolic link) that says "Here is a symbolic link". So I think it is fine to return $use=0 for any symbolic link from use_wt_file. Anything you do there will be replaced by the loop over %symlink that appears later in the caller. The caller discards $wt_sha1 when $use=0 is returned, so the second return value does not matter. -- 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