bug#17425: 24.4.50; tramp-cleanup-this-connection on push-button

bug#17425: 24.4.50; tramp-cleanup-this-connection on push-button

[Sam Steingold]

when I push-button with mouse (hoping to visit the url in a browser), I
get the message from tramp

--8<---------------cut here---------------start------------->8---
Tramp: Opening connection for kurush using scp...
Tramp: Sending command `exec ssh   -o ControlPath=/var/folders/5k/3w_vc1qs6pv4k3wvnrwr_5100000gn/T/tramp.16778kYm.%r@%h:%p -o ControlMaster=auto -o ControlPersist=no -e none kurush'
Tramp: Waiting for prompts from remote shell...
Tramp failed to connect.  If this happens repeatedly, try
    `M-x tramp-cleanup-this-connection'
--8<---------------cut here---------------end--------------->8---

and the url (http://google.com) is not visited.

"kurush" is an ssh alias not available on my current network,

I don't see why tramp is activated (1st bug) and why tramp's failure
prevents the browse-url from completing its task (2nd bug).

In GNU Emacs 24.4.50.3 (x86_64-apple-darwin13.1.0, NS apple-appkit-1265.19)
 of 2014-04-30 on sds-macbook-pro.home
Windowing system distributor `Apple', version 10.3.1265
Configured using:
 `configure --with-ns'

Configured features:
IMAGEMAGICK ACL GNUTLS LIBXML2 ZLIB

Important settings:
  value of $LANG: C
  locale-coding-system: utf-8-unix

Major mode: rcirc

Minor modes in effect:
  diff-auto-refine-mode: t
  rcirc-track-minor-mode: t
  which-function-mode: t
  url-handler-mode: t
  show-paren-mode: t
  desktop-save-mode: t
  shell-dirtrack-mode: t
  tooltip-mode: t
  electric-indent-mode: t
  mouse-wheel-mode: t
  use-hard-newlines: t
  menu-bar-mode: t
  file-name-shadow-mode: t
  global-font-lock-mode: t
  font-lock-mode: t
  blink-cursor-mode: t
  auto-composition-mode: t
  auto-encryption-mode: t
  auto-compression-mode: t
  column-number-mode: t
  line-number-mode: t

Recent input:
<down> SPC n u q SPC q o SPC p SPC = q p SPC q p SPC 
q p SPC SPC q p SPC q p SPC q p SPC q p p SPC q p SPC 
q p SPC q SPC p SPC q p SPC q g <down-mouse-2> <mouse-1> 
SPC <select-window> q SPC q SPC SPC q SPC SPC u q SPC 
q SPC <down-mouse-2> <mouse-1> <down-mouse-1> <mouse-1> 
<up> SPC <down> SPC q q SPC <down-mouse-1> <mouse-1> 
C-c C-SPC C-x b D o <tab> C-g C-x d ~ / D o w n <tab> 
C-a C-k ~ / D o <tab> <return> C-s d n s C-s C-s C-s 
<return> C-s u p d C-s C-s <help-echo> <help-echo> 
<down-mouse-1> <mouse-1> <down> <return> z <up> <return> 
<down> <down> <down> <down> <down> <down> <down> <down> 
<down> <down> <down> <down> <down> <down> <down> <down> 
<up> <up> <up> <up> <up> <up> <up> <up> <up> <up> <up> 
<return> C-v C-c C-v C-v C-s r o u e r <backspace> 
<backspace> t e r C-c C-SPC <help-echo> <down-mouse-2> 
<mouse-1> <help-echo> <down-mouse-1> <drag-mouse-1> 
<down-mouse-1> <mouse-1> q <wheel-left> <double-wheel-left> 
<triple-wheel-left> <help-echo> <down-mouse-1> <mouse-1> 
<up> <up> <up> <up> <up> <left> <C-f12> <return> <down-mouse-1> 
<drag-mouse-1> <down-mouse-1> <mouse-1> q M-x t r a 
n <tab> <tab> <tab> <M-backspace> <M-backspace> <M-backspace> 
t r a n o <backspace> <backspace> m p - l <backspace> 
c - c <tab> <return> <tab> <return> <return> M-x t 
r M-x t r a <tab> <return> <tab> <tab> <return> y M-x 
<up> <return> <tab> <return> M-x <up> <return> <down> 
<down> <return> M-x k k u r u <tab> <backspace> <backspace> 
<backspace> <backspace> C-] C-] C-x l C-x k k u r u 
<tab> C-] C-] C-] C-h o t r a C-] C-] C-] <help-echo> 
<down-mouse-2> <mouse-1> M-x M-x r e p o <tab> <re
turn>

Recent messages:
Quit
call-interactively: No recursive edit is in progress [2 times]
Tramp: Opening connection for kurush using scp...
Tramp: Sending command `exec ssh   -o ControlPath=/var/folders/5k/3w_vc1qs6pv4k3wvnrwr_5100000gn/T/tramp.16778kYm.%r@%h:%p -o ControlMaster=auto -o ControlPersist=no -e none kurush'
Tramp: Waiting for prompts from remote shell...
Tramp failed to connect.  If this happens repeatedly, try
    `M-x tramp-cleanup-this-connection'
Tramp: Waiting for prompts from remote shell...failed
Tramp: Opening connection for kurush using scp...failed
byte-code: Tramp failed to connect.  If this happens repeatedly, try
    `M-x tramp-cleanup-this-connection'

Load-path shadows:
None found.

Features:
(shadow bbdb-message mailalias cookie1 emacsbug sendmail tar-mode nndoc
gnus-fun sort gnus-cite smiley shr mm-archive gnus-async gnus-bcklg
gnus-dup qp mail-extr gnus-ml spam spam-stat gnus-uu yenc nndraft nnmh
gnus-agent gnus-srvr gnus-score score-mode nnvirtual gnus-msg gnus-art
mm-uu mml2015 epg-config mm-view mml-smime smime dig mailcap utf-7
gnus-cache gnus-sum bbdb-gnus gnutls nntp gnus-group gnus-undo
gnus-start gnus-cloud nnimap nnmail mail-source utf7 netrc nnoo
parse-time gnus-spec gnus-int gnus-range gnus-win grep etags remember
whitespace bug-reference autoconf autoconf-mode vc-bzr vc-sccs vc-svn
vc-cvs vc-rcs derived ispell log-edit message rfc822 mml mml-sec
mm-decode mm-bodies mm-encode mail-parse rfc2231 rfc2047 rfc2045
ietf-drums gmm-utils mailheader smerge-mode eieio-opt find-func log-view
pcvs-util diff-mode disp-table network-stream starttls tls cal-move
cal-x cal-china cal-bahai cal-islam cal-julian holidays hol-loaddefs
cal-iso cal-hebrew lunar solar cal-dst appt diary-lib diary-loaddefs
cal-menu calendar cal-loaddefs jka-compr nroff-mode tramp-cmds
time-stamp pp dabbrev skeleton cl-indent apropos tramp-cache tramp-sh
misearch multi-isearch mule-util help-mode debug make-mode sgml-mode
dired-aux vlf vlf-base dired noutline outline cc-langs cc-mode cc-fonts
cc-guess cc-menus cc-cmds cc-styles cc-align cc-engine sql view pig-mode
python vc-git vc-dir ewoc vc vc-dispatcher sh-script smie vc-hg
conf-mode add-log package midnight warnings gnus gnus-ems nnheader
mail-utils wid-edit bbdb-mua bbdb-com crm mailabbrev bbdb-loaddefs bbdb
bbdb-site timezone rcirc server which-func imenu url-handlers url-parse
url-vars paren help-at-pt desktop frameset cus-start cus-load ido
ess-toolbar ess-mouse mouseme thingatpt browse-url ess-menu ess-swv
ess-noweb ess-noweb-font-lock-mode ess-bugs-l essd-els ess-sas-d
ess-sas-l ess-sas-a ess-sta-d ess-sta-l cc-vars cc-defs make-regexp
ess-sp6-d ess-sp3-d ess-julia ess-r-d ess-tracebug compile tramp
tramp-compat auth-source eieio byte-opt bytecomp byte-compile cconv
eieio-core gnus-util mm-util mail-prsvr password-cache tramp-loaddefs
trampver shell pcomplete format-spec ess-roxy advice easy-mmode hideshow
ess-help info reporter ess-developer ess-r-args eldoc help-fns ess-s-l
speedbar sb-image ezimage dframe ess ess-inf comint ansi-color ring
ess-mode ess-noweb-mode edmacro kmacro ess-utils ess-custom cl-macs
executable easymenu ess-compat ess-site cl gv cl-loaddefs cl-lib
time-date tooltip electric uniquify ediff-hook vc-hooks lisp-float-type
mwheel ns-win tool-bar dnd fontset image regexp-opt fringe
tabulated-list newcomment lisp-mode prog-mode register page menu-bar
rfn-eshadow timer select scroll-bar mouse jit-lock font-lock syntax
facemenu font-core frame cham georgian utf-8-lang misc-lang vietnamese
tibetan thai tai-viet lao korean japanese hebrew greek romanian slovak
czech european ethiopic indian cyrillic chinese case-table epa-hook
jka-cmpr-hook help simple abbrev minibuffer nadvice loaddefs button
faces cus-face macroexp files text-properties overlay sha1 md5 base64
format env code-pages mule custom widget hashtable-print-readable
backquote make-network-process cocoa ns multi-tty emacs)

Memory information:
((conses 16 762936 115442)
 (symbols 48 72665 1)
 (miscs 40 7516 2209)
 (strings 32 207994 14679)
 (string-bytes 1 5392304)
 (vectors 16 61525)
 (vector-slots 8 1607452 84679)
 (floats 8 1017 2273)
 (intervals 56 34604 956)
 (buffers 960 120))

-- 
Sam Steingold (http://sds.podval.org/) on darwin Ns 10.3.1265
http://www.childpsy.net/ http://openvotingconsortium.org http://ffii.org
http://think-israel.org http://iris.org.il http://memri.org
Volume(Pizza of radius Z and thickness A) = PI * Z * Z * A

bug#17425: 24.4.50; tramp-cleanup-this-connection on push-button

[Glenn Morris]

Sam Steingold wrote:

> when I push-button with mouse (hoping to visit the url in a browser), I
> get the message from tramp

Maybe you could say what button you are referring to.
Since your report mentions rcirc major mode, I'm going to guess this was
in rcirc. I did

emacs -Q -f rcirc

and mouse-1 clicked on "http://www.freenode.net", and it just opened
in firefox as expected.

> Major mode: rcirc

bug#17425: 24.4.50; tramp-cleanup-this-connection on push-button

[Sam Steingold]

You need to open a remote file using tramp first, then make the remove
host unavailable, the click on a button.

On Tue, May 6, 2014 at 7:59 PM, Glenn Morris <rgm@gnu.org> wrote:
> Sam Steingold wrote:
>
>> when I push-button with mouse (hoping to visit the url in a browser), I
>> get the message from tramp
>
> Maybe you could say what button you are referring to.
> Since your report mentions rcirc major mode, I'm going to guess this was
> in rcirc. I did
>
> emacs -Q -f rcirc
>
> and mouse-1 clicked on "http://www.freenode.net", and it just opened
> in firefox as expected.
>
>> Major mode: rcirc



-- 
Sam Steingold <http://sds.podval.org> <http://www.childpsy.net/>

bug#17425: 24.4.50; tramp-cleanup-this-connection on push-button

[Glenn Morris]

Sam Steingold wrote:

> You need to open a remote file using tramp first, then make the remove
> host unavailable, the click on a button.

You really need to mention these kind of details when you first make a
report.
I can't easily replicate that.
Perhaps you left default-directory bound to a remote value.

bug#17425: 24.4.50; tramp-cleanup-this-connection on push-button

[Michael Albinus]

Sam Steingold <sds@gnu.org> writes:

Hi Sam,

> You need to open a remote file using tramp first, then make the remove
> host unavailable, the click on a button.

I'm also having a problem reproducing this problem. Could you, please,
give a recipe how to reproduce, starting with "emacs -Q"?

Thanks, and best regards, Michael.

bug#17425: 24.4.50; tramp-cleanup-this-connection on push-button

[Sam Steingold]

Hi,

To reproduce the problem:

1. edit ~/.ssh/config to make host "zzz" point to a known and working
host to which you have ssh access.

2. start "emacs -Q"; do
C-x d /zzz:~ RET
to start tramp connected to "zzz"

3. edit ~/.ssh/xonfig to remove "zzz"

4. kill the tramp ssh process (find it using `ps aux | grep zzz`).

5. in the "emacs -Q", start irc to create a buffer with
buttons. clicking on these buttons works just fine.

6. Now you need to do something eminently stupid: in the irc buffer with
buttons, evaluate this (using M-: or command eval-expression):
(setq default-directory #("/scp:zzz:/home/.../" 1 4 (tramp-default t)))
(replace "..." with your username)
Now, when you click on the buttons, you get the
tramp-cleanup-this-connection message instead of a new tab in the
browser.

Yes, one should not set default-directory to a remote directory as we do
above.  And, of course, I did not do that intentionally in the process
(long dead since) from which I reported the bug.

Still, this is a bug.  In fact, two bugs:

1. tramp should not be activated on button press
2. tramp's failure should not prevents browse-url from completing its task

Thanks!

-- 
Sam Steingold (http://sds.podval.org/) on darwin Ns 10.3.1265
http://www.childpsy.net/ http://www.memritv.org http://camera.org
http://ffii.org http://palestinefacts.org
Yellow wine is called "white" because it is made out of green grapes.

bug#17425: 24.4.50; tramp-cleanup-this-connection on push-button

[Glenn Morris]

Does this help?

*** lisp/net/browse-url.el	2014-02-10 01:34:22 +0000
--- lisp/net/browse-url.el	2014-05-07 23:02:02 +0000
***************
*** 812,819 ****
  			   browse-url-mailto-function)
  		      browse-url-browser-function))
  	;; Ensure that `default-directory' exists and is readable (b#6077).
! 	(default-directory (if (and (file-directory-p default-directory)
! 				    (file-readable-p default-directory))
  			       default-directory
  			     (expand-file-name "~/"))))
      ;; When connected to various displays, be careful to use the display of
--- 812,820 ----
  			   browse-url-mailto-function)
  		      browse-url-browser-function))
  	;; Ensure that `default-directory' exists and is readable (b#6077).
! 	(default-directory
!           (if (and (not (file-remote-p default-directory))
!                    (file-accessible-directory-p default-directory))
                default-directory
              (expand-file-name "~/"))))
      ;; When connected to various displays, be careful to use the display of

bug#17425: 24.4.50; tramp-cleanup-this-connection on push-button

[Michael Albinus]

Glenn Morris <rgm@gnu.org> writes:

> Does this help?

>   	;; Ensure that `default-directory' exists and is readable (b#6077).
> ! 	(default-directory
> !           (if (and (not (file-remote-p default-directory))
> !                    (file-accessible-directory-p default-directory))
>                 default-directory
>               (expand-file-name "~/"))))

I haven't checked further, but wouldn't this break url-handlers? See

(progn
  (url-handler-mode 1)
  (file-remote-p "http://user@host/"))

Best regards, Michael.

bug#17425: 24.4.50; tramp-cleanup-this-connection on push-button

[Glenn Morris]

Michael Albinus wrote:

>> ! 	(default-directory
>> !           (if (and (not (file-remote-p default-directory))
>> !                    (file-accessible-directory-p default-directory))
>>                 default-directory
>>               (expand-file-name "~/"))))
>
> I haven't checked further, but wouldn't this break url-handlers? See
>
> (progn
>   (url-handler-mode 1)
>   (file-remote-p "http://user@host/"))

I don't see how? Default-directory above is the one that browse-url uses
for the external browser that it starts.

I mean, maybe there's a problem if you want to visit a relative
`file:///' url using browse-url from a buffer with a remote
default-directory. But that would not work now, would it, since
browse-url starts a local browser process, which would not be able to
access remote files anyway?

Or can browse-url call eww these days?

The simplest thing is probably to do nothing, since I don't think this
is a real issue.

bug#17425: 24.4.50; tramp-cleanup-this-connection on push-button

[Sam Steingold]

On Thu, May 8, 2014 at 12:10 PM, Glenn Morris <rgm@gnu.org> wrote:
>
> The simplest thing is probably to do nothing,

this is _always_ the simplest thing. :-)

why does browse-url depend on default-directory anyway?

> since I don't think this is a real issue.

real enough for me.

-- 
Sam Steingold <http://sds.podval.org> <http://www.childpsy.net/>

bug#17425: 24.4.50; tramp-cleanup-this-connection on push-button

[Michael Albinus]

Sam Steingold <sds@gnu.org> writes:

> why does browse-url depend on default-directory anyway?

It could call an external process, indirectly. And when
default-directory is a remote directory handled by Tramp, that process
(the browser) would be called on that remote host. We don't want this.

Best regards, Michael.

bug#17425: 24.4.50; tramp-cleanup-this-connection on push-button

[Glenn Morris]

Sam Steingold wrote:

> why does browse-url depend on default-directory anyway?

Because it starts a process, which needs a directory to run in.
It needs to ehck that it still exists, else
http://debbugs.gnu.org/6077

> real enough for me.

If you set default-directory to a remote value, and the remote host
goes away, you are going to have problems.

bug#17425: 24.4.50; tramp-cleanup-this-connection on push-button

[Michael Albinus]

Glenn Morris <rgm@gnu.org> writes:

> Because it starts a process, which needs a directory to run in.
> It needs to ehck that it still exists, else
> http://debbugs.gnu.org/6077

I'm not against setting default-directory to a sensible value. But your
patch was kind of sledge-hammer to me.

In Tramp, I wrap just the call of the external process by a let-bind to
a local default-directory. As close as possible to the call.

By this, there are less collateral damages.

Best regards, Michael.

bug#17425: 24.4.50; tramp-cleanup-this-connection on push-button

[Michael Albinus]

Glenn Morris <rgm@gnu.org> writes:

> Michael Albinus wrote:
>
>>> ! 	(default-directory
>>> !           (if (and (not (file-remote-p default-directory))
>>> !                    (file-accessible-directory-p default-directory))
>>>                 default-directory
>>>               (expand-file-name "~/"))))
>>
>> I haven't checked further, but wouldn't this break url-handlers? See
>>
>> (progn
>>   (url-handler-mode 1)
>>   (file-remote-p "http://user@host/"))
>
> I don't see how? Default-directory above is the one that browse-url uses
> for the external browser that it starts.

Maybe I'm overreacting, but when you touch default-directory, file
operations from url-handlers might not work anymore.

(let ((default-directory "http://debbugs.gnu.org/"))
  (url-handler-mode 1)
  (expand-file-name "17245"))

returns a valid url which could be passed to browse-url.

I have no idea, whether browse-url and the operations in url-handlers
will be called together, 'tho. Maybe never.

But I have seen too many libraries which massage Tramp internals in
unexpected ways. So my personal alarm bell is ringing.

Feel free to ignore it.

Best regards, Michael.

bug#17425: 24.4.50; tramp-cleanup-this-connection on push-button

[Glenn Morris]

Michael Albinus wrote:

> (let ((default-directory "http://debbugs.gnu.org/"))
>   (url-handler-mode 1)
>   (expand-file-name "17245"))

Hey, this fails with a recursive load in emacs-24.3.90 -Q! :)

> returns a valid url which could be passed to browse-url.

But surely if it worked it would just return "http://debbugs.gnu.org/17245"?
Why is that a problem?

What does it even mean for browse-url to call eg firefox with a
default-directory of "http://debbugs.gnu.org/"?
It surely cannot run firefox on debbugs.gnu.org...

bug#17425: 24.4.50; tramp-cleanup-this-connection on push-button

[Michael Albinus]

Glenn Morris <rgm@gnu.org> writes:

> Michael Albinus wrote:
>
>> (let ((default-directory "http://debbugs.gnu.org/"))
>>   (url-handler-mode 1)
>>   (expand-file-name "17245"))
>
> Hey, this fails with a recursive load in emacs-24.3.90 -Q! :)

So we still have something to fix during the pretest :-)
Reminds me on test case tramp-test33-recursive-load, which I have
written some weeks ago due to a similar error in Tramp.

I'll check tomorrow what's up (but I don't know url-* too much).

>> returns a valid url which could be passed to browse-url.
>
> But surely if it worked it would just return "http://debbugs.gnu.org/17245"?

Yes.

> Why is that a problem?

It's not a problem. It would be a problem, if it does *not* return that
url, when default-directory is changed to something unrelated.

> What does it even mean for browse-url to call eg firefox with a
> default-directory of "http://debbugs.gnu.org/"?
> It surely cannot run firefox on debbugs.gnu.org...

Don't know. Some weeks ago, we have added further schemes to
url-handlers.el, see url-handler-regexp (don't remember whether it was
in the pretest or in trunk). And there is the scheme "ssh", for example.
This would allow to run firefox on a remote host. But I don't know
whether such a url goes ever to browse-url ...

Again, I have no scenario which shows that your patch will fail. But I
have bad feeling to touch default-directory such a way. Experience from
10 years Tramp maintainership.

Best regards, Michael.

bug#17425: 24.4.50; tramp-cleanup-this-connection on push-button

[Michael Albinus]

Glenn Morris <rgm@gnu.org> writes:

> Michael Albinus wrote:
>
>> (let ((default-directory "http://debbugs.gnu.org/"))
>>   (url-handler-mode 1)
>>   (expand-file-name "17245"))
>
> Hey, this fails with a recursive load in emacs-24.3.90 -Q! :)

Fixed in the emacs-24 branch.

Best regards, Michael.

bug#17425: 24.4.50; tramp-cleanup-this-connection on push-button

[Stefan Monnier]

>>> !           (if (and (not (file-remote-p default-directory))
>>> !                    (file-accessible-directory-p default-directory))

I think the test should be along the lines of
unhandled-file-name-directory.


        Stefan

bug#17425: 24.4.50; tramp-cleanup-this-connection on push-button

[Michael Albinus]

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

>>>> !           (if (and (not (file-remote-p default-directory))
>>>> !                    (file-accessible-directory-p default-directory))
>
> I think the test should be along the lines of
> unhandled-file-name-directory.

Maybe it is time to extend the semantics of
`file-accessible-directory-p'. When FILENAME is remote, Tramp's (or any
other) respective handler would return nil if there isn't an established
connection. IOW, Tramp wouldn't open a non-existing connection.

By this, we could just test `(file-accessible-directory-p default-directory)'
without the threat to damage other packages like url-handlers.el.

>         Stefan

Best regards, Michael.

bug#17425: 24.4.50; tramp-cleanup-this-connection on push-button

[Stefan Monnier]

> Maybe it is time to extend the semantics of
> `file-accessible-directory-p'.

Not sure what you mean: file-accessible-directory-p should test whether
that directory can be used as "cwd".  Nothing more nothing less.

> When FILENAME is remote, Tramp's (or any other) respective handler
> would return nil if there isn't an established connection. IOW, Tramp
> wouldn't open a non-existing connection.

Why?

> By this, we could just test `(file-accessible-directory-p default-directory)'

Sorry, I lost you: Where would you use this test, instead of what?

> without the threat to damage other packages like url-handlers.el.

And I have no idea what damage you're referring to.


        Stefan

bug#17425: 24.4.50; tramp-cleanup-this-connection on push-button

[Michael Albinus]

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

>> Maybe it is time to extend the semantics of
>> `file-accessible-directory-p'.
>
> Not sure what you mean: file-accessible-directory-p should test whether
> that directory can be used as "cwd".  Nothing more nothing less.
>
>> When FILENAME is remote, Tramp's (or any other) respective handler
>> would return nil if there isn't an established connection. IOW, Tramp
>> wouldn't open a non-existing connection.
>
> Why?

The scenario of the OP is as follows:

- There's a buffer with an existing remote default-directory, connection
  established.

- No problem to run browse-url in that buffer (the example was an
  rcirc buffer). It doesn't hurt, that default-directory is remote.

- After a while, the remote host wasn't available any more, for whatever
  reason.

- Now browse-url fails, because it checks file-directory-p and
  file-readable-p. Both operations try to access the remote
  default-directory.

If the check would be replaced file-accessible-directory-p, and this
operation doesn't try to reopen a stalled connection, it would be
sufficient for that use case.

Glenn has proposed a similar change, but with also checking (not
(file-remote-p default-directory)). I became a little bit nervous,
because file-remote-p isn't reserved not only for Tramp, but for any
file name handler (like url-handlers.el).

>> By this, we could just test `(file-accessible-directory-p default-directory)'
>
> Sorry, I lost you: Where would you use this test, instead of what?

--8<---------------cut here---------------start------------->8---
*** /home/albinus/src/emacs/lisp/net/browse-url.el.~117091~	2014-05-12 20:47:03.821360420 +0200
--- /home/albinus/src/emacs/lisp/net/browse-url.el	2014-05-12 20:46:53.969311566 +0200
***************
*** 812,819 ****
  			   browse-url-mailto-function)
  		      browse-url-browser-function))
  	;; Ensure that `default-directory' exists and is readable (b#6077).
! 	(default-directory (if (and (file-directory-p default-directory)
! 				    (file-readable-p default-directory))
  			       default-directory
  			     (expand-file-name "~/"))))
      ;; When connected to various displays, be careful to use the display of
--- 812,818 ----
  			   browse-url-mailto-function)
  		      browse-url-browser-function))
  	;; Ensure that `default-directory' exists and is readable (b#6077).
! 	(default-directory (if (file-accessible-directory-p default-directory)
  			       default-directory
  			     (expand-file-name "~/"))))
      ;; When connected to various displays, be careful to use the display of
--8<---------------cut here---------------end--------------->8---

>> without the threat to damage other packages like url-handlers.el.
>
> And I have no idea what damage you're referring to.

That's the whole thread about, I've discussed with Glenn.

>         Stefan

Best regards, Michael.

bug#17425: 24.4.50; tramp-cleanup-this-connection on push-button

[Stefan Monnier]

> - There's a buffer with an existing remote default-directory, connection
>   established.
> - No problem to run browse-url in that buffer (the example was an
>   rcirc buffer). It doesn't hurt, that default-directory is remote.
> - After a while, the remote host wasn't available any more, for whatever
>   reason.
> - Now browse-url fails, because it checks file-directory-p and
>   file-readable-p. Both operations try to access the remote
>   default-directory.

If the process is supposed to run locally, then the right test should be
based on unhandled-file-name-directory (that's what it's for).  If the
process is supposed to be run "on the host referred to be by
default-directory", then re-connecting is the right thing to do.

> If the check would be replaced file-accessible-directory-p, and this
> operation doesn't try to reopen a stalled connection, it would be
> sufficient for that use case.

file-accessible-directory-p is really just another test like
file-directory-p: it should setup a connection if needed.


        Stefan

bug#17425: 24.4.50; tramp-cleanup-this-connection on push-button

[Michael Albinus]

Stefan Monnier <monnier@IRO.UMontreal.CA> writes:

>> - There's a buffer with an existing remote default-directory, connection
>>   established.
>> - No problem to run browse-url in that buffer (the example was an
>>   rcirc buffer). It doesn't hurt, that default-directory is remote.
>> - After a while, the remote host wasn't available any more, for whatever
>>   reason.
>> - Now browse-url fails, because it checks file-directory-p and
>>   file-readable-p. Both operations try to access the remote
>>   default-directory.
>
> If the process is supposed to run locally, then the right test should be
> based on unhandled-file-name-directory (that's what it's for).  If the
> process is supposed to be run "on the host referred to be by
> default-directory", then re-connecting is the right thing to do.

unhandled-file-name-directory will always return a local directory. This
could destroy the work of url-handlers.el. Imagine the snippet

(let ((default-directory "http://debbugs.gnu.org"))
  (url-handler-mode 1)
  (browse-url "17425"))

If one of the browse-url-* functions uses expand-file-name internally,
it shall open the url "http://debbugs.gnu.org/17425". This wouldn't
work, if unhandled-file-name-directory changes default-directory.

If you don't care, then I'm ok with you. I just wanted to show the
consequence. 

>         Stefan

Best regards, Michael.

bug#17425: 24.4.50; tramp-cleanup-this-connection on push-button

[Michael Albinus]

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

> If one of the browse-url-* functions uses expand-file-name internally,
> it shall open the url "http://debbugs.gnu.org/17425". This wouldn't
> work, if unhandled-file-name-directory changes default-directory.

Maybe a solution is, that we expand-file-name the url in browse-url if
it is a relative file name. After this, it is no problem to use
unhandled-file-name-directory.

Best regards, Michael.

bug#17425: 24.4.50; tramp-cleanup-this-connection on push-button

[Stefan Monnier]

> (let ((default-directory "http://debbugs.gnu.org"))
>   (url-handler-mode 1)
>   (browse-url "17425"))

For the above to work (assuming browse-url spawns an external process),
browse-url will have to pass the default-directory to the process.
It can't do it using POSIX's "current working directory", so it will
have to pass it some other way.  Once that's done, browse-url can use
unhandled-file-name-directory and change default-directory without
breaking anything.

I.e. unhandled-file-name-directory should be used close to the
call-process/process-start, and for it to work we need to make sure none
of the parameters are relative file-names, indeed.  But that's the only
way it can work reliably, since there's no way to pass relative file
names to a process when the base directory doesn't exist in the OS.


        Stefan

bug#17425: 24.4.50; tramp-cleanup-this-connection on push-button

[Michael Albinus]

Stefan Monnier <monnier@IRO.UMontreal.CA> writes:

> I.e. unhandled-file-name-directory should be used close to the
> call-process/process-start, and for it to work we need to make sure none
> of the parameters are relative file-names, indeed.  But that's the only
> way it can work reliably, since there's no way to pass relative file
> names to a process when the base directory doesn't exist in the OS.

Well, I have committed a fix to the emacs-24 branch. expand-file-name is
called only in case url-handler-mode is non nil (otherwise there are
strange results), and default-directory is passed through
unhandled-file-name-directory. According to my tests, this seems to be
sufficient.

Sam, you might retest it in your environment. Feel free to reopen if
there are still problems.

>         Stefan

Best regards, Michael.

bug#17425: 24.4.50; tramp-cleanup-this-connection on push-button

[Stefan Monnier]

> Well, I have committed a fix to the emacs-24 branch.

Thanks.

> expand-file-name is called only in case url-handler-mode is non nil

Sounds good.

> (otherwise there are strange results),

Indeed: the argument is supposed to be a URL, not a file name ;-)


        Stefan