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

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

[Chong Yidong]

> Trying to load the attached file raises the error "End of file during
> parsing: c:/.emacs-history".
>
> There is no real end-of-file problem, however: This file was created
> by savehist.el using Emacs 23.1, and it loads fine in all releases of
> Emacs (20 through 23).  An error is raised only for the Emacs
> 23.1.91.1 pretest.
>
> However, something weird is going on. If the file is in c:/ when I try
> to load it, then the error is raised. If the file is in c:/mydir/ when
> I try to load it, then it loads with no error. I do not understand
> this at all. Exactly the same file, different behavior. And it doesn't
> matter whether I make the file copy using Emacs C-x C-w or using
> Windows copy+paste.

I'm afraid I can't reproduce this.  There's no error loading the file.
Maybe it's a Windows-only bug.

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

[Lennart Borgman]

On Sat, Jan 16, 2010 at 8:33 PM, Chong Yidong <cyd@stupidchicken.com> wrote:
>> Trying to load the attached file raises the error "End of file during
>> parsing: c:/.emacs-history".
>>
>> There is no real end-of-file problem, however: This file was created
>> by savehist.el using Emacs 23.1, and it loads fine in all releases of
>> Emacs (20 through 23).  An error is raised only for the Emacs
>> 23.1.91.1 pretest.
>>
>> However, something weird is going on. If the file is in c:/ when I try
>> to load it, then the error is raised. If the file is in c:/mydir/ when
>> I try to load it, then it loads with no error. I do not understand
>> this at all. Exactly the same file, different behavior. And it doesn't
>> matter whether I make the file copy using Emacs C-x C-w or using
>> Windows copy+paste.
>
> I'm afraid I can't reproduce this.  There's no error loading the file.
> Maybe it's a Windows-only bug.

I can confirm the bug is there. If I do

   M-x load-file

and the file is named c:/emacs-history it fails, but not if the name
is c:/dl/emacs-history.

The problem is in file-name-handler-alist. Removing
tramp-completion-file-name-handler makes the error go away. But
unfortunately that is not TRTTD...

Here is the command to remove it:

(setq file-name-handler-alist
      (delete '("\\`\\([a-zA-Z]:\\)?/[^/]*\\'" .
tramp-completion-file-name-handler)
	      file-name-handler-alist))

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

[Kevin Rodgers]

Lennart Borgman wrote:
> The problem is in file-name-handler-alist. Removing
> tramp-completion-file-name-handler makes the error go away. But
> unfortunately that is not TRTTD...
> 
> Here is the command to remove it:
> 
> (setq file-name-handler-alist
>       (delete '("\\`\\([a-zA-Z]:\\)?/[^/]*\\'" .
> tramp-completion-file-name-handler)
> 	      file-name-handler-alist))

Or:

(setq file-name-handler-alista
       (delq (rassq 'tramp-completion-file-name-handler file-name-handler-alist)
	    file-name-handler-alist))

-- 
Kevin Rodgers
Denver, Colorado, USA

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

[Michael Albinus]

Lennart Borgman <lennart.borgman@gmail.com> writes:

> The problem is in file-name-handler-alist. Removing
> tramp-completion-file-name-handler makes the error go away. But
> unfortunately that is not TRTTD...
>
> Here is the command to remove it:
>
> (setq file-name-handler-alist
>       (delete '("\\`\\([a-zA-Z]:\\)?/[^/]*\\'" .
> tramp-completion-file-name-handler)
> 	      file-name-handler-alist))

I will check it. Usually, I do not run W32 machines; it may take a
couple of days until I'm able to do it.

Best regards, Michael.

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

[Lennart Borgman]

On Sun, Jan 17, 2010 at 5:54 PM, Michael Albinus <michael.albinus@gmx.de> wrote:
> Lennart Borgman <lennart.borgman@gmail.com> writes:
>
>> The problem is in file-name-handler-alist. Removing
>> tramp-completion-file-name-handler makes the error go away. But
>> unfortunately that is not TRTTD...
>>
>> Here is the command to remove it:
>>
>> (setq file-name-handler-alist
>>       (delete '("\\`\\([a-zA-Z]:\\)?/[^/]*\\'" .
>> tramp-completion-file-name-handler)
>>             file-name-handler-alist))
>
> I will check it. Usually, I do not run W32 machines; it may take a
> couple of days until I'm able to do it.
>
> Best regards, Michael.


That sounds very nice, but why do you need a w32 machine? The file
name regexp above simply matches a file at the top of a w32 drive.
They look like (in Emacs file syntax) like "c:/file.txt". What more do
you need to know?

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

[Michael Albinus]

Lennart Borgman <lennart.borgman@gmail.com> writes:

> That sounds very nice, but why do you need a w32 machine? The file
> name regexp above simply matches a file at the top of a w32 drive.
> They look like (in Emacs file syntax) like "c:/file.txt". What more do
> you need to know?

Tramp has a mechanism which detects, whether it is in "completion mode",
or not. Completion mode means, that user name or host name shall be
expanded, and the remote file name is not completed yet.

It looks, like something is broken here. And I suspect, this is special
on W32.

Maybe I find a kind colleague tomorrow, who let's her machine for me for
an hour or so.

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

[Lennart Borgman]

On Sun, Jan 17, 2010 at 6:18 PM, Michael Albinus <michael.albinus@gmx.de> wrote:
> Lennart Borgman <lennart.borgman@gmail.com> writes:
>
>> That sounds very nice, but why do you need a w32 machine? The file
>> name regexp above simply matches a file at the top of a w32 drive.
>> They look like (in Emacs file syntax) like "c:/file.txt". What more do
>> you need to know?
>
> Tramp has a mechanism which detects, whether it is in "completion mode",
> or not. Completion mode means, that user name or host name shall be
> expanded, and the remote file name is not completed yet.
>
> It looks, like something is broken here. And I suspect, this is special
> on W32.


Yes, it is broken and it has been so for very long time. I just have
not understood before that it was in the special case with a file in
the root of a w32 drive.

However I wonder why those files at all are interesting for tramp. I
know little about tramp, but does not remote file names always start
with something like "/ssh:", "/ftp:", "/telnet:" etc?

If so why look for file names starting with "c:/"?

And why does this file handler at all jump in during `load'? Shouldn't
tramp-completion-file-name-handler just come in during completion? It
should be invoked when the operations are file-name-completion or
file-name-all-completion (see
tramp-completion-file-name-handler-alist), but are these operations
used during `load'?


A note: tramp-completion-file-name-regexp-unified is built from

;;;###autoload
(defconst tramp-root-regexp
  (if (memq system-type '(cygwin windows-nt))
      "\\`\\([a-zA-Z]:\\)?/"
    "\\`/")
  "Beginning of an incomplete Tramp file name.
Usually, it is just \"\\\\`/\".  On W32 systems, there might be a
volume letter, which will be removed by `tramp-drop-volume-letter'.")



> Maybe I find a kind colleague tomorrow, who let's her machine for me for
> an hour or so.
>

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

[Michael Albinus]

[-- Attachment #1: Type: text/plain, Size: 297 bytes --]

Lennart Borgman <lennart.borgman@gmail.com> writes:

> Yes, it is broken and it has been so for very long time. I just have
> not understood before that it was in the special case with a file in
> the root of a w32 drive.

I could break down the example file from Drew to the following contents:


[-- Attachment #2: emacs-history --]
[-- Type: text/plain, Size: 45 bytes --]

(setq command-history '((describe-key "\x1a")))

[-- Attachment #3: Type: text/plain, Size: 53 bytes --]


When enabling traces of all Tramp functions, I see


[-- Attachment #4: 2 --]
[-- Type: text/plain, Size: 3689 bytes --]

======================================================================
1 -> tramp-completion-file-name-handler: operation=load args=("c:/emacs-history" nil nil t)
| 2 -> tramp-completion-run-real-handler: operation=load args=("c:/emacs-history" nil nil t)
| | 3 -> tramp-completion-file-name-handler: operation=substitute-in-file-name args=("c:/emacs-history")
| | | 4 -> tramp-completion-run-real-handler: operation=substitute-in-file-name args=("c:/emacs-history")
| | | 4 <- tramp-completion-run-real-handler: "c:/emacs-history"
| | 3 <- tramp-completion-file-name-handler: "c:/emacs-history"
| | 3 -> tramp-completion-file-name-handler: operation=expand-file-name args=("c:/emacs-history" "l:/usr/local/src/emacs-jabber")
| | | 4 -> tramp-completion-run-real-handler: operation=expand-file-name args=("c:/emacs-history" "l:/usr/local/src/emacs-jabber")
| | | 4 <- tramp-completion-run-real-handler: "c:/emacs-history"
| | 3 <- tramp-completion-file-name-handler: "c:/emacs-history"
| | 3 -> tramp-completion-file-name-handler: operation=expand-file-name args=("c:/emacs-history" nil)
| | | 4 -> tramp-completion-run-real-handler: operation=expand-file-name args=("c:/emacs-history" nil)
| | | 4 <- tramp-completion-run-real-handler: "c:/emacs-history"
| | 3 <- tramp-completion-file-name-handler: "c:/emacs-history"
| | 3 -> tramp-completion-file-name-handler: operation=file-readable-p args=("c:/emacs-history")
| | | 4 -> tramp-completion-run-real-handler: operation=file-readable-p args=("c:/emacs-history")
| | | | 5 -> tramp-completion-file-name-handler: operation=expand-file-name args=("c:/emacs-history" nil)
| | | | | 6 -> tramp-completion-run-real-handler: operation=expand-file-name args=("c:/emacs-history" nil)
| | | | | 6 <- tramp-completion-run-real-handler: "c:/emacs-history"
| | | | 5 <- tramp-completion-file-name-handler: "c:/emacs-history"
| | | 4 <- tramp-completion-run-real-handler: t
| | 3 <- tramp-completion-file-name-handler: t
| | 3 -> tramp-completion-file-name-handler: operation=expand-file-name args=("c:/emacs-history" "c:/")
| | | 4 -> tramp-completion-run-real-handler: operation=expand-file-name args=("c:/emacs-history" "c:/")
| | | 4 <- tramp-completion-run-real-handler: "c:/emacs-history"
| | 3 <- tramp-completion-file-name-handler: "c:/emacs-history"
| | 3 -> tramp-completion-file-name-handler: operation=file-directory-p args=("c:/emacs-history")
| | | 4 -> tramp-completion-run-real-handler: operation=file-directory-p args=("c:/emacs-history")
| | | | 5 -> tramp-completion-file-name-handler: operation=expand-file-name args=("c:/emacs-history" "c:/")
| | | | | 6 -> tramp-completion-run-real-handler: operation=expand-file-name args=("c:/emacs-history" "c:/")
| | | | | 6 <- tramp-completion-run-real-handler: "c:/emacs-history"
| | | | 5 <- tramp-completion-file-name-handler: "c:/emacs-history"
| | | 4 <- tramp-completion-run-real-handler: nil
| | 3 <- tramp-completion-file-name-handler: nil
| | 3 -> tramp-completion-file-name-handler: operation=file-truename args=("c:/emacs-history")
| | | 4 -> tramp-completion-run-real-handler: operation=file-truename args=("c:/emacs-history")
| | | | 5 -> tramp-completion-file-name-handler: operation=expand-file-name args=("c:/emacs-history" nil)
| | | | | 6 -> tramp-completion-run-real-handler: operation=expand-file-name args=("c:/emacs-history" nil)
| | | | | 6 <- tramp-completion-run-real-handler: "c:/emacs-history"
| | | | 5 <- tramp-completion-file-name-handler: "c:/emacs-history"
| | | 4 <- tramp-completion-run-real-handler: "c:/emacs-history"
| | 3 <- tramp-completion-file-name-handler: "c:/emacs-history"
======================================================================

[-- Attachment #5: Type: text/plain, Size: 1360 bytes --]


As you can see, the error happens inside
`tramp-completion-file-name-handler'.  I've debugged it; the last action
I can see is disabling Tramp' file name completion handler, and calling
`load', again. The error does not seem to be caused inside a Tramp
function.

As I am not able to debug C sources on W32, I must let it to you. It
seems to be something inside FLoad.

> However I wonder why those files at all are interesting for tramp. I
> know little about tramp, but does not remote file names always start
> with something like "/ssh:", "/ftp:", "/telnet:" etc?
>
> If so why look for file names starting with "c:/"?

Some packages ( I don't remember which ones) do add the volume letter to
file names, which haven't one. Tramp silently removes the volume letter
then, and continues.

> And why does this file handler at all jump in during `load'? Shouldn't
> tramp-completion-file-name-handler just come in during completion? It
> should be invoked when the operations are file-name-completion or
> file-name-all-completion (see
> tramp-completion-file-name-handler-alist), but are these operations
> used during `load'?

In `tramp-completion-file-name-handler' it is checked, whether Emacs is
in (file name) completion mode. If not, Tramp's file name completion
handler is disables, and the function is re-called, recursively.

Best regards, Michael.

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

[Drew Adams]

> I could break down the example file from Drew to the 
> following contents:

[FWIW, I had trouble following your mail, Michael, since, at least with my mail
client, Outlook, all of the message contents except the sentence above, took the
form of separate attachments.]

You say, "As I am not able to debug C sources on W32, I must let it to you. It
seems to be something inside FLoad."

I don't know who "you" is that you're leaving this to. I hope it's not me (or
Lennart). That is, I hope that someone knowledgeable about this does follow up
and fix it, as it seems to be quite a serious bug.

You are right that the problem seems to be `load', not Tramp. If I do (load
"c:/the-file.el"), then I get the "end-of-file during parsing" error. If I load
exactly the same file from another location, it loads fine: (load
"c:/somewhere/the-file.el").

I hope someone follows up on this. Thx.

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

[Eli Zaretskii]

> From: "Drew Adams" <drew.adams@oracle.com>
> Date: Tue, 19 Jan 2010 09:39:46 -0800
> Cc: 5303@debbugs.gnu.org, 'Chong Yidong' <cyd@stupidchicken.com>
> 
> You are right that the problem seems to be `load', not Tramp. If I do (load
> "c:/the-file.el"), then I get the "end-of-file during parsing" error. If I load
> exactly the same file from another location, it loads fine: (load
> "c:/somewhere/the-file.el").

What is in the-file.el? will an empty file do? if not, what should be
there to reproduce the problem?

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

[Chong Yidong]

Eli Zaretskii <eliz@gnu.org> writes:

>> From: "Drew Adams" <drew.adams@oracle.com>
>> Date: Tue, 19 Jan 2010 09:39:46 -0800
>> Cc: 5303@debbugs.gnu.org, 'Chong Yidong' <cyd@stupidchicken.com>
>> 
>> You are right that the problem seems to be `load', not Tramp. If I do (load
>> "c:/the-file.el"), then I get the "end-of-file during parsing" error. If I load
>> exactly the same file from another location, it loads fine: (load
>> "c:/somewhere/the-file.el").
>
> What is in the-file.el? will an empty file do? if not, what should be
> there to reproduce the problem?

Note by the way that Emacs 23.1 does not display this problem, so if
someone can reproduce this and has access to the repository, please try
to find the offending change by bisecting.

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

[Michael Albinus]

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 "\x1a")))
------

Best regards, Michael.

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

[Drew Adams]

> > You are right that the problem seems to be `load', not 
> > Tramp. If I do (load "c:/the-file.el"), then I get the
> > "end-of-file during parsing" error. If I load
> > exactly the same file from another location, it loads fine: (load
> > "c:/somewhere/the-file.el").
> 
> What is in the-file.el? will an empty file do? if not, what should be
> there to reproduce the problem?

Please read the thread. It is the attachment, `pared-hist', that I sent to the
second mail (the attachment to the first mail, `emacs-history', likewise, but
the second one is much smaller and is sufficient to reproduce the problem).

The file contains a (valid) lisp sexp from my .emacs-history file written by
savehist.el. Put the file in c:/ and you get the eof parsing error when loading.
Put the file somewhere else (e.g. c:/foo/) and you get no error.

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

[Eli Zaretskii]

> From: "Drew Adams" <drew.adams@oracle.com>
> Cc: <michael.albinus@gmx.de>, <lennart.borgman@gmail.com>,
>         <5303@debbugs.gnu.org>, <cyd@stupidchicken.com>
> Date: Tue, 19 Jan 2010 11:37:03 -0800
> 
> > > You are right that the problem seems to be `load', not 
> > > Tramp. If I do (load "c:/the-file.el"), then I get the
> > > "end-of-file during parsing" error. If I load
> > > exactly the same file from another location, it loads fine: (load
> > > "c:/somewhere/the-file.el").
> > 
> > What is in the-file.el? will an empty file do? if not, what should be
> > there to reproduce the problem?
> 
> Please read the thread.

Is it so hard to just tell the bottom line?  Most of the thread was
irrelevant to the real problem, which was just discovered now.  Why
punish me by forcing me to wade through all that?

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

[Drew Adams]

> > Please read the thread.
> 
> Is it so hard to just tell the bottom line?  Most of the thread was
> irrelevant to the real problem, which was just discovered now.  Why
> punish me by forcing me to wade through all that?

Did I not "tell the bottom line"?
Did I write _only_ "please read the thread"?

I pointed you to the file you asked about (and I gave you the recipe again).

Do you need me to attach the file again? I can do that.

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

[Lennart Borgman]

On Tue, Jan 19, 2010 at 2:40 PM, Michael Albinus <michael.albinus@gmx.de> wrote:
> Lennart Borgman <lennart.borgman@gmail.com> writes:
>
>> Yes, it is broken and it has been so for very long time. I just have
>> not understood before that it was in the special case with a file in
>> the root of a w32 drive.


Hi Michael,
Thanks for looking into this.


> I could break down the example file from Drew to the following contents:
>
>
> (setq command-history '((describe-key " ")))


That is very good to know. Thanks. This makes me guess that it is a
decoding problem.


> When enabling traces of all Tramp functions, I see
>
> ======================================================================
> 1 -> tramp-completion-file-name-handler: operation=load args=("c:/emacs-history" nil nil t)
..
> | | 3 <- tramp-completion-file-name-handler: "c:/emacs-history"
> ======================================================================
>
>
> As you can see, the error happens inside
> `tramp-completion-file-name-handler'.


Eh, no. How do I see that ... ;-)

Or do you mean that it does not happen inside
tramp-completion-file-name-handler?


> I've debugged it; the last action
> I can see is disabling Tramp' file name completion handler, and calling
> `load', again. The error does not seem to be caused inside a Tramp
> function.
>
> As I am not able to debug C sources on W32, I must let it to you. It
> seems to be something inside FLoad.


Thanks. I have looked into it a bit. I think that what happens is that
Vload_source_file_function is not called when the file is in the root
of a w32 drive. Instead the alternate readloop further down in `load'
(in lread.c) is called.

I have not debugged it, this is what I have done instead:

  (defun temp-load (fullname file &optional noerror nomessage)
    (message "temp-load %s" fullname)
    (load-with-code-conversion fullname file noerror nomessage))
  (setq force-load-messages t)
  (setq load-source-file-function 'temp-load)

  (load "c:/emacs-history")
  (load "c:/dl/emacs-history")

I got this output

  Loading c:/emacs-history.el (source)...
  apply: End of file during parsing: c:/emacs-history.el

  temp-load c:/dl/emacs-history
  Loading c:/dl/emacs-history...done

Looking at the code for `load' in lread.c I think the above shows that
loading take different paths in the two cases.

When the file is in the root it is not loaded by
Vload_source_file_function. I am not sure what is happening, but maybe
it goes further down, passing the message statements and further to
the readevalloop.

When the file is not in the root it is loaded by the Vload_source_file function.



>> However I wonder why those files at all are interesting for tramp. I
>> know little about tramp, but does not remote file names always start
>> with something like "/ssh:", "/ftp:", "/telnet:" etc?
>>
>> If so why look for file names starting with "c:/"?
>
> Some packages ( I don't remember which ones) do add the volume letter to
> file names, which haven't one. Tramp silently removes the volume letter
> then, and continues.


But is not that a bug in those packages then? Does not trying to fix
it in Tramp introduce this new bug?


>> And why does this file handler at all jump in during `load'? Shouldn't
>> tramp-completion-file-name-handler just come in during completion? It
>> should be invoked when the operations are file-name-completion or
>> file-name-all-completion (see
>> tramp-completion-file-name-handler-alist), but are these operations
>> used during `load'?
>
> In `tramp-completion-file-name-handler' it is checked, whether Emacs is
> in (file name) completion mode. If not, Tramp's file name completion
> handler is disables, and the function is re-called, recursively.


You mean that tramp-completion-run-real-handler is called when Emacs
is not in file completion mode.

It looks like it is disabling tramp-completion-file-name-handler. But
why is the definition of it inside a (prog ...)?


> Best regards, Michael.
>
>

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

[Eli Zaretskii]

> 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.

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.

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?]

(Are you saying "Huh??" yet?)  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?

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 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.

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

[Eli Zaretskii]

> From: "Drew Adams" <drew.adams@oracle.com>
> Cc: <michael.albinus@gmx.de>, <lennart.borgman@gmail.com>,
>         <5303@debbugs.gnu.org>, <cyd@stupidchicken.com>
> Date: Tue, 19 Jan 2010 11:52:50 -0800
> 
> > > Please read the thread.
> > 
> > Is it so hard to just tell the bottom line?  Most of the thread was
> > irrelevant to the real problem, which was just discovered now.  Why
> > punish me by forcing me to wade through all that?
> 
> Did I not "tell the bottom line"?

No.

> Did I write _only_ "please read the thread"?
> 
> I pointed you to the file you asked about (and I gave you the recipe again).

I asked for the file's contents, not for a pointer.

You said:

> Please read the thread. It is the attachment, `pared-hist', that I sent to the
> second mail (the attachment to the first mail, `emacs-history', likewise, but
> the second one is much smaller and is sufficient to reproduce the problem).

The only reasonable way to find ``the second mail'' in a thread that
is going on for several days is to go to the bug tracker site and
start reading the whole chain from the beginning, then download the
file attachment, etc.  But if I wanted to do that, I wouldn't be
asking for the file's contents, would I?

> Do you need me to attach the file again? I can do that.

No need, Michael solved it by showing me the offending one-liner.

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

[Eli Zaretskii]

> Date: Tue, 19 Jan 2010 23:24:55 +0200
> From: Eli Zaretskii <eliz@gnu.org>
> Cc: 5303@debbugs.gnu.org, cyd@stupidchicken.com
> 
> 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.

Looks like that: in Emacs 23.1, the first call to `load' does not
treat the file as ``magic'': `openp' returns a positive file
descriptor, and we then load the file with load-with-code-conversion,
as expected.  End of story; there's no recursive call to `load' after
disabling Tramp's file-name completion handler, no nothing.

It sounds like it's Tramp-related after all, or maybe TRamp uncovers
some old hidden bug.

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.
>

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

[Michael Albinus]

Eli Zaretskii <eliz@gnu.org> writes:

> 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 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.

Tramp inhibits the file name handler of `load'. `openp' checks for a
file name handler of `file-exists-p', which is not inhibited.

Best regards, Michael.

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

[Lennart Borgman]

On Wed, Jan 20, 2010 at 9:44 AM, Michael Albinus <michael.albinus@gmx.de> wrote:
> Eli Zaretskii <eliz@gnu.org> writes:
>
>> 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 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.
>
> Tramp inhibits the file name handler of `load'. `openp' checks for a
> file name handler of `file-exists-p', which is not inhibited.


But how is tramp-completion-file-name-handler involved there?

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

[Michael Albinus]

Lennart Borgman <lennart.borgman@gmail.com> writes:

>>> 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 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.
>>
>> Tramp inhibits the file name handler of `load'. `openp' checks for a
>> file name handler of `file-exists-p', which is not inhibited.
>
> But how is tramp-completion-file-name-handler involved there?

Still the same game. "C:/the-file.el" matches its regexp in
file-name-handler-alist'.

Best regards, Michael.

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

[Michael Albinus]

Lennart Borgman <lennart.borgman@gmail.com> writes:

>>> However I wonder why those files at all are interesting for tramp. I
>>> know little about tramp, but does not remote file names always start
>>> with something like "/ssh:", "/ftp:", "/telnet:" etc?
>>>
>>> If so why look for file names starting with "c:/"?
>>
>> Some packages ( I don't remember which ones) do add the volume letter to
>> file names, which haven't one. Tramp silently removes the volume letter
>> then, and continues.
>
> But is not that a bug in those packages then? Does not trying to fix
> it in Tramp introduce this new bug?

In theory, you are right. But Tramp supports also GNU Emacs 21, 22, and
XEmacs. This makes it impossible to fix all involved packages.

Best regards, Michael.

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

[Lennart Borgman]

On Wed, Jan 20, 2010 at 9:56 AM, Michael Albinus <michael.albinus@gmx.de> wrote:
> Lennart Borgman <lennart.borgman@gmail.com> writes:
>
>> But how is tramp-completion-file-name-handler involved there?
>
> Still the same game. "C:/the-file.el" matches its regexp in
> file-name-handler-alist'.

Ah, yes. Sorry ;-)

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

[Lennart Borgman]

On Wed, Jan 20, 2010 at 10:00 AM, Michael Albinus
<michael.albinus@gmx.de> wrote:
> Lennart Borgman <lennart.borgman@gmail.com> writes:
>
>>>> However I wonder why those files at all are interesting for tramp. I
>>>> know little about tramp, but does not remote file names always start
>>>> with something like "/ssh:", "/ftp:", "/telnet:" etc?
>>>>
>>>> If so why look for file names starting with "c:/"?
>>>
>>> Some packages ( I don't remember which ones) do add the volume letter to
>>> file names, which haven't one. Tramp silently removes the volume letter
>>> then, and continues.
>>
>> But is not that a bug in those packages then? Does not trying to fix
>> it in Tramp introduce this new bug?
>
> In theory, you are right. But Tramp supports also GNU Emacs 21, 22, and
> XEmacs. This makes it impossible to fix all involved packages.


Ok, but that does perhaps not make it impossible to fix this in GNU Emacs 23?

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

[Michael Albinus]

Michael Albinus <michael.albinus@gmx.de> writes:

> Tramp inhibits the file name handler of `load'. `openp' checks for a
> file name handler of `file-exists-p', which is not inhibited.

The following patch could solve the problem (untested, and I don't know
whether it is TRTTD)

--8<---------------cut here---------------start------------->8---
*** /home/albinus/src/emacs/src/lread.c.~1.422.~	2009-12-08 13:25:31.000000000 +0100
--- /home/albinus/src/emacs/src/lread.c	2010-01-20 10:34:23.000000000 +0100
***************
*** 1487,1493 ****
  	     It's not clear why that was the case and it breaks things like
  	     (load "/bar.el") where the file is actually "/bar.el.gz".  */
  	  string = build_string (fn);
! 	  handler = Ffind_file_name_handler (string, Qfile_exists_p);
  	  if ((!NILP (handler) || !NILP (predicate)) && !NATNUMP (predicate))
              {
  	      if (NILP (predicate))
--- 1487,1496 ----
  	     It's not clear why that was the case and it breaks things like
  	     (load "/bar.el") where the file is actually "/bar.el.gz".  */
  	  string = build_string (fn);
! 	  handler = Ffind_file_name_handler (string,
! 					     NILP (Vinhibit_file_name_operation)
! 					     ? Qfile_exists_p
! 					     : Vinhibit_file_name_operation);
  	  if ((!NILP (handler) || !NILP (predicate)) && !NATNUMP (predicate))
              {
  	      if (NILP (predicate))
--8<---------------cut here---------------end--------------->8---

Best regards, Michael.

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

[Michael Albinus]

Lennart Borgman <lennart.borgman@gmail.com> writes:

>> In theory, you are right. But Tramp supports also GNU Emacs 21, 22, and
>> XEmacs. This makes it impossible to fix all involved packages.
>
> Ok, but that does perhaps not make it impossible to fix this in GNU Emacs 23?

Maybe. But this does apply for Core Emacs packages only; there are still
other packages used in the wild.

And I vaguely remember, that there were problems also with Cygwin Emacs
(but I might be wrong).

Best regards, Michael.

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

[Lennart Borgman]

On Wed, Jan 20, 2010 at 11:13 AM, Michael Albinus
<michael.albinus@gmx.de> wrote:
> Lennart Borgman <lennart.borgman@gmail.com> writes:
>
>>> In theory, you are right. But Tramp supports also GNU Emacs 21, 22, and
>>> XEmacs. This makes it impossible to fix all involved packages.
>>
>> Ok, but that does perhaps not make it impossible to fix this in GNU Emacs 23?
>
> Maybe. But this does apply for Core Emacs packages only; there are still
> other packages used in the wild.
>
> And I vaguely remember, that there were problems also with Cygwin Emacs
> (but I might be wrong).
>
> Best regards, Michael.


I understand your concern, but maybe this is the right time to try to fix it?

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

[Eli Zaretskii]

> From: Michael Albinus <michael.albinus@gmx.de>
> Cc: drew.adams@oracle.com,  lennart.borgman@gmail.com,  5303@debbugs.gnu.org,  cyd@stupidchicken.com
> Date: Wed, 20 Jan 2010 10:45:55 +0100
> 
> ! 	  handler = Ffind_file_name_handler (string,
> ! 					     NILP (Vinhibit_file_name_operation)
> ! 					     ? Qfile_exists_p
> ! 					     : Vinhibit_file_name_operation);

But this assumes that `openp is called for the same file op for which
the value inhibit-file-name-operation was set, doesn't it?  I'm not
sure such an assumption is safe: `openp' is a very popular utility
function, it is called by many Lisp APIs, and could very well be
called by some primitive that has nothing to do with
inhibit-file-name-operation's value.

> > Tramp inhibits the file name handler of `load'. `openp' checks for a
> > file name handler of `file-exists-p', which is not inhibited.

Let me turn the table and ask you: why doesn't Tramp inhibit _all_ its
file name handlers in this case?  IIUC, what we want here is to have
`load' do its thing with a local file.  So which other handlers could
we want to be active in this case?

(There's still a broader issue with the apparent discrepancy between
the handler test in `load' and the similar test in `openp', of
course.)

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

[Michael Albinus]

Eli Zaretskii <eliz@gnu.org> writes:

>> ! 	  handler = Ffind_file_name_handler (string,
>> ! 					     NILP (Vinhibit_file_name_operation)
>> ! 					     ? Qfile_exists_p
>> ! 					     : Vinhibit_file_name_operation);
>
> But this assumes that `openp is called for the same file op for which
> the value inhibit-file-name-operation was set, doesn't it?  I'm not
> sure such an assumption is safe: `openp' is a very popular utility
> function, it is called by many Lisp APIs, and could very well be
> called by some primitive that has nothing to do with
> inhibit-file-name-operation's value.

Then one could set Vinhibit_file_name_operation to Qfile_exists_p in
Fload, before calling openp. Temporarily, of course.

>> > Tramp inhibits the file name handler of `load'. `openp' checks for a
>> > file name handler of `file-exists-p', which is not inhibited.
>
> Let me turn the table and ask you: why doesn't Tramp inhibit _all_ its
> file name handlers in this case?  IIUC, what we want here is to have
> `load' do its thing with a local file.  So which other handlers could
> we want to be active in this case?

`jka-compr-handler', `epa-file-handler' for example. Imagine, you want
to load "C:/this_file.gz" or "C:/this_file.gpg".

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

[Michael Albinus]

Lennart Borgman <lennart.borgman@gmail.com> writes:

> I understand your concern, but maybe this is the right time to try to fix it?

Not during 23.2 pretest.

Best regards, Michael.

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

[Lennart Borgman]

On Wed, Jan 20, 2010 at 1:01 PM, Michael Albinus <michael.albinus@gmx.de> wrote:
> Lennart Borgman <lennart.borgman@gmail.com> writes:
>
>> I understand your concern, but maybe this is the right time to try to fix it?
>
> Not during 23.2 pretest.
>
> Best regards, Michael.


Perhaps not, you know more about what is involved. But the pretest is
for fixing bugs and this is a bug.

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

[Michael Albinus]

Lennart Borgman <lennart.borgman@gmail.com> writes:

> Perhaps not, you know more about what is involved. But the pretest is
> for fixing bugs and this is a bug.

???

Dropping the volume letter if it hurts Tramp, is not a bug.

Identifying packages, which add the volume letter silently, might be bug
hunting, but I don't know whether we shall spent the effort just now. It
will bring instabilities to Tramp, when we change it.

If you want to check yourself, you could change `tramp-root-regexp' in
your tramp.el.

Best regards, Michael.

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

[Lennart Borgman]

On Wed, Jan 20, 2010 at 1:15 PM, Michael Albinus <michael.albinus@gmx.de> wrote:
> Lennart Borgman <lennart.borgman@gmail.com> writes:
>
>> Perhaps not, you know more about what is involved. But the pretest is
>> for fixing bugs and this is a bug.
>
> ???


Eh, my bad. I thought yesterday that dropping the volume letter might
be the cause of the bug when loading c:/emacs-history. But that does
not seem to be the case. Sorry.


> Dropping the volume letter if it hurts Tramp, is not a bug.
>
> Identifying packages, which add the volume letter silently, might be bug
> hunting, but I don't know whether we shall spent the effort just now. It
> will bring instabilities to Tramp, when we change it.
>
> If you want to check yourself, you could change `tramp-root-regexp' in
> your tramp.el.
>
> Best regards, Michael.
>

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

[Drew Adams]

> > maybe this is the right time to try to fix it?
> 
> Not during 23.2 pretest.

Dunno if you are speaking only of a _particular_ fix proposal or generally.

Obviously, speaking generally, this needs to be fixed before the 23.2 release,
since it is a _regression_. The problem never manifested itself previously
(Emacs 20 through 23.1 release), even if, as some have suggested, 23.1 perhaps
dodged a bullet.

AFAIK, the default location on Windows for files such as .emacs-history (for
savehist.el) is C:/ (or perhaps $HOME, defaulting to C:/). 

Other Windows users _will_ hit this bug. Some solution needs to be found now,
IMO.

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

[Michael Albinus]

Drew Adams <drew.adams@oracle.com> writes:

>> > maybe this is the right time to try to fix it?
>> 
>> Not during 23.2 pretest.
>
> Dunno if you are speaking only of a _particular_ fix proposal or generally.

I'm speaking about dropping the volume letter in Tramp. This is NOT a
bug; Tramp behaves correct according to the spec. It handles
`inhibit-file-name-operation' as it should.

> Obviously, speaking generally, this needs to be fixed before the 23.2 release,
> since it is a _regression_. The problem never manifested itself previously
> (Emacs 20 through 23.1 release), even if, as some have suggested, 23.1 perhaps
> dodged a bullet.

On my colleague's PC, I could reproduce the problem with

GNU Emacs 23.0.60.1 (i386-mingw-nt5.1.2600)
 of 2009-01-15 on LENNART-69DE564

It doesn't seem to be a regression wrt Emacs 23.1, at least.

Best regards, Michael.

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

[Drew Adams]

> > Obviously, speaking generally, this needs to be fixed 
> > before the 23.2 release, since it is a _regression_.
> >
> > The problem never manifested itself previously
> > (Emacs 20 through 23.1 release), even if, as some have 
> > suggested, 23.1 perhaps dodged a bullet.
> 
> On my colleague's PC, I could reproduce the problem with
> GNU Emacs 23.0.60.1 (i386-mingw-nt5.1.2600)
>  of 2009-01-15 on LENNART-69DE564
> 
> It doesn't seem to be a regression wrt Emacs 23.1, at least.

I cannot reproduce the problem using the 23.1 _release_.
So it is a regression wrt what we have actually released to users, AFAICT.

In GNU Emacs 23.1.1 (i386-mingw-nt5.1.2600)
 of 2009-07-29 on SOFT-MJASON
Windowing system distributor `Microsoft Corp.', version 5.1.2600
configured using `configure --with-gcc (4.4)'

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

[Eli Zaretskii]

> From: "Drew Adams" <drew.adams@oracle.com>
> Cc: "'Eli Zaretskii'" <eliz@gnu.org>, "'Chong Yidong'" <cyd@stupidchicken.com>,
>         <5303@debbugs.gnu.org>
> Date: Wed, 20 Jan 2010 07:32:03 -0800
> 
> Obviously, speaking generally, this needs to be fixed before the 23.2 release,
> since it is a _regression_. The problem never manifested itself previously
> (Emacs 20 through 23.1 release), even if, as some have suggested, 23.1 perhaps
> dodged a bullet.
> 
> AFAIK, the default location on Windows for files such as .emacs-history (for
> savehist.el) is C:/ (or perhaps $HOME, defaulting to C:/). 
> 
> Other Windows users _will_ hit this bug. Some solution needs to be found now,
> IMO.

Don't worry, it will be fixed, one way or the other.

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

[Eli Zaretskii]

> From: Michael Albinus <michael.albinus@gmx.de>
> Cc: "'Lennart Borgman'" <lennart.borgman@gmail.com>,
>         "'Eli Zaretskii'" <eliz@gnu.org>,
>         "'Chong Yidong'" <cyd@stupidchicken.com>,
>         "5303\@debbugs.gnu.org" <5303@debbugs.gnu.org>
> Date: Wed, 20 Jan 2010 16:41:55 +0100
> 
> > Obviously, speaking generally, this needs to be fixed before the 23.2 release,
> > since it is a _regression_. The problem never manifested itself previously
> > (Emacs 20 through 23.1 release), even if, as some have suggested, 23.1 perhaps
> > dodged a bullet.
> 
> On my colleague's PC, I could reproduce the problem with
> 
> GNU Emacs 23.0.60.1 (i386-mingw-nt5.1.2600)
>  of 2009-01-15 on LENNART-69DE564
> 
> It doesn't seem to be a regression wrt Emacs 23.1, at least.

It doesn't happen for me with stock Emacs 23.1, so it _is_ a
regression.

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

[Lennart Borgman]

On Wed, Jan 20, 2010 at 7:18 PM, Eli Zaretskii <eliz@gnu.org> wrote:
>
> It doesn't happen for me with stock Emacs 23.1, so it _is_ a
> regression.

A quick check with my old binaries shows that 2009-08-04 works but
2009-09-15 fails.

Some change in that period added tramp-completion-file-name-handler to
file-name-handler-alist.
Removing it from there makes (load "c:/emacs-history") succeed again.

Note however that tramp-completion-file-name-handler run the real handler.
So it looks more like it is the presence of the pattern

  ("\\`\\([a-zA-Z]:\\)?/[^/]*\\'" . tramp-completion-file-name-handler)

in file-name-handler-alist that causes the problem. Which probably
means that the problem is in `load' as Eli has suggested.

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

[Eli Zaretskii]

> From: Lennart Borgman <lennart.borgman@gmail.com>
> Date: Thu, 21 Jan 2010 02:02:49 +0100
> Cc: Michael Albinus <michael.albinus@gmx.de>, drew.adams@oracle.com, cyd@stupidchicken.com, 
> 	5303@debbugs.gnu.org
> 
> On Wed, Jan 20, 2010 at 7:18 PM, Eli Zaretskii <eliz@gnu.org> wrote:
> >
> > It doesn't happen for me with stock Emacs 23.1, so it _is_ a
> > regression.
> 
> A quick check with my old binaries shows that 2009-08-04 works but
> 2009-09-15 fails.
> 
> Some change in that period added tramp-completion-file-name-handler to
> file-name-handler-alist.
> Removing it from there makes (load "c:/emacs-history") succeed again.
> 
> Note however that tramp-completion-file-name-handler run the real handler.
> So it looks more like it is the presence of the pattern
> 
>   ("\\`\\([a-zA-Z]:\\)?/[^/]*\\'" . tramp-completion-file-name-handler)
> 
> in file-name-handler-alist that causes the problem. Which probably
> means that the problem is in `load' as Eli has suggested.

To fix the problem (which I also believe is in `load' or maybe between
`load' and `openp'), I need to better understand some of the code in
`load'.

Would someone who knows please help me answer the following questions
regarding the fragment below?  Stefan? Yidong? Richard? anyone?

The questions are:

  . Why do we test for ".elc" OR non-zero return value from
    safe_to_load_p?  What is the purpose of handling files without the
    .elc extension as byte-compiled?

  . Was safe_to_load_p intended to return non-zero value for invalid
    file descriptors such as -2?


  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!  */
    {
      if (fd != -2)
	{
	  struct stat s1, s2;
	  int result;

	  GCPRO3 (file, found, hist_file_name);

	  if (version < 0
	      && ! (version = safe_to_load_p (fd)))
	    {
	      safe_p = 0;
	      if (!load_dangerous_libraries)
		{
		  if (fd >= 0)
		    emacs_close (fd);
		  error ("File `%s' was not compiled in Emacs",
			 SDATA (found));
		}
	      else if (!NILP (nomessage) && !force_load_messages)
		message_with_string ("File `%s' not compiled in Emacs", found, 1);
	    }

	  compiled = 1;

	  efound = ENCODE_FILE (found);

#ifdef DOS_NT
	  fmode = "rb";
#endif /* DOS_NT */
	  stat ((char *)SDATA (efound), &s1);
	  SSET (efound, SBYTES (efound) - 1, 0);
	  result = stat ((char *)SDATA (efound), &s2);
	  SSET (efound, SBYTES (efound) - 1, 'c');

	  if (result >= 0 && (unsigned) s1.st_mtime < (unsigned) s2.st_mtime)
	    {
	      /* Make the progress messages mention that source is newer.  */
	      newer = 1;

	      /* If we won't print another message, mention this anyway.  */
	      if (!NILP (nomessage) && !force_load_messages)
		{
		  Lisp_Object msg_file;
		  msg_file = Fsubstring (found, make_number (0), make_number (-1));
		  message_with_string ("Source file `%s' newer than byte-compiled file",
				       msg_file, 1);
		}
	    }
	  UNGCPRO;
	}
    }
  else
    {
      /* 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);
	}
    }

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

[Chong Yidong]

Eli Zaretskii <eliz@gnu.org> writes:

>   . Why do we test for ".elc" OR non-zero return value from
>     safe_to_load_p?  What is the purpose of handling files without the
>     .elc extension as byte-compiled?

I'm no expert on this part of the code, but I think this is to allow
Emacs to load elc files even if the filename does not end with .elc.  In
other words, something roughly similar to how we now handle image files.

>   . Was safe_to_load_p intended to return non-zero value for invalid
>     file descriptors such as -2?

I think that is a bug.

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

[Chong Yidong]

Chong Yidong <cyd@stupidchicken.com> writes:

>>   . Was safe_to_load_p intended to return non-zero value for invalid
>>     file descriptors such as -2?
>
> I think that is a bug.

Does it fix the bug if safe_to_load_p returns 0 when the call to
emacs_read on line 895 returns a value <= 0?