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

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

[Johannes Sixt]

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 <johannes.sixt@telecom.at>
---
 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

[PATCH 2/2] Tell multi-parent diff about core.symlinks.

[Johannes Sixt]

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 <johannes.sixt@telecom.at>
---

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

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

[Johannes Schindelin]

[-- Attachment #1: Type: TEXT/PLAIN, Size: 420 bytes --]

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

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

[Johannes Sixt]

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

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

[Johannes Schindelin]

Hi,

On Sat, 3 Mar 2007, Johannes Sixt wrote:

> 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.

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

Ciao,
Dscho

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

[Johannes Sixt]

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

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

[Junio C Hamano]

Johannes Sixt <johannes.sixt@telecom.at> writes:

> 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?

You can always use --index-info.