Re: [PATCH v5 1/5] t0060: Add test for manipulating symlinks via absolute paths

[Martin Erik Werner <martinerikwerner@gmail.com>]

On Mon, Feb 03, 2014 at 10:50:17AM -0800, Junio C Hamano wrote:
> Martin Erik Werner <martinerikwerner@gmail.com> writes:
> 
> > When symlinks in the working tree are manipulated using the absolute
> > path, git dereferences them, and tries to manipulate the link target
> > instead.
> 
> The above may a very good description of the root cause, but
> can we have description of a symptom that is visible by end-users
> that is caused by the bug being fixed with the series here, by
> ending the above like so:
> 
> 	... link target instead.  This causes "git foo bar" to
> 	misbehave in this and that way.
> 
> Testing the low-level underlying machinery like this patch does is
> not wrong per-se, but I suspect that this series was triggered by
> somebody noticing breakage in a end-user command "git foo $path"
> with $path being a full pathname to an in-tree symbolic link.  It
> wouldn't be like somebody who was bored and ran "test-path-utils"
> for fun noticed the root cause without realizing how the fix would
> affect the behaviour that would be visible to end-users, right?
> 
> Can we have that "git foo $path" to the testsuite as well?  That is
> the breakage we do not want to repeat in the future by regressing.
> 
> I am guessing "foo" is "add", but I wasn't closely following the
> progression of the series.
> 
> Thanks.

Indeed, it was first discovered via git-mv, (by Richard, using
git-annex) and me reproducing and reporting it was the start of the
thread: http://thread.gmane.org/gmane.comp.version-control.git/240467

In going further (PATCHv0):
> I've done a bit more digging into this: The issue applies to pretty
> much all commands which can be given paths to files which are present
> in the work tree, so add, cat-file, rev-list, etc.

At this stage I kind of dropped the reference to any specific top-level
command since it seemed to apply to all of them in some way, and I
figured it made more sense with a generic explanation that would apply
to all commands. But it might definitely be worth to mention it in order
for the commit messages to be less technical, and add at least one test
which would actually trigger it in a user-manner. So for the
explanation, something like that?:

	This causes for example 'git add /dir/repo/symlink' to attempt
	to add the target of the symlink rather than the symlink itself,
	which is usually not what the user intends to do.

Hmm, come to think of it, I even made some of those tests back before I
found it could be narrowly tested via prefix_path... I guess I'll pick
out the git-add one since it's the simplest, should that be added to
t0060-path-utils.sh as well, or would it fit better in t3700-add.sh?:

>From 910d8c9f51c3b3f2c03dbf15ce3cf7ea94de8d27 Mon Sep 17 00:00:00 2001
From: Martin Erik Werner <martinerikwerner@gmail.com>
Date: Thu, 16 Jan 2014 00:24:43 +0100
Subject: [PATCH] Add test for manipulating symlinks via absolute paths

When symlinks in the working tree are manipulated using the absolute
path, git derferences them, and tries to manipulate the link target
instead.

Add three known-breakage tests using add, mv, and rev-list which
checks this behaviour.

The failure of the git-add test is a regression introduced by 18e051a:
  setup: translate symlinks in filename when using absolute paths
(which did not take symlinks in the work tree into consideration).
---
 t/t0054-symlinks-via-abs-path.sh | 38 ++++++++++++++++++++++++++++++++++++++
 1 file changed, 38 insertions(+)
 create mode 100755 t/t0054-symlinks-via-abs-path.sh

diff --git a/t/t0054-symlinks-via-abs-path.sh b/t/t0054-symlinks-via-abs-path.sh
new file mode 100755
index 0000000..0b3c91e
--- /dev/null
+++ b/t/t0054-symlinks-via-abs-path.sh
@@ -0,0 +1,38 @@
+#!/bin/sh
+
+test_description='symlinks via sbsolute paths
+
+This test checks the behavior when symlinks in the working tree are manipulated
+via absolute path arguments.
+'
+. ./test-lib.sh
+
+test_expect_failure SYMLINKS 'git add symlink with absolute path' '
+
+	ln -s target symlink &&
+	git add "$(pwd)/symlink"
+
+'
+
+rm -f symlink
+
+test_expect_failure SYMLINKS 'git mv symlink with absolute path' '
+
+	ln -s target symlink &&
+	git add symlink &&
+	git mv "$(pwd)"/symlink moved
+
+'
+
+rm -f symlink moved
+
+test_expect_failure 'git rev-list symlink with absolute path' '
+
+	ln -s target symlink &&
+	git add symlink &&
+	git commit -m show &&
+	test "$(git rev-list HEAD -- symlink)" = "$(git rev-list HEAD -- $(pwd)/symlink)"
+
+'
+
+test_done
-- 
1.8.5.2