vim-patch:8.0.1469: when package path is a symlink 'runtimepath' is wrong

Problem:    When package path is a symlink adding it to 'runtimepath' happens
            at the end.
Solution:   Do not resolve symlinks before locating the position in
            'runtimepath'. (Ozaki Kiichi, closes vim/vim#2604)
2374faae11

src/nvim/ex_cmds2.c

@@ -2536,25 +2536,15 @@ static void source_all_matches(char_u *pat)
   }
 }
 
-// used for "cookie" of add_pack_plugin()
+/// Add the package directory to 'runtimepath'
-static int APP_ADD_DIR;
+static int add_pack_dir_to_rtp(char_u *fname)
-static int APP_LOAD;
-static int APP_BOTH;
-
-static void add_pack_plugin(char_u *fname, void *cookie)
 {
   char_u *p4, *p3, *p2, *p1, *p;
   char_u *buf = NULL;
+  char *afterdir = NULL;
+  int retval = FAIL;
 
-  char *const ffname = fix_fname((char *)fname);
+  p4 = p3 = p2 = p1 = get_past_head(fname);
-
-  if (ffname == NULL) {
-    return;
-  }
-
-  if (cookie != &APP_LOAD && strstr((char *)p_rtp, ffname) == NULL) {
-    // directory is not yet in 'runtimepath', add it
-    p4 = p3 = p2 = p1 = get_past_head((char_u *)ffname);
   for (p = p1; *p; MB_PTR_ADV(p)) {
     if (vim_ispathsep_nocolon(*p)) {
       p4 = p3; p3 = p2; p2 = p1; p1 = p;
@@ -2566,8 +2556,15 @@ static void add_pack_plugin(char_u *fname, void *cookie)
   //    p4   p3   p2   p1
   //
   // find the part up to "pack" in 'runtimepath'
+  p4++;  // append pathsep in order to expand symlink
   char_u c = *p4;
   *p4 = NUL;
+  char *const ffname = fix_fname((char *)fname);
+  *p4 = c;
+
+  if (ffname == NULL) {
+    return FAIL;
+  }
 
   // Find "ffname" in "p_rtp", ignoring '/' vs '\' differences
   size_t fname_len = strlen(ffname);
@@ -2597,17 +2594,16 @@ static void add_pack_plugin(char_u *fname, void *cookie)
     // append after the matching directory.
     insp--;
   }
-    *p4 = c;
 
   // check if rtp/pack/name/start/name/after exists
-    char *afterdir = concat_fnames(ffname, "after", true);
+  afterdir = concat_fnames((char *)fname, "after", true);
   size_t afterlen = 0;
   if (os_isdir((char_u *)afterdir)) {
     afterlen = strlen(afterdir) + 1;  // add one for comma
   }
 
   const size_t oldlen = STRLEN(p_rtp);
-    const size_t addlen = strlen(ffname) + 1;  // add one for comma
+  const size_t addlen = STRLEN(fname) + 1;  // add one for comma
   const size_t new_rtp_len = oldlen + addlen + afterlen + 1;
   // add one for NUL -------------------------------------^
   char *const new_rtp = try_malloc(new_rtp_len);
@@ -2619,7 +2615,7 @@ static void add_pack_plugin(char_u *fname, void *cookie)
   memmove(new_rtp, p_rtp, keep);
   new_rtp_fill += keep;
   new_rtp[new_rtp_fill++] = ',';
-    memmove(new_rtp + new_rtp_fill, ffname, addlen);
+  memmove(new_rtp + new_rtp_fill, fname, addlen);
   new_rtp_fill += addlen - 1;
   assert(new_rtp[new_rtp_fill] == NUL || new_rtp[new_rtp_fill] == ',');
   if (p_rtp[keep] != NUL) {
@@ -2635,13 +2631,23 @@ static void add_pack_plugin(char_u *fname, void *cookie)
   new_rtp[new_rtp_fill] = NUL;
   set_option_value("rtp", 0L, new_rtp, 0);
   xfree(new_rtp);
+  retval = OK;
+
+theend:
+  xfree(buf);
+  xfree(ffname);
   xfree(afterdir);
+  return retval;
 }
 
-  if (cookie != &APP_ADD_DIR) {
+/// Load scripts in "plugin" and "ftdetect" directories of the package.
+static int load_pack_plugin(char_u *fname)
+{
   static const char *plugpat = "%s/plugin/**/*.vim";  // NOLINT
   static const char *ftpat = "%s/ftdetect/*.vim";  // NOLINT
 
+  int retval = FAIL;
+  char *const ffname = fix_fname((char *)fname);
   size_t len = strlen(ffname) + STRLEN(ftpat);
   char_u *pat = try_malloc(len + 1);
   if (pat == NULL) {
@@ -2662,11 +2668,31 @@ static void add_pack_plugin(char_u *fname, void *cookie)
   }
   xfree(cmd);
   xfree(pat);
-  }
+  retval = OK;
 
 theend:
-  xfree(buf);
   xfree(ffname);
+
+  return retval;
+}
+
+// used for "cookie" of add_pack_plugin()
+static int APP_ADD_DIR;
+static int APP_LOAD;
+static int APP_BOTH;
+
+static void add_pack_plugin(char_u *fname, void *cookie)
+{
+  if (cookie != &APP_LOAD && strstr((char *)p_rtp, (char *)fname) == NULL) {
+    // directory is not yet in 'runtimepath', add it
+    if (add_pack_dir_to_rtp(fname) == FAIL) {
+      return;
+    }
+  }
+
+  if (cookie != &APP_ADD_DIR) {
+    load_pack_plugin(fname);
+  }
 }
 
 /// Add all packages in the "start" directory to 'runtimepath'.

test/functional/legacy/packadd_spec.lua

@@ -14,6 +14,10 @@ describe('packadd', function()
     clear()
 
     source([=[
+      func Escape(s)
+        return escape(a:s, '\~')
+      endfunc
+
       func SetUp()
         let s:topdir = expand(getcwd() . '/Xdir')
         exe 'set packpath=' . s:topdir
@@ -50,8 +54,8 @@ describe('packadd', function()
         call assert_equal(77, g:plugin_also_works)
         call assert_true(17, g:ftdetect_works)
         call assert_true(len(&rtp) > len(rtp))
-        call assert_true(&rtp =~ (escape(s:plugdir, '\') . '\($\|,\)'))
+        call assert_match(Escape(s:plugdir) . '\($\|,\)', &rtp)
-        call assert_true(&rtp =~ escape(expand(s:plugdir . '/after$'), '\'))
+        call assert_match(Escape(expand(s:plugdir . '/after$')), &rtp)
 
         " Check exception
         call assert_fails("packadd directorynotfound", 'E919:')
@@ -73,7 +77,7 @@ describe('packadd', function()
 
         call assert_equal(24, g:plugin_works)
         call assert_true(len(&rtp) > len(rtp))
-        call assert_true(&rtp =~ (escape(plugdir, '\') . '\($\|,\)'))
+        call assert_match(Escape(plugdir) . '\($\|,\)', &rtp)
       endfunc
 
       func Test_packadd_noload()
@@ -90,7 +94,7 @@ describe('packadd', function()
         packadd! mytest
 
         call assert_true(len(&rtp) > len(rtp))
-        call assert_true(&rtp =~ (escape(s:plugdir, '\') . '\($\|,\)'))
+        call assert_match(Escape(s:plugdir) . '\($\|,\)', &rtp)
         call assert_equal(0, g:plugin_works)
 
         " check the path is not added twice
@@ -122,7 +126,7 @@ describe('packadd', function()
         packadd mytest
 
         " Must have been inserted in the middle, not at the end
-        call assert_true(&rtp =~ escape(expand('/pack/mine/opt/mytest').',', '\'))
+        call assert_match(Escape(expand('/pack/mine/opt/mytest').','), &rtp)
         call assert_equal(44, g:plugin_works)
 
         " No change when doing it again.
@@ -135,6 +139,48 @@ describe('packadd', function()
         exec "silent !" (has('win32') ? "rd /q/s" : "rm") top2_dir
       endfunc
 
+      func Test_packadd_symlink_dir2()
+        let top2_dir = expand(s:topdir . '/Xdir2')
+        let real_dir = expand(s:topdir . '/Xsym/pack')
+        call mkdir(top2_dir, 'p')
+        call mkdir(real_dir, 'p')
+        let &rtp = top2_dir . ',' . top2_dir . '/after'
+        let &packpath = &rtp
+
+        if has('win32')
+          exec "silent! !mklink /d" top2_dir "Xsym"
+        else
+          exec "silent !ln -s ../Xsym/pack"  top2_dir . '/pack'
+        endif
+        let s:plugdir = expand(top2_dir . '/pack/mine/opt/mytest')
+        call mkdir(s:plugdir . '/plugin', 'p')
+
+        exe 'split ' . s:plugdir . '/plugin/test.vim'
+        call setline(1, 'let g:plugin_works = 48')
+        wq
+        let g:plugin_works = 0
+
+        packadd mytest
+
+        " Must have been inserted in the middle, not at the end
+        call assert_match(Escape(expand('/Xdir2/pack/mine/opt/mytest').','), &rtp)
+        call assert_equal(48, g:plugin_works)
+
+        " No change when doing it again.
+        let rtp_before = &rtp
+        packadd mytest
+        call assert_equal(rtp_before, &rtp)
+
+        set rtp&
+        let rtp = &rtp
+        if has('win32')
+          exec "silent !rd /q/s" top2_dir
+        else
+          exec "silent !rm" top2_dir . '/pack'
+          exec "silent !rmdir" top2_dir
+        endif
+      endfunc
+
       func Test_packloadall()
         " plugin foo with an autoload directory
         let fooplugindir = &packpath . '/pack/mine/start/foo/plugin'
@@ -190,9 +236,9 @@ describe('packadd', function()
         helptags ALL
 
         let tags1 = readfile(docdir1 . '/tags')
-        call assert_true(tags1[0] =~ 'look-here')
+        call assert_match('look-here', tags1[0])
         let tags2 = readfile(docdir2 . '/tags')
-        call assert_true(tags2[0] =~ 'look-away')
+        call assert_match('look-away', tags2[0])
       endfunc
 
       func Test_colorscheme()