Subject: bug#14116: closed (Re: bug#14116: [PATCH] ln: allow to overwrite relative symlink)

From: [email-masked] (GNU bug Tracking System)
Date: Wed, 03 Apr 2013 17:48:02 +0000

Your bug report

#14116: [PATCH] ln: allow to overwrite relative symlink

which was filed against the coreutils package, has been closed.

The explanation is attached below, along with your original report.
If you require more details, please reply to [email-masked].

-- 
14116: http://debbugs.gnu.org/cgi/bugreport.cgi?bug=14116
GNU Bug Tracking System
Contact [email-masked] with problems

Subject: Re: bug#14116: [PATCH] ln: allow to overwrite relative symlink
From: Pádraig Brady <[email-masked]>
To: Rémy Lefevre <[email-masked]>
Cc: [email-masked]
Date: Wed, 03 Apr 2013 18:43:53 +0100
List-Id: <debbugs-submit.debbugs.gnu.org>

On 04/02/2013 11:30 AM, Rémy Lefevre wrote:
> I'm not sure to understand your first sentence. Resolving the last component is already the existing behavior, but maybe not the intended one.
> 
> Anyway, I agree that the path should be resolved without its last component.
> 
> I wrote a new patch for this. I hope that this one will not break anything.

It looks good.

I've attached the full patch in your name with a test and NEWS entry.
Also attached is a related doc change, detailing the symlink
resolution done with ln --relative.

thanks!
Pádraig.

>From 926cc41a37e89e34c17e7299cd70aea66f9ae280 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?R=C3=A9my=20Lefevre?= <[email-masked]>
Date: Tue, 2 Apr 2013 02:48:28 +0100
Subject: [PATCH 1/2] ln: --relative: fix updating of existing symlinks

Don't dereference an existing symlink being replaced.
I.E. generate the symlink relative to the symlink's containing dir,
rather than to some arbitrary place it points to.

* src/ln.c (convert_abs_rel): Don't consider the final component
of the symlink name when canonicalizing, as we to avoid
dreferencing the final component.
* tests/ln/relative.sh: Add a test case.
* NEWS: Mention the fix.
---
 NEWS                 |    4 ++++
 src/ln.c             |   14 ++++++++------
 tests/ln/relative.sh |    5 +++++
 3 files changed, 17 insertions(+), 6 deletions(-)

diff --git a/NEWS b/NEWS
index 0c2daad..b9fc6d2 100644
--- a/NEWS
+++ b/NEWS
@@ -9,6 +9,10 @@ GNU coreutils NEWS                                    -*- outline -*-
   permissions.
   [This bug was present in "the beginning".]
 
+  ln --relative now updates existing symlinks correctly.  Previously it based
+  the relative link on the dereferenced path of an existing link.
+  [This bug was introduced when --relative was added in coreutils-8.16.]
+
 ** New features
 
   join accepts a new option: --zero-terminated (-z). As with the sort,uniq
diff --git a/src/ln.c b/src/ln.c
index 1aa1473..2489b9a 100644
--- a/src/ln.c
+++ b/src/ln.c
@@ -132,22 +132,24 @@ target_directory_operand (char const *file)
 static char *
 convert_abs_rel (const char *from, const char *target)
 {
-  char *realtarget = canonicalize_filename_mode (target, CAN_MISSING);
+  /* Get dirname to generate paths relative to.  We don't resolve
+     the full TARGET as the last component could be an existing symlink.  */
+  char *targetdir = dir_name (target);
+
+  char *realdest = canonicalize_filename_mode (targetdir, CAN_MISSING);
   char *realfrom = canonicalize_filename_mode (from, CAN_MISSING);
 
   /* Write to a PATH_MAX buffer.  */
   char *relative_from = xmalloc (PATH_MAX);
 
-  /* Get dirname to generate paths relative to.  */
-  realtarget[dir_len (realtarget)] = '\0';
-
-  if (!relpath (realfrom, realtarget, relative_from, PATH_MAX))
+  if (!relpath (realfrom, realdest, relative_from, PATH_MAX))
     {
       free (relative_from);
       relative_from = NULL;
     }
 
-  free (realtarget);
+  free (targetdir);
+  free (realdest);
   free (realfrom);
 
   return relative_from ? relative_from : xstrdup (from);
diff --git a/tests/ln/relative.sh b/tests/ln/relative.sh
index 0418b8a..818da83 100755
--- a/tests/ln/relative.sh
+++ b/tests/ln/relative.sh
@@ -29,4 +29,9 @@ test $(readlink usr/bin/foo) = '../lib/foo/foo' || fail=1
 ln -sr usr/bin/foo usr/lib/foo/link-to-foo
 test $(readlink usr/lib/foo/link-to-foo) = 'foo' || fail=1
 
+# Correctly update an existing link, which was broken in <= 8.21
+ln -s dir1/dir2/f existing_link
+ln -srf here existing_link
+test $(readlink existing_link) = 'here' || fail=1
+
 Exit $fail
-- 
1.7.7.6

>From c5dc794a2723656b1943e7c9f52f91ea2b4b55fe Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?P=C3=A1draig=20Brady?= <[email-masked]>
Date: Wed, 3 Apr 2013 18:33:43 +0100
Subject: [PATCH 2/2] doc: add details on ln --relative symlink resolution

* doc/coreutils.texi (ln invocation): Describe how symlinks are
resolved with --relative, and give an example showing the greater
control available through realpath(1).
* tests/ln/relative.sh: Add a test to demonstrate full symlink
resolution, in a case where it might not be wanted.
---
 doc/coreutils.texi   |   18 +++++++++++++++++-
 tests/ln/relative.sh |   11 +++++++++++
 2 files changed, 28 insertions(+), 1 deletions(-)

diff --git a/doc/coreutils.texi b/doc/coreutils.texi
index dfa9b1c..4cfe4c5 100644
--- a/doc/coreutils.texi
+++ b/doc/coreutils.texi
@@ -9798,8 +9798,24 @@ ln -srv /a/file /tmp
 '/tmp/file' -> '../a/file'
 @end smallexample
 
+Relative symbolic links are generated based on their canonicalized
+containing directory, and canonicalized targets.  I.E. all symbolic
+links in these file names will be resolved.
 @xref{realpath invocation}, which gives greater control
-over relative file name generation.
+over relative file name generation, as demonstrated in the following example:
+
+@example
+@verbatim
+ln--relative() {
+  test "$1" = --no-symlinks && { nosym=$1; shift; }
+  target="$1";
+  test -d "$2" && link="$2/." || link="$2"
+  rtarget="$(realpath $nosym -m "$target" \
+              --relative-to "$(dirname "$link")")"
+  ln -s -v "$rtarget" "$link"
+}
+@end verbatim
+@end example
 
 @item -s
 @itemx --symbolic
diff --git a/tests/ln/relative.sh b/tests/ln/relative.sh
index 818da83..bbe7db7 100755
--- a/tests/ln/relative.sh
+++ b/tests/ln/relative.sh
@@ -34,4 +34,15 @@ ln -s dir1/dir2/f existing_link
 ln -srf here existing_link
 test $(readlink existing_link) = 'here' || fail=1
 
+# Demonstrate resolved symlinks used to generate relative links
+# so here,'web/latest' will not be linked to the intermediate 'latest' link.
+# You'd probably want to use realpath(1) in conjunction
+# with ln(1) without --relative to give greater control.
+ln -s release1 alpha
+ln -s release2 beta
+ln -s beta latest
+mkdir web
+ln -sr latest web/latest
+test $(readlink web/latest) = '../release2' || fail=1
+
 Exit $fail
-- 
1.7.7.6

Subject: [PATCH] ln: allow to overwrite relative symlink
From: Rémy Lefevre <[email-masked]>
To: [email-masked]
Date: Mon, 1 Apr 2013 16:40:59 +0200
List-Id: <debbugs-submit.debbugs.gnu.org>

Overwriting relative symlink leads to undesirable behavior. Consider the
following example:

# Create some directories
mkdir test
mkdir test/folder1
mkdir test/folder2

#Create some files
touch test/folder1/file1
touch test/folder1/file2

#Create a relative symlink in folder2 to file1
ln -sr test/folder1/file1 test/folder2/link

#Check the link
ls -l test/folder2/link
# Correctly output a link to ../folder1/file1

#Overwrite the symlink to point to file2
ln -sfr test/folder1/file2 test/folder2/link

#Check the link
ls -l test/folder2/link
# Wrongly output a link to file2 instead of ../folder1/file2

This undesirable behavior is due to a dereferencing of the target when the
relative path is computed. Passing CAN_NOLINKS flag to
canonicalize_filename_mode solves the problem.