Re: Viper tests failure?

[Eli Zaretskii]

> From: address@hidden (Phillip Lord)
> Date: Tue, 21 Jun 2016 15:27:28 +0100
> Cc: address@hidden
> 
> > With emacs-25 commit e5e886d12f3eff3102916c82a5aa82e0db714579
> > Emacs compiled using Mingw64 I get a strange error
> > running `make check':
> >
> > $ src/emacs --batch -Q -l ../emacs/test/automated/viper-tests.el --eval 
> > '(ert-
> > run-tests-batch t)'
> > Running 6 tests (2016-06-21 07:01:58+0200)
> > Loading viper...
> > Test viper-test-fix backtrace:
> >   byte-code(\204'\306\307\310!CB\301    CB\302\nCB\303
> >                                                       CB\311\307\311!
> >   load("viper")
> >   load-library("viper")
> >   viper-mode()
> >   (progn (viper-mode) (switch-to-buffer (get-buffer-create "*viper-tes
> >   (unwind-protect (progn (viper-mode) (switch-to-buffer (get-buffer-cr
> >   (let ((noninteractive nil) (viper-inhibit-startup-message (quote t))
> >   viper-test-undo-kmacro([])
> >   apply(viper-test-undo-kmacro [])
> >   (setq value-4 (apply fn-2 args-3))
> >   (unwind-protect (setq value-4 (apply fn-2 args-3)) (setq form-descri
> >   (if (unwind-protect (setq value-4 (apply fn-2 args-3)) (setq form-de
> >   (let (form-description-6) (if (unwind-protect (setq value-4 (apply f
> >   (let ((value-4 (quote ert-form-evaluation-aborted-5))) (let (form-de
> >   (let ((fn-2 (function viper-test-undo-kmacro)) (args-3 (list []))) (
> >   (lambda nil (let ((fn-2 (function viper-test-undo-kmacro)) (args-3 (
> >   ert--run-test-internal([cl-struct-ert--test-execution-info [cl-struc
> >   ert-run-test([cl-struct-ert-test viper-test-fix "Test that the viper
> >   ert-run-or-rerun-test([cl-struct-ert--stats t [[cl-struct-ert-test v
> >   ert-run-tests(t #[385 "\306\307\"address@hidden
> >   ert-run-tests-batch(t)
> >   eval((ert-run-tests-batch t))
> >   command-line-1(("-l" "../emacs/test/automated/viper-tests.el" "--eva
> >   command-line()
> >   normal-top-level()
> > Test viper-test-fix condition:
> >     (file-error "Opening stdio stream" "No such file or directory" 
> > "c:/Users/Fabrice/AppData/Local/Temp/viper-tests21052GpM")
> >    FAILED  1/6  viper-test-fix
> >
> > I'm not sure what is happening there.
> > I checked the temp file is created and does exist, so why is there 
> > a failure to open it?
> >
> > Regards,
> >
> > Fabrice
> 
> 
> The file is only created to be empty (viper loads the custom-file
> whatever I do, so making an empty one seemed the only way to disable it.
> 
> I've checked and the error comes from the viper-load-custom-file
> function which is calle during load. The file exists according to emacs,
> but it cannot be loaded.
> 
> Not sure what to test next.

The problem is real.  There's a bug in Fload: it blindly overwrites
the last character of the file name, without making sure it ends in
".elc".  That's why the simple change below (which is really a
workaround) fixes the problem for me.

The bug is here:

          /* openp already checked for newness, no point doing it again.
             FIXME would be nice to get a message when openp
             ignores suffix order due to load_prefer_newer.  */
          if (!load_prefer_newer)
            {
              result = stat (SSDATA (efound), &s1);
              if (result == 0)
                {
                  SSET (efound, SBYTES (efound) - 1, 0);
                  result = stat (SSDATA (efound), &s2);
                  SSET (efound, SBYTES (efound) - 1, 'c');
                }

This removes the last character of the file, then replaces it with
'c', but we can end up in this fragment when the file name does not
end in a ".elc", because the condition which guards this says:

  if (suffix_p (found, ".elc") || (fd >= 0 && (version = safe_to_load_version 
(fd)) > 0))

and the second alternative does not guarantee that the file name ends
in ".elc".

The error message we see on Windows is misleading, because it uses the
correct file name, not the one Emacs tries to open.  The error only
happens on Windows, because we don't use fdopen there, but instead
close and reopen the file -- using the wrong name.

And here's the simple workaround I mentioned:

diff --git a/test/lisp/emulation/viper-tests.el 
b/test/lisp/emulation/viper-tests.el
index 074dd63..0d6095b 100644
--- a/test/lisp/emulation/viper-tests.el
+++ b/test/lisp/emulation/viper-tests.el
@@ -38,7 +38,7 @@ viper-test-undo-kmacro
         ;; Select an expert-level for the same reason.
         (viper-expert-level 5)
         ;; viper loads this even with -q so make sure it's empty!
-        (viper-custom-file-name (make-temp-file "viper-tests"))
+        (viper-custom-file-name (make-temp-file "viper-tests" nil ".elc"))
         (before-buffer (current-buffer)))
     (unwind-protect
         (progn