bug#5303: 23.1.91; Cannot load .emacs-history from savehist.el

[Lennart Borgman]

On Tue, Jan 19, 2010 at 10:24 PM, Eli Zaretskii <eliz@gnu.org> wrote:
>> From: Michael Albinus <michael.albinus@gmx.de>
>> Cc: Drew Adams <drew.adams@oracle.com>,  lennart.borgman@gmail.com,  
>> 5303@debbugs.gnu.org,  cyd@stupidchicken.com
>> Date: Tue, 19 Jan 2010 20:36:27 +0100
>>
>> Eli Zaretskii <eliz@gnu.org> writes:
>>
>> > What is in the-file.el? will an empty file do? if not, what should be
>> > there to reproduce the problem?
>>
>> ------
>> (setq command-history '((describe-key "^Z")))
>> ------
>
> Thanks.
>
> I see the problem, but there's too many dragons here, or maybe it's
> too late.


Too many dragons ;-)


> Here's the story I got so far.
>
> The root cause for the problem is that we read this file in text mode,
> not in binary mode.  Because of that, the literal C-z character in the
> file is treated as EOF (a CP/M legacy, no less), and the rest is
> history.


Yes.


> Why do we read the file in text mode?  That's where things become
> rather murky.  AFAIK, we ought to be reading the file with
> load-with-code-conversion, which is the value of
> Vload_source_file_function:
>
>      /* We are loading a source file (*.el).  */
>      if (!NILP (Vload_source_file_function))
>        {
>          Lisp_Object val;
>
>          if (fd >= 0)
>            emacs_close (fd);
>          val = call4 (Vload_source_file_function, found, hist_file_name,
>                       NILP (noerror) ? Qnil : Qt,
>                       (NILP (nomessage) || force_load_messages) ? Qnil : Qt);
>          return unbind_to (count, val);
>        }
>
> But we don't get to this code because we are caught here:
>
>  if (!bcmp (SDATA (found) + SBYTES (found) - 4,
>             ".elc", 4)
>      || (version = safe_to_load_p (fd)) > 0)
>    /* Load .elc files directly, but not when they are
>       remote and have no handler!  */
>
> [Btw, I don't understand this condition.  What it's supposed to do?
> allow us to load a byte-compiled file whose name does not end in a
> .elc?]


The condition looks just wrong. Shouldn't it be just

  if (bcmp (SDATA (found) + SBYTES (found) - 4,
             ".elc", 4)
      && (version = safe_to_load_p (fd)) > 0)


> (Are you saying "Huh??" yet?)


No. I am shaking my head.


>  And we are caught here because,
> although the file name does not end in .elc, safe_to_load_p returns
> 1.  Why? because fd is -2, and safe_to_load_p bravely returns 1 when
> it fails to read from the file:
>
>  static int
>  safe_to_load_p (fd)
>       int fd;
>  {
>    char buf[512];
>    int nbytes, i;
>    int safe_p = 1;
>    int version = 1;
>
>    /* Read the first few bytes from the file, and look for a line
>       specifying the byte compiler version used.  */
>    nbytes = emacs_read (fd, buf, sizeof buf - 1);
>    if (nbytes > 0)
>      {
>      ...
>      }
>    if (safe_p)
>      safe_p = version;
>
>    lseek (fd, 0, SEEK_SET);
>    return safe_p;
>  }

> Granted, trying to read from fd = -2 fails, and we return 1 here.  Is
> that really supposed to happen?


No. I think it is too brave. Default need to be -1 (or whatever
"impossible" should be).


> Another question is why fd is -2.  That means `openp' detected that
> C:/the-file.el is a ``magic'' file, i.e. it has a file handler.  But
> this happens in a recursive call to `load', the one where, as Michael
> says:


The first thing I see is that complete_filename_p lacks a check for
w32. (Not important here, but should probably be fixed. Or at least
commented.)

Yes, indeed tramp thinks a file in a w32 top directory is a remote file:

  (find-file-name-handler "c:/emacs-history" 'file-exists-p) => t
  (find-file-name-handler "c:/dl/emacs-history" 'file-exists-p) => nil

I have no idea why, but the cause of the problem seems to be that
tramp for some reason think this handler should be invoked for a lot
of operations when the file is in the root. (Operation file-exists-p
does not seem to be involved but a lot of other operations.)

Trying trace-function I saw that tramp-completion-handler is invoked
inside tramp-completion-run-real-handler:

======================================================================
1 -> tramp-completion-file-name-handler: operation=file-readable-p
args=("c:/emacs-history.elc")
| 2 -> tramp-completion-run-real-handler: operation=file-readable-p
args=("c:/emacs-history.elc")
| | 3 -> tramp-completion-file-name-handler:
operation=expand-file-name args=("c:/emacs-history.elc" nil)
| | | 4 -> tramp-completion-run-real-handler:
operation=expand-file-name args=("c:/emacs-history.elc" nil)
| | | 4 <- tramp-completion-run-real-handler: "c:/emacs-history.elc"
| | 3 <- tramp-completion-file-name-handler: "c:/emacs-history.elc"
| 2 <- tramp-completion-run-real-handler: nil
1 <- tramp-completion-file-name-handler: nil
======================================================================

If dynamic variable handling is not broken that means that
inhibit-file-name-handlers have gotten a new value again after the
call to tramp-completion-run-real-handler where
tramp-completion-file-name-handler is no more included. Maybe that is
a bit strange?

I give up here at the moment ;-)


>> the last action I can see is disabling Tramp' file name completion
>> handler, and calling `load', again.
>
> It looks like disabling Tramp's file name completion handler does not
> prevent `openp' from thinking that C:/the-file.el has a handler.
> However, when we actually try to find this handler after `openp'
> returns -2, the handler turns out to be nil:
>
>  /* If FD is -2, that means openp found a magic file.  */
>  if (fd == -2)
>    {
>      if (NILP (Fequal (found, file)))
>        /* If FOUND is a different file name from FILE,
>           find its handler even if we have already inhibited
>           the `load' operation on FILE.  */
>        handler = Ffind_file_name_handler (found, Qt);
>      else
>        handler = Ffind_file_name_handler (found, Qload);
>      if (! NILP (handler))
>        return call5 (handler, Qload, found, noerror, nomessage, Qt);
>    }
>
> Thus, we don't call the handler, and end up with fd = -2, but in the
> portion of code that doesn't seem to be able to handle this situation
> well: it just calls readevalloop.  What is this part of Fload for? is
> it for the case where we don't yet have mule.el loaded, and thus
> load-with-code-conversion is not available?
>
> I didn't yet have time to see why Emacs 23.1 does not hit this
> problem, but given the above mess, it could well be by sheer luck.
>
> Anyway, it's too late now.  If no one beats me to it, I will continue
> tomorrow.
>