Shell commands in deleted directories

Shell commands in deleted directories

[Antoine Levitt]

Hi,

[shell] cd
[shell] mkdir test
[emacs] M-x cd RET ~/test RET
[shell] rmdir test
[emacs] M-! echo hi

Debugger entered--Lisp error: (file-error "Setting current directory"
"no such file or directory" "/home/antoine/test/")

  call-process-region(1 1 "/bin/bash" "/tmp/emacsmVCHW6" #<buffer *Shell
  Command Output*> nil "-c" "echo hi")
  
  shell-command-on-region(1 1 "echo hi" nil nil nil)
  shell-command("echo hi" nil nil)
  shell-command-replace("echo hi" nil nil)
  call-interactively(shell-command-replace nil nil)

Admittedly, this is a bit of an edge case, but it happens to me using
ERC: I usually run ERC from dired or somesuch, on a directory that may
later be removed. Then all the erc buffers have the deleted directory as
current path, and all shell commands fail (and I'm using shell commands
for notification, which means that some hooks fail, and my whole ERC
session basically fails).

The following patch works for me. I'm not sure what the behaviour should
be, but since the "silent fallback to ~" is already used above in the
code in a different case, it should be OK. There's also a bit of code
copied over from above, but I couldn't figure out a way to factorize it
properly. What do you think?

diff --git a/src/callproc.c b/src/callproc.c
index f2543f2..856760b 100644
--- a/src/callproc.c
+++ b/src/callproc.c
@@ -335,8 +335,17 @@ usage: (call-process PROGRAM &optional INFILE BUFFER DISPLAY &rest ARGS)  */)
     current_dir = Ffile_name_as_directory (current_dir);
 
     if (NILP (Ffile_accessible_directory_p (current_dir)))
-      report_file_error ("Setting current directory",
-			 Fcons (current_buffer->directory, Qnil));
+      {
+	/* If current_dir is unreachable (typically, does not exist), use
+	   ~/ as default */
+	current_dir = build_string ("~/");
+	current_dir = expand_and_dir_to_file (current_dir, Qnil);
+	current_dir = Ffile_name_as_directory (current_dir);
+	if (NILP (Ffile_accessible_directory_p (current_dir)))
+	  {
+	    report_file_error ("Setting current directory to ~/", Qnil);
+	  }
+      }
 
     if (STRING_MULTIBYTE (infile))
       infile = ENCODE_FILE (infile);

Re: Shell commands in deleted directories

[Paul Eggert]

On 02/13/2011 01:02 PM, Antoine Levitt wrote:
> +	/* If current_dir is unreachable (typically, does not exist), use
> +	   ~/ as default */

This looks like a bad idea.  Suppose I run the shell
command "rm *" in a deleted directory?
The change would cause me to remove files in
my home directory.  Better safe than sorry.

Re: Shell commands in deleted directories

[Eli Zaretskii]

> From: Antoine Levitt <antoine.levitt@gmail.com>
> Date: Sun, 13 Feb 2011 22:02:34 +0100
> 
>      if (NILP (Ffile_accessible_directory_p (current_dir)))
> -      report_file_error ("Setting current directory",
> -			 Fcons (current_buffer->directory, Qnil));
> +      {
> +	/* If current_dir is unreachable (typically, does not exist), use
> +	   ~/ as default */
> +	current_dir = build_string ("~/");
> +	current_dir = expand_and_dir_to_file (current_dir, Qnil);
> +	current_dir = Ffile_name_as_directory (current_dir);
> +	if (NILP (Ffile_accessible_directory_p (current_dir)))
> +	  {
> +	    report_file_error ("Setting current directory to ~/", Qnil);
> +	  }
> +      }

And "who is watching the watchdog"?  That is, what if "~" does not
exist, either?

Re: Shell commands in deleted directories

[Antoine Levitt]

13/02/11 22:13, Eli Zaretskii
>> From: Antoine Levitt <antoine.levitt@gmail.com>
>> Date: Sun, 13 Feb 2011 22:02:34 +0100
>> 
>>      if (NILP (Ffile_accessible_directory_p (current_dir)))
>> -      report_file_error ("Setting current directory",
>> -			 Fcons (current_buffer->directory, Qnil));
>> +      {
>> +	/* If current_dir is unreachable (typically, does not exist), use
>> +	   ~/ as default */
>> +	current_dir = build_string ("~/");
>> +	current_dir = expand_and_dir_to_file (current_dir, Qnil);
>> +	current_dir = Ffile_name_as_directory (current_dir);
>> +	if (NILP (Ffile_accessible_directory_p (current_dir)))
>> +	  {
>> +	    report_file_error ("Setting current directory to ~/", Qnil);
>> +	  }
>> +      }
>
> And "who is watching the watchdog"?  That is, what if "~" does not
> exist, either?

That's precisely the report_file_error.

Re: Shell commands in deleted directories

[Antoine Levitt]

13/02/11 22:07, Paul Eggert
> On 02/13/2011 01:02 PM, Antoine Levitt wrote:
>> +	/* If current_dir is unreachable (typically, does not exist), use
>> +	   ~/ as default */
>
> This looks like a bad idea.  Suppose I run the shell
> command "rm *" in a deleted directory?
> The change would cause me to remove files in
> my home directory.  Better safe than sorry.

Mmh, true. But then again, if you're using "rm *" as a shell command
without being absolutely sure where you are, you're just asking for
trouble. But it's a valid point, and there might be other commands which
might have undesirable/confusing side effects when run from ~/. It might
even be a problem for the first test:

    current_dir = Funhandled_file_name_directory (current_dir);
    if (NILP (current_dir))
      /* If the file name handler says that current_dir is unreachable, use
	 a sensible default. */
      current_dir = build_string ("~/");

I don't see how to resolve this, though.

Re: Shell commands in deleted directories

[Ted Zlatanov]

On Sun, 13 Feb 2011 22:21:38 +0100 Antoine Levitt <antoine.levitt@gmail.com> wrote: 

AL> 13/02/11 22:07, Paul Eggert
>> On 02/13/2011 01:02 PM, Antoine Levitt wrote:
>>> +	/* If current_dir is unreachable (typically, does not exist), use
>>> +	   ~/ as default */
>> 
>> This looks like a bad idea.  Suppose I run the shell
>> command "rm *" in a deleted directory?
>> The change would cause me to remove files in
>> my home directory.  Better safe than sorry.

AL> Mmh, true. But then again, if you're using "rm *" as a shell command
AL> without being absolutely sure where you are, you're just asking for
AL> trouble.

I am strongly against this kind of surprise and hope it doesn't go into
Emacs.

Ted

Re: Shell commands in deleted directories

[Antoine Levitt]

13/02/11 22:34, Ted Zlatanov
> On Sun, 13 Feb 2011 22:21:38 +0100 Antoine Levitt <antoine.levitt@gmail.com> wrote: 
>
> AL> 13/02/11 22:07, Paul Eggert
>>> On 02/13/2011 01:02 PM, Antoine Levitt wrote:
>>>> +	/* If current_dir is unreachable (typically, does not exist), use
>>>> +	   ~/ as default */
>>> 
>>> This looks like a bad idea.  Suppose I run the shell
>>> command "rm *" in a deleted directory?
>>> The change would cause me to remove files in
>>> my home directory.  Better safe than sorry.
>
> AL> Mmh, true. But then again, if you're using "rm *" as a shell command
> AL> without being absolutely sure where you are, you're just asking for
> AL> trouble.
>
> I am strongly against this kind of surprise and hope it doesn't go into
> Emacs.

I wasn't suggesting it be merged after the objection Paul raised (see
the other half of my post). In the end, it's probably better to be
handled by the user or by error-catching from calling code. Maybe there
should be some kind of checks when creating new buffers though? Like if
a new buffer is created that isn't associated with a file, then if its
pwd doesn't exist, it's reset to ~/?

Re: Shell commands in deleted directories

[Ted Zlatanov]

On Sun, 13 Feb 2011 23:05:56 +0100 Antoine Levitt <antoine.levitt@gmail.com> wrote: 

AL> 13/02/11 22:34, Ted Zlatanov
>> On Sun, 13 Feb 2011 22:21:38 +0100 Antoine Levitt <antoine.levitt@gmail.com> wrote: 
>> 
AL> 13/02/11 22:07, Paul Eggert
>>>> On 02/13/2011 01:02 PM, Antoine Levitt wrote:
>>>>> +	/* If current_dir is unreachable (typically, does not exist), use
>>>>> +	   ~/ as default */
>>>> 
>>>> This looks like a bad idea.  Suppose I run the shell
>>>> command "rm *" in a deleted directory?
>>>> The change would cause me to remove files in
>>>> my home directory.  Better safe than sorry.
>> 
AL> Mmh, true. But then again, if you're using "rm *" as a shell command
AL> without being absolutely sure where you are, you're just asking for
AL> trouble.
>> 
>> I am strongly against this kind of surprise and hope it doesn't go into
>> Emacs.

AL> I wasn't suggesting it be merged after the objection Paul raised (see
AL> the other half of my post).

I agreed with you :)

AL> In the end, it's probably better to be handled by the user or by
AL> error-catching from calling code. Maybe there should be some kind of
AL> checks when creating new buffers though? Like if a new buffer is
AL> created that isn't associated with a file, then if its pwd doesn't
AL> exist, it's reset to ~/?

There are many situations where this solution would be a problem.  I
don't know if there is a good solution that doesn't involve lots of
error-catching at some level.

Ted

Re: Shell commands in deleted directories

[Stefan Monnier]

>      if (NILP (Ffile_accessible_directory_p (current_dir)))
> -      report_file_error ("Setting current directory",
> -			 Fcons (current_buffer->directory, Qnil));
> +      {
> +	/* If current_dir is unreachable (typically, does not exist), use
> +	   ~/ as default */
> +	current_dir = build_string ("~/");
> +	current_dir = expand_and_dir_to_file (current_dir, Qnil);
> +	current_dir = Ffile_name_as_directory (current_dir);
> +	if (NILP (Ffile_accessible_directory_p (current_dir)))
> +	  {
> +	    report_file_error ("Setting current directory to ~/", Qnil);
> +	  }
> +      }
 
This is a recurrent problem and the above patch is very tempting, but
it's sadly wrong when the command uses relative file names.  So, the
problem has to be fixed on a case by case basis.  In the case of M-!
there's no way for Emacs to know whether the command will use relative
file names, so signalling an error is the best it can do.

OTOH if ERC runs commands internally, ERC presumably knows whether those
commands care about default-directory, and if they don't, then it can
use ~/ instead.

We should maybe provide some canonical way to fix this problem, tho.
E.g. a new macro `with-existing-default-directory' to wrap the relevant
call(s) to start|call-process.


        Stefan