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 protected]>
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 protected]>
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

Reply via email to