Re: Viper tests failure?

[Phillip Lord]

Eli Zaretskii <address@hidden> writes:
>> 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


Okay, I was wondering whether it was a problem with the suffix.

The viper-test is fix obviously fine by me -- should have done it that
way in the first place, although uncovering the Fload but is a nice side
effect. Can you add it to emacs-25 (I'm away from my windows machine,
and I'd rather not commit something I can't test).


Phil