Reported by: "Drew Adams" <drew.adams <at> oracle.com>
Date: Tue, 5 Jan 2010 14:09:02 UTC
Severity: normal
Merged with 5309
Done: Eli Zaretskii <eliz <at> gnu.org>
Bug is archived. No further changes may be made.
View this message in rfc822 format
From: Lennart Borgman <lennart.borgman <at> gmail.com> To: Eli Zaretskii <eliz <at> gnu.org> Cc: 5303 <at> debbugs.gnu.org, cyd <at> stupidchicken.com, Michael Albinus <michael.albinus <at> gmx.de>, drew.adams <at> oracle.com Subject: bug#5303: 23.1.91; Cannot load .emacs-history from savehist.el Date: Wed, 20 Jan 2010 02:25:33 +0100
On Tue, Jan 19, 2010 at 10:24 PM, Eli Zaretskii <eliz <at> gnu.org> wrote:
>> From: Michael Albinus <michael.albinus <at> gmx.de>
>> Cc: Drew Adams <drew.adams <at> oracle.com>, lennart.borgman <at> gmail.com, 5303 <at> debbugs.gnu.org, cyd <at> stupidchicken.com
>> Date: Tue, 19 Jan 2010 20:36:27 +0100
>>
>> Eli Zaretskii <eliz <at> 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.
>
GNU bug tracking system
Copyright (C) 1999 Darren O. Benham,
1997,2003 nCipher Corporation Ltd,
1994-97 Ian Jackson.