changeset 21580:93dae47699fb v8.2.1340

patch 8.2.1340: some tests fail on Cirrus CI and/or with FreeBSD Commit: https://github.com/vim/vim/commit/b86abadf87bd0f85f800077171ec4b98aefff776 Author: Bram Moolenaar <Bram@vim.org> Date: Sat Aug 1 16:08:19 2020 +0200 patch 8.2.1340: some tests fail on Cirrus CI and/or with FreeBSD Problem: Some tests fail on Cirrus CI and/or with FreeBSD. Solution: Make 'backupskip' empty. Do not run tests as root. Check for directory when using viminfo. (Ozaki Kiichi, closes #6596)
author Bram Moolenaar <Bram@vim.org>
date Sat, 01 Aug 2020 16:15:04 +0200
parents 30c6a9fb9e8d
children bf7e83295fc3
files .cirrus.yml src/testdir/test_backup.vim src/testdir/test_edit.vim src/testdir/test_viminfo.vim src/testdir/test_writefile.vim src/version.c src/viminfo.c
diffstat 7 files changed, 57 insertions(+), 45 deletions(-) [+]
line wrap: on
line diff
--- a/.cirrus.yml
+++ b/.cirrus.yml
@@ -11,6 +11,9 @@ freebsd_12_task:
     - NPROC=$(getconf _NPROCESSORS_ONLN)
     - ./configure --with-features=${FEATURES}
     - make -j${NPROC}
+  test_script:
     - src/vim --version
-  test_script:
-    - make test
+      # run tests as user "cirrus" instead of root
+    - pw useradd cirrus -m
+    - chown -R cirrus:cirrus .
+    - sudo -u cirrus make test
--- a/src/testdir/test_backup.vim
+++ b/src/testdir/test_backup.vim
@@ -19,6 +19,22 @@ func Test_backup()
   call delete('Xbackup.txt~')
 endfunc
 
+func Test_backup_backupskip()
+  set backup backupdir=. backupskip=*.txt
+  new
+  call setline(1, ['line1', 'line2'])
+  :f Xbackup.txt
+  :w! Xbackup.txt
+  " backup file is only created after
+  " writing a second time (before overwriting)
+  :w! Xbackup.txt
+  call assert_false(filereadable('Xbackup.txt~'))
+  bw!
+  set backup&vim backupdir&vim backupskip&vim
+  call delete('Xbackup.txt')
+  call delete('Xbackup.txt~')
+endfunc
+
 func Test_backup2()
   set backup backupdir=.// backupskip=
   new
@@ -30,7 +46,7 @@ func Test_backup2()
   :w! Xbackup.txt
   sp *Xbackup.txt~
   call assert_equal(['line1', 'line2', 'line3'], getline(1,'$'))
-  let f=expand('%')
+  let f = expand('%')
   call assert_match('%testdir%Xbackup.txt\~', f)
   bw!
   bw!
@@ -50,7 +66,7 @@ func Test_backup2_backupcopy()
   :w! Xbackup.txt
   sp *Xbackup.txt~
   call assert_equal(['line1', 'line2', 'line3'], getline(1,'$'))
-  let f=expand('%')
+  let f = expand('%')
   call assert_match('%testdir%Xbackup.txt\~', f)
   bw!
   bw!
@@ -61,14 +77,11 @@ endfunc
 
 " Test for using a non-existing directory as a backup directory
 func Test_non_existing_backupdir()
-  CheckNotBSD
-  let save_backup = &backupdir
-  set backupdir=./non_existing_dir
+  set backupdir=./non_existing_dir backupskip=
   call writefile(['line1'], 'Xfile')
   new Xfile
-  " TODO: write doesn't fail in Cirrus FreeBSD CI test
   call assert_fails('write', 'E510:')
-  let &backupdir = save_backup
+  set backupdir&vim backupskip&vim
   call delete('Xfile')
 endfunc
 
--- a/src/testdir/test_edit.vim
+++ b/src/testdir/test_edit.vim
@@ -1682,7 +1682,6 @@ endfunc
 " Test for editing a file without read permission
 func Test_edit_file_no_read_perm()
   CheckUnix
-  CheckNotBSD
   call writefile(['one', 'two'], 'Xfile')
   call setfperm('Xfile', '-w-------')
   new
--- a/src/testdir/test_viminfo.vim
+++ b/src/testdir/test_viminfo.vim
@@ -807,7 +807,7 @@ func Test_viminfo_perm()
 
   " Try to write the viminfo to a directory
   call mkdir('Xdir')
-  call assert_fails('wviminfo Xdir', 'E886:')
+  call assert_fails('wviminfo Xdir', 'E137:')
   call delete('Xdir', 'rf')
 endfunc
 
--- a/src/testdir/test_writefile.vim
+++ b/src/testdir/test_writefile.vim
@@ -136,9 +136,7 @@ func Test_writefile_sync_arg()
 endfunc
 
 func Test_writefile_sync_dev_stdout()
-  if !has('unix')
-    return
-  endif
+  CheckUnix
   if filewritable('/dev/stdout')
     " Just check that this doesn't cause an error.
     call writefile(['one'], '/dev/stdout')
@@ -371,13 +369,10 @@ endfunc
 
 " Test for writing to a readonly file
 func Test_write_readonly()
-  " In Cirrus-CI, the freebsd tests are run under a root account. So this test
-  " doesn't fail.
-  CheckNotBSD
   call writefile([], 'Xfile')
   call setfperm('Xfile', "r--------")
   edit Xfile
-  set noreadonly
+  set noreadonly backupskip=
   call assert_fails('write', 'E505:')
   let save_cpo = &cpo
   set cpo+=W
@@ -386,37 +381,32 @@ func Test_write_readonly()
   call setline(1, ['line1'])
   write!
   call assert_equal(['line1'], readfile('Xfile'))
+  set backupskip&
   call delete('Xfile')
 endfunc
 
 " Test for 'patchmode'
 func Test_patchmode()
-  CheckNotBSD
   call writefile(['one'], 'Xfile')
-  set patchmode=.orig nobackup writebackup
+  set patchmode=.orig nobackup backupskip= writebackup
   new Xfile
   call setline(1, 'two')
   " first write should create the .orig file
   write
-  " TODO: Xfile.orig is not created in Cirrus FreeBSD CI test
   call assert_equal(['one'], readfile('Xfile.orig'))
   call setline(1, 'three')
   " subsequent writes should not create/modify the .orig file
   write
   call assert_equal(['one'], readfile('Xfile.orig'))
-  set patchmode& backup& writebackup&
+  set patchmode& backup& backupskip& writebackup&
   call delete('Xfile')
   call delete('Xfile.orig')
 endfunc
 
 " Test for writing to a file in a readonly directory
 func Test_write_readonly_dir()
-  if !has('unix') || has('bsd')
-    " On MS-Windows, modifying files in a read-only directory is allowed.
-    " In Cirrus-CI for Freebsd, tests are run under a root account where
-    " modifying files in a read-only directory are allowed.
-    return
-  endif
+  " On MS-Windows, modifying files in a read-only directory is allowed.
+  CheckUnix
   call mkdir('Xdir')
   call writefile(['one'], 'Xdir/Xfile1')
   call setfperm('Xdir', 'r-xr--r--')
@@ -426,12 +416,12 @@ func Test_write_readonly_dir()
   call assert_fails('write', 'E212:')
   " try to create a backup file in the directory
   edit! Xdir/Xfile1
-  set backupdir=./Xdir
+  set backupdir=./Xdir backupskip=
   set patchmode=.orig
   call assert_fails('write', 'E509:')
   call setfperm('Xdir', 'rwxr--r--')
   call delete('Xdir', 'rf')
-  set backupdir& patchmode&
+  set backupdir& backupskip& patchmode&
 endfunc
 
 " Test for writing a file using invalid file encoding
--- a/src/version.c
+++ b/src/version.c
@@ -755,6 +755,8 @@ static char *(features[]) =
 static int included_patches[] =
 {   /* Add new patch number below this line */
 /**/
+    1340,
+/**/
     1339,
 /**/
     1338,
--- a/src/viminfo.c
+++ b/src/viminfo.c
@@ -3007,6 +3007,7 @@ read_viminfo(
 {
     FILE	*fp;
     char_u	*fname;
+    stat_T	st;		// mch_stat() of existing viminfo file
 
     if (no_viminfo())
 	return FAIL;
@@ -3031,6 +3032,11 @@ read_viminfo(
     vim_free(fname);
     if (fp == NULL)
 	return FAIL;
+    if (mch_fstat(fileno(fp), &st) < 0 || S_ISDIR(st.st_mode))
+    {
+	fclose(fp);
+	return FAIL;
+    }
 
     viminfo_errcnt = 0;
     do_viminfo(fp, NULL, flags);
@@ -3054,12 +3060,12 @@ write_viminfo(char_u *file, int forceit)
     FILE	*fp_out = NULL;	// output viminfo file
     char_u	*tempname = NULL;	// name of temp viminfo file
     stat_T	st_new;		// mch_stat() of potential new file
+    stat_T	st_old;		// mch_stat() of existing viminfo file
 #if defined(UNIX) || defined(VMS)
     mode_t	umask_save;
 #endif
 #ifdef UNIX
     int		shortname = FALSE;	// use 8.3 file name
-    stat_T	st_old;		// mch_stat() of existing viminfo file
 #endif
 #ifdef MSWIN
     int		hidden = FALSE;
@@ -3097,20 +3103,20 @@ write_viminfo(char_u *file, int forceit)
 	// write the new viminfo into, in the same directory as the
 	// existing viminfo file, which will be renamed once all writing is
 	// successful.
+	if (mch_fstat(fileno(fp_in), &st_old) < 0
+		|| S_ISDIR(st_old.st_mode)
 #ifdef UNIX
-	// For Unix we check the owner of the file.  It's not very nice to
-	// overwrite a user's viminfo file after a "su root", with a
-	// viminfo file that the user can't read.
-	st_old.st_dev = (dev_t)0;
-	st_old.st_ino = 0;
-	st_old.st_mode = 0600;
-	if (mch_stat((char *)fname, &st_old) == 0
-		&& getuid() != ROOT_UID
-		&& !(st_old.st_uid == getuid()
-			? (st_old.st_mode & 0200)
-			: (st_old.st_gid == getgid()
-				? (st_old.st_mode & 0020)
-				: (st_old.st_mode & 0002))))
+		// For Unix we check the owner of the file.  It's not very nice
+		// to overwrite a user's viminfo file after a "su root", with a
+		// viminfo file that the user can't read.
+		|| (getuid() != ROOT_UID
+		    && !(st_old.st_uid == getuid()
+			    ? (st_old.st_mode & 0200)
+			    : (st_old.st_gid == getgid()
+				    ? (st_old.st_mode & 0020)
+				    : (st_old.st_mode & 0002))))
+#endif
+		)
 	{
 	    int	tt = msg_didany;
 
@@ -3120,7 +3126,6 @@ write_viminfo(char_u *file, int forceit)
 	    fclose(fp_in);
 	    goto end;
 	}
-#endif
 #ifdef MSWIN
 	// Get the file attributes of the existing viminfo file.
 	hidden = mch_ishidden(fname);