[PATCH 1/2] Handle core.symlinks=false case in merge-recursive.

If the file system does not support symbolic links (core.symlinks=false),
merge-recursive must write the merged symbolic link text into a regular
file.

While we are here, fix a tiny memory leak in the if-branch that writes
real symbolic links.

Signed-off-by: Johannes Sixt <redacted>
---
 merge-recursive.c         |    3 +-
 t/t6025-merge-symlinks.sh |   62 +++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 64 insertions(+), 1 deletions(-)
 create mode 100644 t/t6025-merge-symlinks.sh

diff --git a/merge-recursive.c b/merge-recursive.c
index 397a7ad..f8be72e 100644
--- a/merge-recursive.c
+++ b/merge-recursive.c

@@ -570,7 +570,7 @@ static void update_file_flags(const unsigned char *sha,
 		if (strcmp(type, blob_type) != 0)
 			die("blob expected for %s '%s'", sha1_to_hex(sha), path);
 
-		if (S_ISREG(mode)) {
+		if (S_ISREG(mode) || (!has_symlinks && S_ISLNK(mode))) {
 			int fd;
 			if (mkdir_p(path, 0777))
 				die("failed to create path %s: %s", path, strerror(errno));

@@ -591,6 +591,7 @@ static void update_file_flags(const unsigned char *sha,
 			mkdir_p(path, 0777);
 			unlink(path);
 			symlink(lnk, path);
+			free(lnk);
 		} else
 			die("do not know what to do with %06o %s '%s'",
 			    mode, sha1_to_hex(sha), path);

diff --git a/t/t6025-merge-symlinks.sh b/t/t6025-merge-symlinks.sh
new file mode 100644
index 0000000..3c1a697
--- /dev/null
+++ b/t/t6025-merge-symlinks.sh

@@ -0,0 +1,62 @@
+#!/bin/sh
+#
+# Copyright (c) 2007 Johannes Sixt
+#
+
+test_description='merging symlinks on filesystem w/o symlink support.
+
+This tests that git-merge-recursive writes merge results as plain files
+if core.symlinks is false.'
+
+. ./test-lib.sh
+
+test_expect_success \
+'setup' '
+git-config core.symlinks false &&
+> file &&
+git-add file &&
+git-commit -m initial &&
+git-branch b-symlink &&
+git-branch b-file &&
+l=$(echo -n file | git-hash-object -t blob -w --stdin) &&
+echo "120000 $l	symlink" | git-update-index --index-info &&
+git-commit -m master &&
+git-checkout b-symlink &&
+l=$(echo -n file-different | git-hash-object -t blob -w --stdin) &&
+echo "120000 $l	symlink" | git-update-index --index-info &&
+git-commit -m b-symlink &&
+git-checkout b-file &&
+echo plain-file > symlink &&
+git-add symlink &&
+git-commit -m b-file'
+
+test_expect_failure \
+'merge master into b-symlink, which has a different symbolic link' '
+! git-checkout b-symlink ||
+git-merge master'
+
+test_expect_success \
+'the merge result must be a file' '
+test -f symlink'
+
+test_expect_failure \
+'merge master into b-file, which has a file instead of a symbolic link' '
+! (git-reset --hard &&
+git-checkout b-file) ||
+git-merge master'
+
+test_expect_success \
+'the merge result must be a file' '
+test -f symlink'
+
+test_expect_failure \
+'merge b-file, which has a file instead of a symbolic link, into master' '
+! (git-reset --hard &&
+git-checkout master) ||
+git-merge b-file'
+
+test_expect_success \
+'the merge result must be a file' '
+test -f symlink'
+
+test_done

-- 
1.5.0.2.4.gdd4e4-dirty

When core.symlinks is false, and a merge of symbolic links had conflicts,
the merge result is left as a file in the working directory. A decision
must be made whether the file is treated as a regular file or as a
symbolic link. This patch treats the file as a symbolic link only if
all merge parents were also symbolic links.

Signed-off-by: Johannes Sixt <redacted>
---

I'm not quite sure whether this patch is worth it. The only thing it seems
to do is to avoid the mode change line (this is after a merge where 'symlink'
had a conflict):

without the patch:

  $ git diff
  diff --cc symlink
  index 1a010b1,30d67d4..0000000
  mode 120000,120000..100644
  --- a/symlink
  +++ b/symlink

with the patch:

  $ git diff
  diff --cc symlink
  index 1a010b1,30d67d4..0000000
  --- a/symlink
  +++ b/symlink


 combine-diff.c |   10 ++++++++++
 1 files changed, 10 insertions(+), 0 deletions(-)

diff --git a/combine-diff.c b/combine-diff.c
index 044633d..e6e3969 100644
--- a/combine-diff.c
+++ b/combine-diff.c

@@ -699,8 +699,18 @@ static void show_patch_diff(struct combine_diff_path *elem, int num_parent,
 			 !fstat(fd, &st)) {
 			size_t len = st.st_size;
 			size_t sz = 0;
+			int is_file, i;
 
 			elem->mode = canon_mode(st.st_mode);
+			/* if symlinks don't work, assume symlink if all parents
+			 * are symlinks
+			 */
+			is_file = has_symlinks;
+			for (i = 0; !is_file && i < num_parent; i++)
+				is_file = !S_ISLNK(elem->parent[i].mode);
+			if (!is_file)
+				elem->mode = canon_mode(S_IFLNK);
+
 			result_size = len;
 			result = xmalloc(len + 1);
 			while (sz < len) {

-- 
1.5.0.2.4.gdd4e4-dirty

Hi,

On Sat, 3 Mar 2007, Johannes Sixt wrote:

If the file system does not support symbolic links 
(core.symlinks=false), merge-recursive must write the merged symbolic 
link text into a regular file.

I think regardless of the value of core.symlinks, merging symbolic links 
does not make sense at all.

I'd suggest having two versions of the symlöink/file, <name>Ã~ours and 
<name>~theirs instead.

Ciao,
Dscho

On Saturday 03 March 2007 20:32, Johannes Sixt wrote:

If the file system does not support symbolic links (core.symlinks=false),
merge-recursive must write the merged symbolic link text into a regular
file.

But how to resolve such a conflict if core.symlinks=false?

It turns out that git-add cannot honor the symlink property recorded in the 
index because read_cache.c:add_file_to_index() will find only entries at 
stage 0, but the conflicting entries are at stages 2 and 3. Can there be 
something done about that?

-- Hannes

On Saturday 03 March 2007 21:11, Johannes Schindelin wrote:

I think regardless of the value of core.symlinks, merging symbolic links
does not make sense at all.

No doubt about that. Currently, the version of the "current" branch remains in 
the working tree. My patch does not change this behavior at all, it just does 
not call symlink(2), but allocates a regular file.

-- Hannes

Hi,

On Sat, 3 Mar 2007, Johannes Sixt wrote:

On Saturday 03 March 2007 21:11, Johannes Schindelin wrote:

quoted

I think regardless of the value of core.symlinks, merging symbolic 
links does not make sense at all.

No doubt about that. Currently, the version of the "current" branch 
remains in the working tree. My patch does not change this behavior at 
all, it just does not call symlink(2), but allocates a regular file.

Oh, I misunderstood! All is well, then. (I had the impression you put 
conflict markers into the file.)

Ciao,
Dscho