bug#9311: 23.3.50; Can't load some byte-compiled files on Windows

bug#9311: 23.3.50; Can't load some byte-compiled files on Windows

[Kazuhiro Ito]

I wrote c:/test.el as below and byte-compiled it.

(defun test ()
  (let ((l 1))
    (dotimes (i 100)
      nil)))

When I load it by load function with absolute path as below, I can't
load it and below message is displayed.  But it doesn't raise error.

(load "c:/test")
-> End of file during parsing: c:/test.elc

It doesn't occur with relative path from home directory or Linux
environment.  I confirmed it occurs with Windows environment and
absolute path, and it depends on the content of .el file (i.e. not all
.elc files can't be read).  It also occurs with pre-compiled binary
(*1).

(*1) http://ftp.gnu.org/pub/gnu/emacs/windows/emacs-23.3-bin-i386.zip

-- 
Kazuhiro Ito

bug#9311: 23.3.50; Can't load some byte-compiled files on Windows

[Eli Zaretskii]

> Date: Wed, 17 Aug 2011 09:24:43 +0900
> From: Kazuhiro Ito <kzhr@d1.dion.ne.jp>
> 
> I wrote c:/test.el as below and byte-compiled it.
> 
> (defun test ()
>   (let ((l 1))
>     (dotimes (i 100)
>       nil)))
> 
> When I load it by load function with absolute path as below, I can't
> load it and below message is displayed.  But it doesn't raise error.
> 
> (load "c:/test")
> -> End of file during parsing: c:/test.elc

This happens because Emacs thinks this file is remote.  It is almost a
duplicate of bug#5303, except that 5303 talked about files without an
extension, while this one is specific to .elc files.

I fixed this in revision 105479 on the trunk, by handling the DOS_NT
specific situation where `openp' returns -2, but there's no `load'
handler for the file.

bug#9311: 23.3.50; Can't load some byte-compiled files on Windows

[Michael Albinus]

Hi Eli,

I'm on a bike tour these days, being back end of next week. I'll have a look on this then.

C:/test shouldn't be regarded as remote. 

Best regards,  Michael.

bug#9311: 23.3.50; Can't load some byte-compiled files on Windows

[Michael Albinus]

Eli Zaretskii <eliz@gnu.org> writes:

> This happens because Emacs thinks this file is remote.  It is almost a
> duplicate of bug#5303, except that 5303 talked about files without an
> extension, while this one is specific to .elc files.

In the discussion of bug#5303 there was the proposal to remove the
volume letter from `tramp-completion-file-name-regexp'. This was
postponed, because we were in pretest phase.

But we haven't done it after that release.

Shall we try it now? Somehow, we are still before a pretest ...

Best regards, Michael.

bug#9311: 23.3.50; Can't load some byte-compiled files on Windows

[Stefan Monnier]

>> This happens because Emacs thinks this file is remote.  It is almost a
>> duplicate of bug#5303, except that 5303 talked about files without an
>> extension, while this one is specific to .elc files.
> In the discussion of bug#5303 there was the proposal to remove the
> volume letter from `tramp-completion-file-name-regexp'. This was
> postponed, because we were in pretest phase.
> But we haven't done it after that release.
> Shall we try it now? Somehow, we are still before a pretest ...

Coulf you post a sample patch, together with a discussion of the impact?


        Stefan

bug#9311: 23.3.50; Can't load some byte-compiled files on Windows

[Michael Albinus]

Stefan Monnier <monnier@iro.umontreal.ca> writes:

>> In the discussion of bug#5303 there was the proposal to remove the
>> volume letter from `tramp-completion-file-name-regexp'. This was
>> postponed, because we were in pretest phase.
>> But we haven't done it after that release.
>> Shall we try it now? Somehow, we are still before a pretest ...
>
> Coulf you post a sample patch, together with a discussion of the impact?

Sometimes (I cannot reproduce it persistently), there are situations
that `expand-file-name' prefixes a remote file name by a volume
letter. Happens on win32 only, of course, and confuses Tramp.

In order to let Tramp continue its work, we have accepted remote file
names with a leading volume letter, which are removed by Tramp
then. This worked almost OK, except such cases as discussed in the
current (and earlier) bug reports.

Eli did suggest to remove this hack from Tramp, and to see during the
test where the unexpected volume letter prefix happens. Problems shall
be fixed there.

The following patch does it, in its final version there could be some
additional minor polishing.

--8<---------------cut here---------------start------------->8---
*** /home/albinus/src/tramp/lisp/tramp.el.~2.843~	2011-08-30 17:10:16.058151065 +0200
--- /home/albinus/src/tramp/lisp/tramp.el	2011-08-30 17:07:38.841961797 +0200
***************
*** 861,873 ****
  Also see `tramp-file-name-structure'.")
  
  ;;;###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'.")
  
  ;;;###autoload
  (defconst tramp-completion-file-name-regexp-unified
--- 861,868 ----
  Also see `tramp-file-name-structure'.")
  
  ;;;###autoload
! (defconst tramp-root-regexp "\\`/"
!   "Beginning of an incomplete Tramp file name.")
  
  ;;;###autoload
  (defconst tramp-completion-file-name-regexp-unified
***************
*** 1503,1509 ****
  but the remote system is Unix, this introduces a superfluous drive
  letter into the file name.  This function removes it."
  	(save-match-data
! 	  (if (string-match tramp-root-regexp name)
  	      (replace-match "/" nil t name)
  	    name)))
  
--- 1498,1504 ----
  but the remote system is Unix, this introduces a superfluous drive
  letter into the file name.  This function removes it."
  	(save-match-data
! 	  (if (string-match "\\`[a-zA-Z]:/" name)
  	      (replace-match "/" nil t name)
  	    name)))
  
--8<---------------cut here---------------end--------------->8---

>         Stefan

Best regards, Michael.

bug#9311: 23.3.50; Can't load some byte-compiled files on Windows

[Stefan Monnier]

> In order to let Tramp continue its work, we have accepted remote file
> names with a leading volume letter, which are removed by Tramp
> then. This worked almost OK, except such cases as discussed in the
> current (and earlier) bug reports.
> Eli did suggest to remove this hack from Tramp, and to see during the
> test where the unexpected volume letter prefix happens. Problems shall
> be fixed there.

Sounds good.  We should fix it right instead of using the current workaround.

> @@ -861,13 +861,8 @@
>  Also see `tramp-file-name-structure'.")
 
>  ;;;###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'.")
> +(defconst tramp-root-regexp "\\`/"
> +  "Beginning of an incomplete Tramp file name.")

Looks OK.

> @@ -1503,7 +1498,7 @@
>  but the remote system is Unix, this introduces a superfluous drive
>  letter into the file name.  This function removes it."
>  	(save-match-data
> -	  (if (string-match tramp-root-regexp name)
> +	  (if (string-match "\\`[a-zA-Z]:/" name)
>  	      (replace-match "/" nil t name)
>  	    name)))
 
Shouldn't tramp-drop-volume-letter disappear completely?


        Stefan


PS: by the way, here's a tiny informational patch about common
subexpression elimination and about how to silence the compiler
without a hack.


=== modified file 'lisp/net/tramp.el'
--- lisp/net/tramp.el	2011-07-24 14:57:37 +0000
+++ lisp/net/tramp.el	2011-08-30 16:27:59 +0000
@@ -871,9 +871,8 @@
 
 ;;;###autoload
 (defconst tramp-completion-file-name-regexp-unified
-  (if (memq system-type '(cygwin windows-nt))
-      (concat tramp-root-regexp "[^/]\\{2,\\}\\'")
-    (concat tramp-root-regexp "[^/]*\\'"))
+  (concat tramp-root-regexp (if (memq system-type '(cygwin windows-nt))
+                                "[^/]\\{2,\\}\\'" "[^/]*\\'"))
   "Value for `tramp-completion-file-name-regexp' for unified remoting.
 GNU Emacs uses a unified filename syntax for Tramp and Ange-FTP.
 See `tramp-file-name-structure' for more explanations.
@@ -1494,9 +1493,9 @@
 (tramp-compat-font-lock-add-keywords
  'emacs-lisp-mode '("\\<tramp-with-progress-reporter\\>"))
 
-(eval-and-compile			;; Silence compiler.
+(defalias 'tramp-drop-volume-letter
   (if (memq system-type '(cygwin windows-nt))
-      (defun tramp-drop-volume-letter (name)
+      (lambda (name)
 	"Cut off unnecessary drive letter from file NAME.
 The functions `tramp-*-handle-expand-file-name' call `expand-file-name'
 locally on a remote file name.  When the local system is a W32 system
@@ -1506,8 +1505,7 @@
 	  (if (string-match tramp-root-regexp name)
 	      (replace-match "/" nil t name)
 	    name)))
-
-    (defalias 'tramp-drop-volume-letter 'identity)))
+    #'identity))
 
 ;;; Config Manipulation Functions:
 

bug#9311: 23.3.50; Can't load some byte-compiled files on Windows

[Eli Zaretskii]

> From: Michael Albinus <michael.albinus@gmx.de>
> Cc: Eli Zaretskii <eliz@gnu.org>,  Kazuhiro Ito <kzhr@d1.dion.ne.jp>,  9311@debbugs.gnu.org
> Date: Tue, 30 Aug 2011 17:26:38 +0200
> 
> The following patch does it, in its final version there could be some
> additional minor polishing.
> 
> --8<---------------cut here---------------start------------->8---
> *** /home/albinus/src/tramp/lisp/tramp.el.~2.843~	2011-08-30 17:10:16.058151065 +0200
> --- /home/albinus/src/tramp/lisp/tramp.el	2011-08-30 17:07:38.841961797 +0200
> ***************
> *** 861,873 ****
>   Also see `tramp-file-name-structure'.")
>   
>   ;;;###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'.")
>   
>   ;;;###autoload
>   (defconst tramp-completion-file-name-regexp-unified
> --- 861,868 ----
>   Also see `tramp-file-name-structure'.")
>   
>   ;;;###autoload
> ! (defconst tramp-root-regexp "\\`/"
> !   "Beginning of an incomplete Tramp file name.")
>   
>   ;;;###autoload
>   (defconst tramp-completion-file-name-regexp-unified
> ***************
> *** 1503,1509 ****
>   but the remote system is Unix, this introduces a superfluous drive
>   letter into the file name.  This function removes it."
>   	(save-match-data
> ! 	  (if (string-match tramp-root-regexp name)
>   	      (replace-match "/" nil t name)
>   	    name)))
>   
> --- 1498,1504 ----
>   but the remote system is Unix, this introduces a superfluous drive
>   letter into the file name.  This function removes it."
>   	(save-match-data
> ! 	  (if (string-match "\\`[a-zA-Z]:/" name)
>   	      (replace-match "/" nil t name)
>   	    name)))
>   
> --8<---------------cut here---------------end--------------->8---

Thanks, this looks good to me.

bug#9311: 23.3.50; Can't load some byte-compiled files on Windows

[Michael Albinus]

Stefan Monnier <monnier@iro.umontreal.ca> writes:

>> Eli did suggest to remove this hack from Tramp, and to see during the
>> test where the unexpected volume letter prefix happens. Problems shall
>> be fixed there.
>
> Sounds good.  We should fix it right instead of using the current workaround.

I've committed the patch (slightly polished, as said). Let's see how it
goes on. I cannot test anything myself, I don't use Windows machines.

> Shouldn't tramp-drop-volume-letter disappear completely?

Unfortunately, not. In tramp-*-handle-expand-file-name, expand-file-name
is called for the local part of a remote file name in order to handle
"./" and "../". On Windows, this prefixes the result with the volume
letter, which must be removed afterwards.

>         Stefan
>
> PS: by the way, here's a tiny informational patch about common
> subexpression elimination and about how to silence the compiler
> without a hack.

Thanks, applied.

Best regards, Michael.