patch 8.0.1469: when package path is a symlink 'runtimepath' is wrongv8.0.1469

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 #2604)

3 files changed, 178 insertions, 112 deletions

diff --git a/src/ex_cmds2.c b/src/ex_cmds2.c
index 1475ef25db..73fe0194be 100644
--- a/src/ex_cmds2.c
+++ b/src/ex_cmds2.c
@@ -3567,13 +3567,11 @@ source_all_matches(char_u *pat)
     }
 }
 
-/* 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)
+/*
+ * Add the package directory to 'runtimepath'.
+ */
+    static int
+add_pack_dir_to_rtp(char_u *fname)
 {
     char_u  *p4, *p3, *p2, *p1, *p;
     char_u  *insp;
@@ -3582,125 +3580,154 @@ add_pack_plugin(char_u *fname, void *cookie)
     int	    keep;
     size_t  oldlen;
     size_t  addlen;
-    char_u  *afterdir;
+    char_u  *afterdir = NULL;
     size_t  afterlen = 0;
-    char_u  *ffname = fix_fname(fname);
+    char_u  *ffname = NULL;
     size_t  fname_len;
     char_u  *buf = NULL;
     char_u  *rtp_ffname;
     int	    match;
+    int	    retval = FAIL;
 
-    if (ffname == NULL)
-	return;
-    if (cookie != &APP_LOAD && strstr((char *)p_rtp, (char *)ffname) == NULL)
-    {
-	/* directory is not yet in 'runtimepath', add it */
-	p4 = p3 = p2 = p1 = get_past_head(ffname);
-	for (p = p1; *p; MB_PTR_ADV(p))
-	    if (vim_ispathsep_nocolon(*p))
-	    {
-		p4 = p3; p3 = p2; p2 = p1; p1 = p;
-	    }
-
-	/* now we have:
-	 * rtp/pack/name/start/name
-	 *    p4   p3   p2    p1
-	 *
-	 * find the part up to "pack" in 'runtimepath' */
-	c = *p4;
-	*p4 = NUL;
-
-	/* Find "ffname" in "p_rtp", ignoring '/' vs '\' differences. */
-	fname_len = STRLEN(ffname);
-	insp = p_rtp;
-	buf = alloc(MAXPATHL);
-	if (buf == NULL)
-	    goto theend;
-	while (*insp != NUL)
+    p4 = p3 = p2 = p1 = get_past_head(fname);
+    for (p = p1; *p; MB_PTR_ADV(p))
+	if (vim_ispathsep_nocolon(*p))
 	{
-	    copy_option_part(&insp, buf, MAXPATHL, ",");
-	    add_pathsep(buf);
-	    rtp_ffname = fix_fname(buf);
-	    if (rtp_ffname == NULL)
-		goto theend;
-	    match = vim_fnamencmp(rtp_ffname, ffname, fname_len) == 0;
-	    vim_free(rtp_ffname);
-	    if (match)
-		break;
+	    p4 = p3; p3 = p2; p2 = p1; p1 = p;
 	}
 
-	if (*insp == NUL)
-	    /* not found, append at the end */
-	    insp = p_rtp + STRLEN(p_rtp);
-	else
-	    /* append after the matching directory. */
-	    --insp;
-	*p4 = c;
-
-	/* check if rtp/pack/name/start/name/after exists */
-	afterdir = concat_fnames(ffname, (char_u *)"after", TRUE);
-	if (afterdir != NULL && mch_isdir(afterdir))
-	    afterlen = STRLEN(afterdir) + 1; /* add one for comma */
-
-	oldlen = STRLEN(p_rtp);
-	addlen = STRLEN(ffname) + 1; /* add one for comma */
-	new_rtp = alloc((int)(oldlen + addlen + afterlen + 1));
-							  /* add one for NUL */
-	if (new_rtp == NULL)
+    /* now we have:
+     * rtp/pack/name/start/name
+     *    p4   p3   p2    p1
+     *
+     * find the part up to "pack" in 'runtimepath' */
+    c = *++p4; /* append pathsep in order to expand symlink */
+    *p4 = NUL;
+    ffname = fix_fname(fname);
+    *p4 = c;
+    if (ffname == NULL)
+	return FAIL;
+
+    /* Find "ffname" in "p_rtp", ignoring '/' vs '\' differences. */
+    fname_len = STRLEN(ffname);
+    insp = p_rtp;
+    buf = alloc(MAXPATHL);
+    if (buf == NULL)
+	goto theend;
+    while (*insp != NUL)
+    {
+	copy_option_part(&insp, buf, MAXPATHL, ",");
+	add_pathsep(buf);
+	rtp_ffname = fix_fname(buf);
+	if (rtp_ffname == NULL)
 	    goto theend;
-	keep = (int)(insp - p_rtp);
-	mch_memmove(new_rtp, p_rtp, keep);
-	new_rtp[keep] = ',';
-	mch_memmove(new_rtp + keep + 1, ffname, addlen);
-	if (p_rtp[keep] != NUL)
-	    mch_memmove(new_rtp + keep + addlen, p_rtp + keep,
-							   oldlen - keep + 1);
-	if (afterlen > 0)
-	{
-	    STRCAT(new_rtp, ",");
-	    STRCAT(new_rtp, afterdir);
-	}
-	set_option_value((char_u *)"rtp", 0L, new_rtp, 0);
-	vim_free(new_rtp);
-	vim_free(afterdir);
+	match = vim_fnamencmp(rtp_ffname, ffname, fname_len) == 0;
+	vim_free(rtp_ffname);
+	if (match)
+	    break;
     }
 
-    if (cookie != &APP_ADD_DIR)
-    {
-	static char *plugpat = "%s/plugin/**/*.vim";
-	static char *ftpat = "%s/ftdetect/*.vim";
-	int	    len;
-	char_u	    *pat;
+    if (*insp == NUL)
+	/* not found, append at the end */
+	insp = p_rtp + STRLEN(p_rtp);
+    else
+	/* append after the matching directory. */
+	--insp;
+
+    /* check if rtp/pack/name/start/name/after exists */
+    afterdir = concat_fnames(fname, (char_u *)"after", TRUE);
+    if (afterdir != NULL && mch_isdir(afterdir))
+	afterlen = STRLEN(afterdir) + 1; /* add one for comma */
+
+    oldlen = STRLEN(p_rtp);
+    addlen = STRLEN(fname) + 1; /* add one for comma */
+    new_rtp = alloc((int)(oldlen + addlen + afterlen + 1));
+    /* add one for NUL */
+    if (new_rtp == NULL)
+	goto theend;
+    keep = (int)(insp - p_rtp);
+    mch_memmove(new_rtp, p_rtp, keep);
+    new_rtp[keep] = ',';
+    mch_memmove(new_rtp + keep + 1, fname, addlen);
+    if (p_rtp[keep] != NUL)
+	mch_memmove(new_rtp + keep + addlen, p_rtp + keep, oldlen - keep + 1);
+    if (afterlen > 0)
+    {
+	STRCAT(new_rtp, ",");
+	STRCAT(new_rtp, afterdir);
+    }
+    set_option_value((char_u *)"rtp", 0L, new_rtp, 0);
+    vim_free(new_rtp);
+    retval = OK;
 
-	len = (int)STRLEN(ffname) + (int)STRLEN(ftpat);
-	pat = alloc(len);
-	if (pat == NULL)
-	    goto theend;
-	vim_snprintf((char *)pat, len, plugpat, ffname);
-	source_all_matches(pat);
+theend:
+    vim_free(buf);
+    vim_free(ffname);
+    vim_free(afterdir);
+    return retval;
+}
+
+/*
+ * Load scripts in "plugin" and "ftdetect" directories of the package.
+ */
+    static int
+load_pack_plugin(char_u *fname)
+{
+    static char *plugpat = "%s/plugin/**/*.vim";
+    static char *ftpat = "%s/ftdetect/*.vim";
+    int		len;
+    char_u	*ffname = fix_fname(fname);
+    char_u	*pat = NULL;
+    int		retval = FAIL;
+
+    if (ffname == NULL)
+	return FAIL;
+    len = (int)STRLEN(ffname) + (int)STRLEN(ftpat);
+    pat = alloc(len);
+    if (pat == NULL)
+	goto theend;
+    vim_snprintf((char *)pat, len, plugpat, ffname);
+    source_all_matches(pat);
 
 #ifdef FEAT_AUTOCMD
-	{
-	    char_u *cmd = vim_strsave((char_u *)"g:did_load_filetypes");
+    {
+	char_u *cmd = vim_strsave((char_u *)"g:did_load_filetypes");
 
-	    /* If runtime/filetype.vim wasn't loaded yet, the scripts will be
-	     * found when it loads. */
-	    if (cmd != NULL && eval_to_number(cmd) > 0)
-	    {
-		do_cmdline_cmd((char_u *)"augroup filetypedetect");
-		vim_snprintf((char *)pat, len, ftpat, ffname);
-		source_all_matches(pat);
-		do_cmdline_cmd((char_u *)"augroup END");
-	    }
-	    vim_free(cmd);
+	/* If runtime/filetype.vim wasn't loaded yet, the scripts will be
+	 * found when it loads. */
+	if (cmd != NULL && eval_to_number(cmd) > 0)
+	{
+	    do_cmdline_cmd((char_u *)"augroup filetypedetect");
+	    vim_snprintf((char *)pat, len, ftpat, ffname);
+	    source_all_matches(pat);
+	    do_cmdline_cmd((char_u *)"augroup END");
 	}
-#endif
-	vim_free(pat);
+	vim_free(cmd);
     }
+#endif
+    vim_free(pat);
+    retval = OK;
 
 theend:
-    vim_free(buf);
     vim_free(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);
 }
 
 /*
diff --git a/src/testdir/test_packadd.vim b/src/testdir/test_packadd.vim
index 09b9b82f55..889d77f414 100644
--- a/src/testdir/test_packadd.vim
+++ b/src/testdir/test_packadd.vim
@@ -37,8 +37,8 @@ func Test_packadd()
   call assert_equal(77, g:plugin_also_works)
   call assert_equal(17, g:ftdetect_works)
   call assert_true(len(&rtp) > len(rtp))
-  call assert_true(&rtp =~ '/testdir/Xdir/pack/mine/opt/mytest\($\|,\)')
-  call assert_true(&rtp =~ '/testdir/Xdir/pack/mine/opt/mytest/after$')
+  call assert_match('/testdir/Xdir/pack/mine/opt/mytest\($\|,\)', &rtp)
+  call assert_match('/testdir/Xdir/pack/mine/opt/mytest/after$', &rtp)
 
   " Check exception
   call assert_fails("packadd directorynotfound", 'E919:')
@@ -60,7 +60,7 @@ func Test_packadd_start()
 
   call assert_equal(24, g:plugin_works)
   call assert_true(len(&rtp) > len(rtp))
-  call assert_true(&rtp =~ '/testdir/Xdir/pack/mine/start/other\($\|,\)')
+  call assert_match('/testdir/Xdir/pack/mine/start/other\($\|,\)', &rtp)
 endfunc
 
 func Test_packadd_noload()
@@ -77,7 +77,7 @@ func Test_packadd_noload()
   packadd! mytest
 
   call assert_true(len(&rtp) > len(rtp))
-  call assert_true(&rtp =~ 'testdir/Xdir/pack/mine/opt/mytest\($\|,\)')
+  call assert_match('testdir/Xdir/pack/mine/opt/mytest\($\|,\)', &rtp)
   call assert_equal(0, g:plugin_works)
 
   " check the path is not added twice
@@ -108,7 +108,7 @@ func Test_packadd_symlink_dir()
   packadd mytest
 
   " Must have been inserted in the middle, not at the end
-  call assert_true(&rtp =~ '/pack/mine/opt/mytest,')
+  call assert_match('/pack/mine/opt/mytest,', &rtp)
   call assert_equal(44, g:plugin_works)
 
   " No change when doing it again.
@@ -121,6 +121,43 @@ func Test_packadd_symlink_dir()
   exec "silent !rm" top2_dir
 endfunc
 
+func Test_packadd_symlink_dir2()
+  if !has('unix')
+    return
+  endif
+  let top2_dir = s:topdir . '/Xdir2'
+  let real_dir = s:topdir . '/Xsym/pack'
+  call mkdir(top2_dir, 'p')
+  call mkdir(real_dir, 'p')
+  let &rtp = top2_dir . ',' . top2_dir . '/after'
+  let &packpath = &rtp
+
+  exec "silent !ln -s ../Xsym/pack"  top2_dir . '/pack'
+  let s:plugdir = 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('/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
+  exec "silent !rm" top2_dir . '/pack'
+  exec "silent !rmdir" top2_dir
+endfunc
+
 " Check command-line completion for 'packadd'
 func Test_packadd_completion()
   let optdir1 = &packpath . '/pack/mine/opt'
@@ -196,9 +233,9 @@ func Test_helptags()
   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()
diff --git a/src/version.c b/src/version.c
index e4a4b51f8d..5786860a5b 100644
--- a/src/version.c
+++ b/src/version.c
@@ -772,6 +772,8 @@ static char *(features[]) =
 static int included_patches[] =
 {   /* Add new patch number below this line */
 /**/
+    1469,
+/**/
     1468,
 /**/
     1467,