threads / patch / 7072

patch, 2 partsHandle core.symlinks=false case in merge-recursive.

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

## tl;dr

7 messages between Mar 3, 2007 and Mar 4, 2007. Diffs are folded; open one to read it.

replies: 6people: 3as markdown or json

Johannes Sixt· Mar 3, 2007, 19:32 UTC · lore

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
Show changes to 2 files +64 −1

merge-recursive.c, 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
Johannes Sixt· Mar 3, 2007, 19:38 UTC · re: Johannes Sixt · lore

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

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(-)
Show changes to combine-diff.c +10 −0
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
Johannes Schindelin· Mar 3, 2007, 20:11 UTC · re: Johannes Sixt · lore

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

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

Johannes Sixt· Mar 3, 2007, 20:39 UTC · re: Johannes Schindelin · lore

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

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
Johannes Schindelin· Mar 3, 2007, 20:44 UTC · re: Johannes Sixt · lore

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

Hi,
On Sat, 3 Mar 2007, Johannes Sixt wrote:
Show 7 quoted lines
> 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

Johannes Sixt· Mar 3, 2007, 20:21 UTC · re: Johannes Sixt · lore

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

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
Junio C Hamano· Mar 4, 2007, 03:42 UTC · re: Johannes Sixt · lore

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

Johannes Sixt <johannes.sixt@telecom.at> writes:
Show 11 quoted lines
> 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.

← back to recent threads