bug#32304: 27.0.50; tramp-tests issue with double slash

classic Classic list List threaded Threaded
27 messages Options
12
Reply | Threaded
Open this post in threaded view
|

bug#32304: 27.0.50; tramp-tests issue with double slash

Ken Brown-6
The following parts of tramp-test04-substitute-in-file-name are failing
on Cygwin (and probably also on MS-Windows, but I can't easily test
that):

(should
    (string-equal
     (substitute-in-file-name "/method:host://foo") "/method:host:/foo"))
(should
    (string-equal (substitute-in-file-name "/method:host:/path///foo")
"/foo"))
(should
    (string-equal (substitute-in-file-name "/method:host://~foo") "/~foo"))

Here are the corresponding evaluations:

(substitute-in-file-name "/method:host://foo")
"/method:host://foo"

(substitute-in-file-name "/method:host:/path///foo")
"//foo"

(substitute-in-file-name "/method:host://~foo")
"//~foo"

This is a consequence of the fact that Tramp calls
substitute-in-file-name on the local part of the file name:

(substitute-in-file-name "//foo")
"//foo"

(substitute-in-file-name "/path///foo")
"//foo"

(substitute-in-file-name "//~foo")
"//~foo"

"//" has a special meaning at the beginning of a file name on Cygwin and
MS-Windows; see the comment near the beginning of
search_embedded_absfilename.  So I don't think there's a real bug here.
Maybe the expected result of the test should simply be adjusted, as in
the attached patch.  Someone who builds on MS-Windows would have to
check this.

Ken


0001-Adjust-expected-results-on-a-tramp-test.patch (2K) Download Attachment
Reply | Threaded
Open this post in threaded view
|

bug#32304: 27.0.50; tramp-tests issue with double slash

Michael Albinus
Ken Brown <[hidden email]> writes:

Hi Ken,

> "//" has a special meaning at the beginning of a file name on Cygwin and
> MS-Windows; see the comment near the beginning of
> search_embedded_absfilename.  So I don't think there's a real bug here.
> Maybe the expected result of the test should simply be adjusted, as in
> the attached patch.  Someone who builds on MS-Windows would have to
> check this.

Thanks for the report. However, your patch seems just to hide the
problem in tramp-tests.el. Wouldn't it be better to fix this?

What about the appended patch?

> Ken

Best regards, Michael.


attachment0 (866 bytes) Download Attachment
Reply | Threaded
Open this post in threaded view
|

bug#32304: 27.0.50; tramp-tests issue with double slash

Ken Brown-6
On 7/29/2018 12:27 PM, Michael Albinus wrote:

Hi Michael,
> Thanks for the report. However, your patch seems just to hide the
> problem in tramp-tests.el. Wouldn't it be better to fix this?

Yes, absolutely.

> What about the appended patch?

I think something further must be needed.  The test still fails in the
same way.

Best regards,

Ken



Reply | Threaded
Open this post in threaded view
|

bug#32304: 27.0.50; tramp-tests issue with double slash

Michael Albinus
Ken Brown <[hidden email]> writes:

> Hi Michael,

Hi Ken,

>> What about the appended patch?
>
> I think something further must be needed.  The test still fails in the
> same way.

OK, needs an MS Windows machine for test. Will do when possible;
officially, I'm on vacation this week. (Don't tell my wife my presence
in emacs-devel :-)

> Best regards,
>
> Ken

Best regards, Michael.



Reply | Threaded
Open this post in threaded view
|

bug#32304: 27.0.50; tramp-tests issue with double slash

Ken Brown-6
On 7/29/2018 12:55 PM, Michael Albinus wrote:
> OK, needs an MS Windows machine for test. Will do when possible;
> officially, I'm on vacation this week. (Don't tell my wife my presence
> in emacs-devel :-)

Your secret is safe with me.  Enjoy your vacation.

Ken




Reply | Threaded
Open this post in threaded view
|

bug#32304: 27.0.50; tramp-tests issue with double slash

Eli Zaretskii
In reply to this post by Michael Albinus
> From: Michael Albinus <[hidden email]>

> Date: Sun, 29 Jul 2018 18:27:40 +0200
> Cc: [hidden email]
>
> > "//" has a special meaning at the beginning of a file name on Cygwin and
> > MS-Windows; see the comment near the beginning of
> > search_embedded_absfilename.  So I don't think there's a real bug here.
> > Maybe the expected result of the test should simply be adjusted, as in
> > the attached patch.  Someone who builds on MS-Windows would have to
> > check this.
>
> Thanks for the report. However, your patch seems just to hide the
> problem in tramp-tests.el. Wouldn't it be better to fix this?
>
> What about the appended patch?
Doesn't seem to help here, in the native MS-Windows build; log
attached.

Can you tell how binding system-type was supposed to solve this?


tramp-tests.log (14K) Download Attachment
Reply | Threaded
Open this post in threaded view
|

bug#32304: 27.0.50; tramp-tests issue with double slash

Michael Albinus
Eli Zaretskii <[hidden email]> writes:

Hi Eli,

> Can you tell how binding system-type was supposed to solve this?

Some functions behave differently depending the system type Emacs is
running on. See for example `shell-quote-argument':

(shell-quote-argument "String with quotation mark \"")

=> "String\\ with\\ quotation\\ mark\\ \\\"" on `gnu/linux'.
=> "^\"String with quotation mark \\^\"^\"" on `windows-nt'

Often, Tramp needs the non-windows kind of result for the remote
host. So it is a common technique in Tramp to force this with
"(let ((system-type 'not-windows)) ...)".

Unfortunately, it doesn't work with `substitute-in-file-name'.  I will
look for another solution.

Best regards, Michael.



Reply | Threaded
Open this post in threaded view
|

bug#32304: 27.0.50; tramp-tests issue with double slash

Michael Albinus
In reply to this post by Ken Brown-6
Ken Brown <[hidden email]> writes:

> Hi Michael,

Hi Ken,

> I think something further must be needed.  The test still fails in the
> same way.

There's another patch, could you pls check?

> Best regards,
>
> Ken

Best regards, Michael.


attachment0 (1K) Download Attachment
Reply | Threaded
Open this post in threaded view
|

bug#32304: 27.0.50; tramp-tests issue with double slash

Ken Brown-6
On 7/30/2018 6:43 AM, Michael Albinus wrote:

Hi Michael,

> There's another patch, could you pls check?

Yes, that fixes it on Cygwin.  Thanks.

Best regards,

Ken




Reply | Threaded
Open this post in threaded view
|

bug#32304: 27.0.50; tramp-tests issue with double slash

Eli Zaretskii
> From: Ken Brown <[hidden email]>
> Date: Mon, 30 Jul 2018 08:17:48 -0400
> Cc: [hidden email]
>
> On 7/30/2018 6:43 AM, Michael Albinus wrote:
>
> Hi Michael,
>
> > There's another patch, could you pls check?
>
> Yes, that fixes it on Cygwin.  Thanks.

The MinGW build also passes, but the other two still fail:

  2 unexpected results:
     FAILED  tramp-test43-auto-load
     FAILED  tramp-test44-unload

Should they be skipped?

Thanks.



Reply | Threaded
Open this post in threaded view
|

bug#32304: 27.0.50; tramp-tests issue with double slash

Michael Albinus
Eli Zaretskii <[hidden email]> writes:

Hi Eli,

> The MinGW build also passes, but the other two still fail:
>
>   2 unexpected results:
>      FAILED  tramp-test43-auto-load
>      FAILED  tramp-test44-unload
>
> Should they be skipped?

I would like to see the ERT error messages.

> Thanks.

Best regards, Michael.



Reply | Threaded
Open this post in threaded view
|

bug#32304: 27.0.50; tramp-tests issue with double slash

Eli Zaretskii
> From: Michael Albinus <[hidden email]>

> Cc: Ken Brown <[hidden email]>,  [hidden email]
> Date: Mon, 30 Jul 2018 22:50:39 +0200
>
> > The MinGW build also passes, but the other two still fail:
> >
> >   2 unexpected results:
> >      FAILED  tramp-test43-auto-load
> >      FAILED  tramp-test44-unload
> >
> > Should they be skipped?
>
> I would like to see the ERT error messages.
The full log is attached.


tramp-tests.log (12K) Download Attachment
Reply | Threaded
Open this post in threaded view
|

bug#32304: 27.0.50; tramp-tests issue with double slash

Michael Albinus
In reply to this post by Ken Brown-6
Ken Brown <[hidden email]> writes:

> Hi Michael,

Hi Ken,

>> There's another patch, could you pls check?
>
> Yes, that fixes it on Cygwin.  Thanks.

Thanks for testing (also to Eli)! I've committed the patch to master.

> Best regards,
>
> Ken

Best regards, Michael.



Reply | Threaded
Open this post in threaded view
|

bug#32304: 27.0.50; tramp-tests issue with double slash

Michael Albinus
In reply to this post by Eli Zaretskii
Eli Zaretskii <[hidden email]> writes:

>> > The MinGW build also passes, but the other two still fail:
>> >
>> >   2 unexpected results:
>> >      FAILED  tramp-test43-auto-load
>> >      FAILED  tramp-test44-unload
>> >
>> > Should they be skipped?
>>
>> I would like to see the ERT error messages.
>
> The full log is attached.

Thanks. I will work on it when I have access to an MS Windows machine;
likely next week.

Best regards, Michael.



Reply | Threaded
Open this post in threaded view
|

bug#32304: 27.0.50; tramp-tests issue with double slash

Andy Moreton-3
In reply to this post by Ken Brown-6
On Sun 29 Jul 2018, Michael Albinus wrote:

> Ken Brown <[hidden email]> writes:
>
> Hi Ken,
>
>> "//" has a special meaning at the beginning of a file name on Cygwin and
>> MS-Windows; see the comment near the beginning of
>> search_embedded_absfilename.  So I don't think there's a real bug here.

POSIX specifies that "//" is handled in an implementation defined
manner, so Cygwin and Windows are not really special.

See POSIX section 4.13 Pathname Resolution:

    A pathname consisting of a single <slash> shall resolve to the root
    directory of the process. A null pathname shall not be successfully
    resolved. If a pathname begins with two successive <slash> characters,
    the first component following the leading <slash> characters may be
    interpreted in an implementation-defined manner, although more than two
    leading <slash> characters shall be treated as a single <slash>
    character.

<http://pubs.opengroup.org/onlinepubs/9699919799/basedefs/V1_chap04.html#tag_04_13>





Reply | Threaded
Open this post in threaded view
|

bug#32304: 27.0.50; tramp-tests issue with double slash

Michael Albinus
Andy Moreton <[hidden email]> writes:

Hi Andy,

> POSIX specifies that "//" is handled in an implementation defined
> manner, so Cygwin and Windows are not really special.
>
> See POSIX section 4.13 Pathname Resolution:
>
>     A pathname consisting of a single <slash> shall resolve to the root
>     directory of the process. A null pathname shall not be successfully
>     resolved. If a pathname begins with two successive <slash> characters,
>     the first component following the leading <slash> characters may be
>     interpreted in an implementation-defined manner, although more than two
>     leading <slash> characters shall be treated as a single <slash>
>     character.

But the Emacs implementation handles only Cygwin and MS Windows as
special. This is POSIX conform, because it declares this as
implementation specific.

The question is, whether Tramp should follow this. It does

(substitute-in-file-name "/ssh::/a///b") => "/b"

Maybe, it shall return on Cygwin and MS-Windows

(substitute-in-file-name "/ssh::/a///b") => "//b"

Don't know. Is this too sophisticated?

Best regards, Michael.



Reply | Threaded
Open this post in threaded view
|

bug#32304: 27.0.50; tramp-tests issue with double slash

Ken Brown-6
On 7/31/2018 4:57 PM, Michael Albinus wrote:
> The question is, whether Tramp should follow this. It does
>
> (substitute-in-file-name "/ssh::/a///b") => "/b"

This seems right to me.  After all, the repeated slash is not at the
beginning of the (total) file name.

> Maybe, it shall return on Cygwin and MS-Windows
>
> (substitute-in-file-name "/ssh::/a///b") => "//b"

I can't think of any situation where this would be useful.

Ken




Reply | Threaded
Open this post in threaded view
|

bug#32304: 27.0.50; tramp-tests issue with double slash

Michael Albinus
Ken Brown <[hidden email]> writes:

> I can't think of any situation where this would be useful.

OK, I don't do anything.

> Ken

Best regards, Michael.



Reply | Threaded
Open this post in threaded view
|

bug#32304: 27.0.50; tramp-tests issue with double slash

Michael Albinus
In reply to this post by Eli Zaretskii
Eli Zaretskii <[hidden email]> writes:

Hi Eli,

>> >   2 unexpected results:
>> >      FAILED  tramp-test43-auto-load
>> >      FAILED  tramp-test44-unload
>> >
>> > Should they be skipped?

I cannot find any MS Windows build of Emacs 27. So I have used the
Cygwin Emacs 27.0.50 I've got recently from Ken.

> The full log is attached.
>
> Test tramp-test43-auto-load condition:
>     (ert-test-failed
>      ((should
>        (string-match "Tramp loaded: t[
> ]+"
>     (shell-command-to-string ...)))
>       :form
>       (string-match "Tramp loaded: t[
> ]+" "Tramp loaded: nil
> ")
>       :value nil))
>    FAILED  59/63  tramp-test43-auto-load (0.203125 sec)

This error didn't happen with the Cygnus Emacs :-(

> Test tramp-test44-unload condition:
>     (ert-test-failed "`tramp-register-archive-file-name-handler' still bound")
>    FAILED  63/63  tramp-test44-unload (0.359375 sec)

Cygnus Emacs has reported also an error for this test, but with a
different message. I have fixed this and committed to master, maybe it
is fixed for you as well.

Anyway, for further investigations I'll need an Emacs 27 for MS
Windows. Do you know where I could get a recent one? I have found only
Emacs 26.1 for MS Windows.

Best regards, Michael.



Reply | Threaded
Open this post in threaded view
|

bug#32304: 27.0.50; tramp-tests issue with double slash

Eli Zaretskii
> From: Michael Albinus <[hidden email]>
> Cc: [hidden email],  [hidden email]
> Date: Wed, 08 Aug 2018 15:48:07 +0200
>
> Cygnus Emacs has reported also an error for this test, but with a
> different message. I have fixed this and committed to master, maybe it
> is fixed for you as well.

Thanks, will do.

> Anyway, for further investigations I'll need an Emacs 27 for MS
> Windows. Do you know where I could get a recent one?

How recent must "recent" be?  The last snapshot here:

  https://alpha.gnu.org/gnu/emacs/pretest/windows/emacs-27/

is from May 4.



12