From 955afeb419ed02a078b45312949767692751274c Mon Sep 17 00:00:00 2001
From: Karen Arutyunov <karen@codesynthesis.com>
Date: Sun, 15 Mar 2020 17:02:22 +0300
Subject: Fix ln builtin not to complete relative target against working
 directory

Now it preserves the relative path when creates a symlink and completes it
against the link directory when creates a hardlink or a copy.
---
 libbutl/builtin.cxx           | 50 +++++++++++++++++++++++++++----------------
 tests/builtin/ln.testscript   |  6 +++---
 tests/dir-iterator/testscript |  2 +-
 tests/mventry/testscript      |  2 +-
 tests/wildcard/testscript     |  2 +-
 5 files changed, 38 insertions(+), 24 deletions(-)

diff --git a/libbutl/builtin.cxx b/libbutl/builtin.cxx
index a041820..f1b6b63 100644
--- a/libbutl/builtin.cxx
+++ b/libbutl/builtin.cxx
@@ -242,16 +242,17 @@ namespace butl
     return ops;
   }
 
-  // Parse and normalize a path. Also, unless it is already absolute, make the
-  // path absolute using the specified directory (must be an absolute path).
-  // Fail if the path is empty, and on parsing and normalization errors.
+  // Parse and normalize a path. Also make a relative path absolute using the
+  // specified directory path if it is not empty (in which case it must be
+  // absolute). Fail if the path is empty, and on parsing and normalization
+  // errors.
   //
   static path
   parse_path (string s,
               const dir_path& d,
               const function<error_record ()>& fail)
   {
-    assert (d.absolute ());
+    assert (d.empty () || d.absolute ());
 
     try
     {
@@ -260,7 +261,7 @@ namespace butl
       if (p.empty ())
         throw invalid_path ("");
 
-      if (p.relative ())
+      if (p.relative () && !d.empty ())
         p = d / move (p);
 
       p.normalize ();
@@ -775,26 +776,31 @@ namespace butl
              const builtin_callbacks& cbs,
              const function<error_record ()>& fail)
   {
-    assert (target.absolute () && target.normalized ());
     assert (link.absolute () && link.normalized ());
 
-    // Determine the target type, fail if the target doesn't exist.
+    // Determine the target type, fail if the target doesn't exist. Note that
+    // to do that we need to complete a relative target path against the link
+    // directory making the target path absolute.
     //
+    const path& atp (target.relative ()
+                     ? link.directory () / target
+                     : target);
+
     bool dir (false);
 
     try
     {
-      pair<bool, entry_stat> pe (path_entry (target));
+      pair<bool, entry_stat> pe (path_entry (atp));
 
       if (!pe.first)
-        fail () << "unable to create symlink to '" << target << "': no such "
+        fail () << "unable to create symlink to '" << atp << "': no such "
                 << "file or directory";
 
       dir = pe.second.type == entry_type::directory;
     }
     catch (const system_error& e)
     {
-      fail () << "unable to stat '" << target << "': " << e;
+      fail () << "unable to stat '" << atp << "': " << e;
     }
 
     // First we try to create a symlink. If that fails (e.g., "Windows
@@ -824,11 +830,14 @@ namespace butl
             (c == ENOSYS || // Not implemented.
              c == EPERM)))  // Not supported by the filesystem(s).
         fail () << "unable to create symlink '" << link << "' to '"
-                << target << "': " << e;
+                << atp << "': " << e;
 
+      // Note that for hardlinking/copying we need to use the complete
+      // (absolute) target path.
+      //
       try
       {
-        mkhardlink (target, link, dir);
+        mkhardlink (atp, link, dir);
 
         if (cbs.create)
           call (fail, cbs.create, link, false /* pre */);
@@ -840,16 +849,16 @@ namespace butl
               (c == ENOSYS || // Not implemented.
                c == EPERM  || // Not supported by the filesystem(s).
                c == EXDEV)))  // On different filesystems.
-          fail () << "unable to create hardlink '" << link << "' to '"
-                  << target << "': " << e;
+          fail () << "unable to create hardlink '" << link << "' to '" << atp
+                  << "': " << e;
 
         if (dir)
-          cpdir (path_cast<dir_path> (target), path_cast<dir_path> (link),
+          cpdir (path_cast<dir_path> (atp), path_cast<dir_path> (link),
                  false /* attrs */,
                  cbs,
                  fail);
         else
-          cpfile (target, link,
+          cpfile (atp, link,
                   false /* overwrite */,
                   true /* attrs */,
                   cbs,
@@ -924,7 +933,9 @@ namespace butl
       //
       if (!link.to_directory ())
       {
-        path target (parse_path (move (*i++), wd, fail));
+        // Don't complete a relative target and pass it to mksymlink() as is.
+        //
+        path target (parse_path (move (*i++), dir_path (), fail));
 
         // If there are multiple targets but no trailing separator for the
         // link, then, most likelly, it is missing.
@@ -940,7 +951,10 @@ namespace butl
       {
         for (; i != e; ++i)
         {
-          path target (parse_path (move (*i), wd, fail));
+          // Don't complete a relative target and pass it to mksymlink() as
+          // is.
+          //
+          path target (parse_path (move (*i), dir_path (), fail));
 
           // Synopsis 2: create a target path symlink in the specified
           // directory.
diff --git a/tests/builtin/ln.testscript b/tests/builtin/ln.testscript
index c12999f..051d0dc 100644
--- a/tests/builtin/ln.testscript
+++ b/tests/builtin/ln.testscript
@@ -112,7 +112,7 @@ $* -u >'option -u' 2>"ln: unknown option '-u'" == 1
     {
       touch a;
 
-      $* -s a b/c >>/~%EOO% 2>>/~%EOE% != 0
+      $* -s $~/a b/c >>/~%EOO% 2>>/~%EOE% != 0
         %create .+/b/c true%
         EOO
         %(
@@ -185,7 +185,7 @@ $* -u >'option -u' 2>"ln: unknown option '-u'" == 1
     {
       mkdir a;
 
-      $* -s a b/c >>/~%EOO% 2>>/~%EOE% != 0
+      $* -s $~/a b/c >>/~%EOO% 2>>/~%EOE% != 0
         %create .+/b/c true%
         EOO
         %(
@@ -205,7 +205,7 @@ $* -u >'option -u' 2>"ln: unknown option '-u'" == 1
   touch a;
   mkdir b c;
 
-  $* -s a b c/ >>/~%EOO% &c/a &c/b;
+  $* -s ../a ../b c/ >>/~%EOO% &c/a &c/b;
     %create .+/c/a true%
     %create .+/c/a false%
     %create .+/c/b true%
diff --git a/tests/dir-iterator/testscript b/tests/dir-iterator/testscript
index ec7338d..03ed164 100644
--- a/tests/dir-iterator/testscript
+++ b/tests/dir-iterator/testscript
@@ -24,7 +24,7 @@ $* a >"dir     b"
 if ($test.target == $build.host)
 {
   +if ($cxx.target.class != 'windows')
-    lnf = ln -s wd/t wd/l &wd/l
+    lnf = ^ln -s t wd/l &wd/l
     lnd = $lnf
   else
     echo 'yes' >=t
diff --git a/tests/mventry/testscript b/tests/mventry/testscript
index 54a3acc..61ef871 100644
--- a/tests/mventry/testscript
+++ b/tests/mventry/testscript
@@ -98,7 +98,7 @@
 if ($test.target == $build.host)
 {
   +if ($cxx.target.class != 'windows')
-    lnf = ln -s t l &l
+    lnf = ^ln -s t l &l
     lnd = $lnf
   else
     echo 'yes' >=t
diff --git a/tests/wildcard/testscript b/tests/wildcard/testscript
index 3590aa3..5f6a767 100644
--- a/tests/wildcard/testscript
+++ b/tests/wildcard/testscript
@@ -650,7 +650,7 @@
   {
     mkdir a;
     touch --no-cleanup a/b;
-    ln -s a/b a/l;
+    ^ln -s b a/l &a/l;
     rm a/b;
 
     touch a/c;
-- 
cgit v1.1