bug#65763: Error opening a file from a Git working directory if Git is not installed

bug#65763: Error opening a file from a Git working directory if Git is not installed

[Paul Pogonyshev]

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

To reproduce:

1. Change current directory to a Git checkout (e.g. `~/git/emacs' or
something).
2. From the command line:

       $ emacs --batch --eval "(progn (setf vc-git-program
\"git-is-not-installed\") (find-file-noselect \"whatever\"))"
       Error: (file-missing "Searching for program" "No such file or
directory" "git-is-not-installed")

Expected: Emacs is able to open the file, Git must not be essential.

Usecase: on the real machine Git _is_ of course installed, but then you run
Emacs inside a Docker container that has _no_ Git, with a directory mapped
from your physical machine.  In this setup there is appearance of a Git
working directory, yet Git executable is not available.

Appears to work fine up to Emacs 27, broken starting with Emacs 28.

Paul

[-- Attachment #2: Type: text/html, Size: 914 bytes --]

bug#65763: Error opening a file from a Git working directory if Git is not installed

[Eli Zaretskii]

> From: Paul Pogonyshev <pogonyshev@gmail.com>
> Date: Tue, 5 Sep 2023 20:54:54 +0200
> 
> To reproduce:
> 
> 1. Change current directory to a Git checkout (e.g. `~/git/emacs' or something).
> 2. From the command line:
> 
>        $ emacs --batch --eval "(progn (setf vc-git-program \"git-is-not-installed\") (find-file-noselect
> \"whatever\"))"
>        Error: (file-missing "Searching for program" "No such file or directory" "git-is-not-installed")
> 
> Expected: Emacs is able to open the file, Git must not be essential.

Emacs does open the file; what you see is not an error, it's a
message.  The code which tries to invoke Git runs with-demoted-errors,
so any error is converted to a simple message.  If you try this
variant of your command:

  $ emacs --batch --eval "(progn (setf vc-git-program \"git-is-not-installed\") (message \"%s\" (find-file-noselect \"README\")))"

you will see:

  Error: (file-missing "Searching for program" "No such file or directory" "git-is-not-installed")
  README

That "README" at the end means that find-file-noselect did read the
file into its buffer, and the error message is just a message.

So if this somehow prevented you from doing something, please tell
more, or maybe the recipe needs more steps?

bug#65763: Error opening a file from a Git working directory if Git is not installed

[Paul Pogonyshev]

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

From what I see it's not just a warning:

$ emacs --batch --eval "(let ((debug-on-error t)) (setf vc-git-program
\"git-is-not-installed\") (find-file-noselect \"whatever\"))"
Debugger entered--Lisp error: (file-missing "Searching for program" "No
such file or directory" "git-is-not-installed")
  call-process("git-is-not-installed" nil (t nil) nil "--no-pager"
"ls-files" "-c" "-z" "--" "whatever")
  process-file("git-is-not-installed" nil (t nil) nil "--no-pager"
"ls-files" "-c" "-z" "--" "whatever")
  vc-git--call((t nil) "ls-files" "-c" "-z" "--" "whatever")
  vc-git--out-ok("ls-files" "-c" "-z" "--" "whatever")
  vc-git-registered("/home/paul/eldev/whatever")
  vc-git-registered("/home/paul/eldev/whatever")
  vc-call-backend(Git registered "/home/paul/eldev/whatever")
  #f(compiled-function (b) #<bytecode 0x15a622e462a366fe>)(Git)
  mapc(#f(compiled-function (b) #<bytecode 0x15a622e462a366fe>) (RCS CVS
SVN SCCS SRC Bzr Git Hg))
  vc-registered("/home/paul/eldev/whatever")
  vc-backend("/home/paul/eldev/whatever")
  vc-refresh-state()
  run-hooks(find-file-hook)
  after-find-file(t t)
  find-file-noselect-1(#<buffer whatever> "~/eldev/whatever" nil nil
"~/eldev/whatever" nil)
  find-file-noselect("whatever")
  (let ((debug-on-error t)) (setf vc-git-program "git-is-not-installed")
(find-file-noselect "whatever"))
  command-line-1(("--eval" "(let ((debug-on-error t)) (setf vc-git-program
\"gi..."))
  command-line()
  normal-top-level()

Paul

On Tue, 5 Sept 2023 at 21:44, Eli Zaretskii <eliz@gnu.org> wrote:

> > From: Paul Pogonyshev <pogonyshev@gmail.com>
> > Date: Tue, 5 Sep 2023 20:54:54 +0200
> >
> > To reproduce:
> >
> > 1. Change current directory to a Git checkout (e.g. `~/git/emacs' or
> something).
> > 2. From the command line:
> >
> >        $ emacs --batch --eval "(progn (setf vc-git-program
> \"git-is-not-installed\") (find-file-noselect
> > \"whatever\"))"
> >        Error: (file-missing "Searching for program" "No such file or
> directory" "git-is-not-installed")
> >
> > Expected: Emacs is able to open the file, Git must not be essential.
>
> Emacs does open the file; what you see is not an error, it's a
> message.  The code which tries to invoke Git runs with-demoted-errors,
> so any error is converted to a simple message.  If you try this
> variant of your command:
>
>   $ emacs --batch --eval "(progn (setf vc-git-program
> \"git-is-not-installed\") (message \"%s\" (find-file-noselect \"README\")))"
>
> you will see:
>
>   Error: (file-missing "Searching for program" "No such file or directory"
> "git-is-not-installed")
>   README
>
> That "README" at the end means that find-file-noselect did read the
> file into its buffer, and the error message is just a message.
>
> So if this somehow prevented you from doing something, please tell
> more, or maybe the recipe needs more steps?
>

[-- Attachment #2: Type: text/html, Size: 4057 bytes --]

bug#65763: Error opening a file from a Git working directory if Git is not installed

[Paul Pogonyshev]

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

Ah, `debug-on-error' makes `with-demoted-errors' not demote it.

It seems in my case `debug-on-error' is let-bound in
`ert--run-test-internal'. I.e. when file is failed to be opened inside a
ERT-based test, there is no warning, it's a hard error. I happen to run ERT
tests inside a Docker container.

I'm not sure if it is a bug, but it does cause problems.

Paul

On Tue, 5 Sept 2023 at 22:06, Paul Pogonyshev <pogonyshev@gmail.com> wrote:

> From what I see it's not just a warning:
>
> $ emacs --batch --eval "(let ((debug-on-error t)) (setf vc-git-program
> \"git-is-not-installed\") (find-file-noselect \"whatever\"))"
> Debugger entered--Lisp error: (file-missing "Searching for program" "No
> such file or directory" "git-is-not-installed")
>   call-process("git-is-not-installed" nil (t nil) nil "--no-pager"
> "ls-files" "-c" "-z" "--" "whatever")
>   process-file("git-is-not-installed" nil (t nil) nil "--no-pager"
> "ls-files" "-c" "-z" "--" "whatever")
>   vc-git--call((t nil) "ls-files" "-c" "-z" "--" "whatever")
>   vc-git--out-ok("ls-files" "-c" "-z" "--" "whatever")
>   vc-git-registered("/home/paul/eldev/whatever")
>   vc-git-registered("/home/paul/eldev/whatever")
>   vc-call-backend(Git registered "/home/paul/eldev/whatever")
>   #f(compiled-function (b) #<bytecode 0x15a622e462a366fe>)(Git)
>   mapc(#f(compiled-function (b) #<bytecode 0x15a622e462a366fe>) (RCS CVS
> SVN SCCS SRC Bzr Git Hg))
>   vc-registered("/home/paul/eldev/whatever")
>   vc-backend("/home/paul/eldev/whatever")
>   vc-refresh-state()
>   run-hooks(find-file-hook)
>   after-find-file(t t)
>   find-file-noselect-1(#<buffer whatever> "~/eldev/whatever" nil nil
> "~/eldev/whatever" nil)
>   find-file-noselect("whatever")
>   (let ((debug-on-error t)) (setf vc-git-program "git-is-not-installed")
> (find-file-noselect "whatever"))
>   command-line-1(("--eval" "(let ((debug-on-error t)) (setf vc-git-program
> \"gi..."))
>   command-line()
>   normal-top-level()
>
> Paul
>
> On Tue, 5 Sept 2023 at 21:44, Eli Zaretskii <eliz@gnu.org> wrote:
>
>> > From: Paul Pogonyshev <pogonyshev@gmail.com>
>> > Date: Tue, 5 Sep 2023 20:54:54 +0200
>> >
>> > To reproduce:
>> >
>> > 1. Change current directory to a Git checkout (e.g. `~/git/emacs' or
>> something).
>> > 2. From the command line:
>> >
>> >        $ emacs --batch --eval "(progn (setf vc-git-program
>> \"git-is-not-installed\") (find-file-noselect
>> > \"whatever\"))"
>> >        Error: (file-missing "Searching for program" "No such file or
>> directory" "git-is-not-installed")
>> >
>> > Expected: Emacs is able to open the file, Git must not be essential.
>>
>> Emacs does open the file; what you see is not an error, it's a
>> message.  The code which tries to invoke Git runs with-demoted-errors,
>> so any error is converted to a simple message.  If you try this
>> variant of your command:
>>
>>   $ emacs --batch --eval "(progn (setf vc-git-program
>> \"git-is-not-installed\") (message \"%s\" (find-file-noselect \"README\")))"
>>
>> you will see:
>>
>>   Error: (file-missing "Searching for program" "No such file or
>> directory" "git-is-not-installed")
>>   README
>>
>> That "README" at the end means that find-file-noselect did read the
>> file into its buffer, and the error message is just a message.
>>
>> So if this somehow prevented you from doing something, please tell
>> more, or maybe the recipe needs more steps?
>>
>

[-- Attachment #2: Type: text/html, Size: 4891 bytes --]

bug#65763: Error opening a file from a Git working directory if Git is not installed

[Eli Zaretskii]

> From: Paul Pogonyshev <pogonyshev@gmail.com>
> Date: Tue, 5 Sep 2023 22:14:15 +0200
> Cc: 65763@debbugs.gnu.org
> 
> Ah, `debug-on-error' makes `with-demoted-errors' not demote it.

Yes, exactly.

> It seems in my case `debug-on-error' is let-bound in `ert--run-test-internal'. I.e. when file is failed to be
> opened inside a ERT-based test, there is no warning, it's a hard error. I happen to run ERT tests
> inside a Docker container.
> 
> I'm not sure if it is a bug, but it does cause problems.

Are the problems it causes happen only when debug-on-error is non-nil
for some reason?  Or did you see it cause problems in other cases as
well?

bug#65763: Error opening a file from a Git working directory if Git is not installed

[Paul Pogonyshev]

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

The problem appears to be only with `debug-on-error'. However, there are
cases where you cannot control it at all, e.g. with ERT (probably also
Buttercup or any other testing framework). In effect, an ERT test fails for
a "random" reason, depending on which machine it is executed, i.e. it fails
inside that Docker container.

Paul

On Wed, 6 Sept 2023 at 04:26, Eli Zaretskii <eliz@gnu.org> wrote:

> > From: Paul Pogonyshev <pogonyshev@gmail.com>
> > Date: Tue, 5 Sep 2023 22:14:15 +0200
> > Cc: 65763@debbugs.gnu.org
> >
> > Ah, `debug-on-error' makes `with-demoted-errors' not demote it.
>
> Yes, exactly.
>
> > It seems in my case `debug-on-error' is let-bound in
> `ert--run-test-internal'. I.e. when file is failed to be
> > opened inside a ERT-based test, there is no warning, it's a hard error.
> I happen to run ERT tests
> > inside a Docker container.
> >
> > I'm not sure if it is a bug, but it does cause problems.
>
> Are the problems it causes happen only when debug-on-error is non-nil
> for some reason?  Or did you see it cause problems in other cases as
> well?
>

[-- Attachment #2: Type: text/html, Size: 1621 bytes --]

bug#65763: Error opening a file from a Git working directory if Git is not installed

[Eli Zaretskii]

> From: Paul Pogonyshev <pogonyshev@gmail.com>
> Date: Wed, 6 Sep 2023 09:29:59 +0200
> Cc: 65763@debbugs.gnu.org
> 
> The problem appears to be only with `debug-on-error'. However, there are cases where you cannot
> control it at all, e.g. with ERT (probably also Buttercup or any other testing framework). In effect, an
> ERT test fails for a "random" reason, depending on which machine it is executed, i.e. it fails inside
> that Docker container.

I see.

Well, we could then protect the execution of the problematic form "by
hand" by using condition-case-unless-debug.  Dmitry, WDYT?

bug#65763: Error opening a file from a Git working directory if Git is not installed

[Dmitry Gutov]

On 06/09/2023 15:13, Eli Zaretskii wrote:
>> From: Paul Pogonyshev<pogonyshev@gmail.com>
>> Date: Wed, 6 Sep 2023 09:29:59 +0200
>> Cc:65763@debbugs.gnu.org
>>
>> The problem appears to be only with `debug-on-error'. However, there are cases where you cannot
>> control it at all, e.g. with ERT (probably also Buttercup or any other testing framework). In effect, an
>> ERT test fails for a "random" reason, depending on which machine it is executed, i.e. it fails inside
>> that Docker container.
> I see.
> 
> Well, we could then protect the execution of the problematic form "by
> hand" by using condition-case-unless-debug.  Dmitry, WDYT?

Maybe the solution is to use the straight condition-case rather than 
condition-case-unless-debug? Because otherwise as long as 
condition-case-unless-debug is used, we would always have this problem.

Rewriting with-demoted-errors is not an option, of course, but we could 
create a special, shorted version of it for vc.

Another option, though, is to rewrite the ERT tests in question: e.g. to 
bind vc-handled-backends to nil, or to some other value if the presence 
of certain VC programs is known and expected in advance.

bug#65763: Error opening a file from a Git working directory if Git is not installed

[Paul Pogonyshev]

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

> Another option, though, is to rewrite the ERT tests in question: e.g. to
> bind vc-handled-backends to nil, or to some other value if the presence
> of certain VC programs is known and expected in advance.

Test in question does not need or use Git in any way. It fails only because
Emacs under it decides to die with an exception when Git is not installed,
`debug-on-error' happens to be t (because of ERT, test itself doesn't even
set it explicitly) and current directory is structured in a particular way.

It feels conceptually wrong to require all tests that open files to rebind
`vc-handled-backends'. This is not what they are testing. It also depends
on knowing particular Emacs quirks (which I, for example, didn't know one
day earlier). If those were to change in some way, would all tests
everywhere need to accomodate?

Paul

On Wed, 6 Sept 2023 at 14:35, Dmitry Gutov <dmitry@gutov.dev> wrote:

> On 06/09/2023 15:13, Eli Zaretskii wrote:
> >> From: Paul Pogonyshev<pogonyshev@gmail.com>
> >> Date: Wed, 6 Sep 2023 09:29:59 +0200
> >> Cc:65763@debbugs.gnu.org
> >>
> >> The problem appears to be only with `debug-on-error'. However, there
> are cases where you cannot
> >> control it at all, e.g. with ERT (probably also Buttercup or any other
> testing framework). In effect, an
> >> ERT test fails for a "random" reason, depending on which machine it is
> executed, i.e. it fails inside
> >> that Docker container.
> > I see.
> >
> > Well, we could then protect the execution of the problematic form "by
> > hand" by using condition-case-unless-debug.  Dmitry, WDYT?
>
> Maybe the solution is to use the straight condition-case rather than
> condition-case-unless-debug? Because otherwise as long as
> condition-case-unless-debug is used, we would always have this problem.
>
> Rewriting with-demoted-errors is not an option, of course, but we could
> create a special, shorted version of it for vc.
>
> Another option, though, is to rewrite the ERT tests in question: e.g. to
> bind vc-handled-backends to nil, or to some other value if the presence
> of certain VC programs is known and expected in advance.
>

[-- Attachment #2: Type: text/html, Size: 2789 bytes --]

bug#65763: Error opening a file from a Git working directory if Git is not installed

[Dmitry Gutov]

On 06/09/2023 15:49, Paul Pogonyshev wrote:
> It feels conceptually wrong to require all tests that open files to 
> rebind `vc-handled-backends'. This is not what they are testing. It also 
> depends on knowing particular Emacs quirks (which I, for example, didn't 
> know one day earlier). If those were to change in some way, would all 
> tests everywhere need to accomodate?

But does that mean that VC needs to be changed, or ERT?

E.g. ERT has an interactive mode where it drops into a debugger for the 
user ('d'), but does it need to enable the debugger for its simple runs 
as well?

bug#65763: Error opening a file from a Git working directory if Git is not installed

[Paul Pogonyshev]

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

I doubt ERT can be changed, it probably sets `debug-on-error' for a reason,
i.e. without it it simply couldn't work. As far as I can tell, the only
good option is to never trigger an error just because a directory looks
like it might be Git, but Git is not installed.

Also consider this from a user point of view. Let's say I have nothing to
do with programming at all and don't have Git installed. Someone emails me
a cool Emacs package as a tarball and accidentally archives `.git' along,
because he is a developer. I unpack it, open some file and now my Emacs
warns me that Git is not installed. But why? I didn't even ask it to do
anything. It *itself* decided to do something (because it saw a `.git'
directory), failed (Git is not installed), and now warns *me*. From my
point of view, if there is a failure for action that wasn't direct result
of user order, Emacs should stay silent.

In other words, I think there are two sane options here:

1) Simply check if Git is installed before doing anything Git-related from
`find-file-hook' callback. If it is not installed, just silently don't do
anything. Reserve errors and warnings until the user actually ask you to do
something Git-related, not simply to open a file.

2) Make sure that this error is always demoted to a warning, even if
`debug-on-error' is t.

I like option 1 better.

Paul

On Wed, 6 Sept 2023 at 14:52, Dmitry Gutov <dmitry@gutov.dev> wrote:

> On 06/09/2023 15:49, Paul Pogonyshev wrote:
> > It feels conceptually wrong to require all tests that open files to
> > rebind `vc-handled-backends'. This is not what they are testing. It also
> > depends on knowing particular Emacs quirks (which I, for example, didn't
> > know one day earlier). If those were to change in some way, would all
> > tests everywhere need to accomodate?
>
> But does that mean that VC needs to be changed, or ERT?
>
> E.g. ERT has an interactive mode where it drops into a debugger for the
> user ('d'), but does it need to enable the debugger for its simple runs
> as well?
>

[-- Attachment #2: Type: text/html, Size: 2579 bytes --]

bug#65763: Error opening a file from a Git working directory if Git is not installed

[Dmitry Gutov]

On 06/09/2023 16:11, Paul Pogonyshev wrote:
> Also consider this from a user point of view. Let's say I have nothing 
> to do with programming at all and don't have Git installed. Someone 
> emails me a cool Emacs package as a tarball and accidentally archives 
> `.git' along, because he is a developer. I unpack it, open some file and 
> now my Emacs warns me that Git is not installed. But why? I didn't even 
> ask it to do anything. It /itself/ decided to do something (because it 
> saw a `.git' directory), failed (Git is not installed), and now warns 
> /me/. From my point of view, if there is a failure for action that 
> wasn't direct result of user order, Emacs should stay silent.

There was a previous discussion on this on the bug tracker which I can't 
find now. If someone is more able than me, it would be a welcome help.

I think Lars was involved, and the general argument was that if .git is 
present, the user is plausibly interested in using our VC features and 
might not understand which programs might be needed to be installed for 
that to work. Or just forget about that aspect e.g. on a new machine. So 
we help them with those warnings.

> In other words, I think there are two sane options here:
> 
> 1) Simply check if Git is installed before doing anything Git-related 
> from `find-file-hook' callback. If it is not installed, just silently 
> don't do anything. Reserve errors and warnings until the user actually 
> ask you to do something Git-related, not simply to open a file.

I don't mind this solution as well, and it has its logic.

So if others like this approach we could do such change (roughly, we'd 
need to capture errors in all 'registered' functions at least, reraise 
as vc-backend-program-missing, and then catch them -- and only them -- 
silently at the top level).

bug#65763: Error opening a file from a Git working directory if Git is not installed

[Eli Zaretskii]

> Date: Wed, 6 Sep 2023 17:31:05 +0300
> Cc: Eli Zaretskii <eliz@gnu.org>, 65763@debbugs.gnu.org
> From: Dmitry Gutov <dmitry@gutov.dev>
> 
> On 06/09/2023 16:11, Paul Pogonyshev wrote:
> > Also consider this from a user point of view. Let's say I have nothing 
> > to do with programming at all and don't have Git installed. Someone 
> > emails me a cool Emacs package as a tarball and accidentally archives 
> > `.git' along, because he is a developer. I unpack it, open some file and 
> > now my Emacs warns me that Git is not installed. But why? I didn't even 
> > ask it to do anything. It /itself/ decided to do something (because it 
> > saw a `.git' directory), failed (Git is not installed), and now warns 
> > /me/. From my point of view, if there is a failure for action that 
> > wasn't direct result of user order, Emacs should stay silent.
> 
> There was a previous discussion on this on the bug tracker which I can't 
> find now. If someone is more able than me, it would be a welcome help.
> 
> I think Lars was involved, and the general argument was that if .git is 
> present, the user is plausibly interested in using our VC features and 
> might not understand which programs might be needed to be installed for 
> that to work. Or just forget about that aspect e.g. on a new machine. So 
> we help them with those warnings.
> 
> > In other words, I think there are two sane options here:
> > 
> > 1) Simply check if Git is installed before doing anything Git-related 
> > from `find-file-hook' callback. If it is not installed, just silently 
> > don't do anything. Reserve errors and warnings until the user actually 
> > ask you to do something Git-related, not simply to open a file.
> 
> I don't mind this solution as well, and it has its logic.
> 
> So if others like this approach we could do such change (roughly, we'd 
> need to capture errors in all 'registered' functions at least, reraise 
> as vc-backend-program-missing, and then catch them -- and only them -- 
> silently at the top level).

I prefer to emit the messages, just make them more user-friendly
("searching for program" "no such file or directory" is not very
friendly, IMO), and suppress the error signaling even when
debug-on-error is non-nil.  The reason is simple: we are talking only
about Git not being installed or accessible here, but the problems
could be different and more obscure: Git could be present but
inoperable for some weird misconfiguration reason, or some other
problem could happen.  So "if it is not installed, just silently don't
do anything" is not a complete solution, and the "silently" part loses
information which users may wish to have presented to them.

bug#65763: Error opening a file from a Git working directory if Git is not installed

[Dmitry Gutov]

On 06/09/2023 18:46, Eli Zaretskii wrote:
> I prefer to emit the messages, just make them more user-friendly
> ("searching for program" "no such file or directory" is not very
> friendly, IMO), and suppress the error signaling even when
> debug-on-error is non-nil.  The reason is simple: we are talking only
> about Git not being installed or accessible here, but the problems
> could be different and more obscure: Git could be present but
> inoperable for some weird misconfiguration reason, or some other
> problem could happen.  So "if it is not installed, just silently don't
> do anything" is not a complete solution, and the "silently" part loses
> information which users may wish to have presented to them.

That also sounds good to me.

bug#65763: Error opening a file from a Git working directory if Git is not installed

[Eli Zaretskii]

> Date: Wed, 6 Sep 2023 18:59:00 +0300
> Cc: pogonyshev@gmail.com, 65763@debbugs.gnu.org
> From: Dmitry Gutov <dmitry@gutov.dev>
> 
> On 06/09/2023 18:46, Eli Zaretskii wrote:
> > I prefer to emit the messages, just make them more user-friendly
> > ("searching for program" "no such file or directory" is not very
> > friendly, IMO), and suppress the error signaling even when
> > debug-on-error is non-nil.  The reason is simple: we are talking only
> > about Git not being installed or accessible here, but the problems
> > could be different and more obscure: Git could be present but
> > inoperable for some weird misconfiguration reason, or some other
> > problem could happen.  So "if it is not installed, just silently don't
> > do anything" is not a complete solution, and the "silently" part loses
> > information which users may wish to have presented to them.
> 
> That also sounds good to me.

There's something strange going on here: even if I wrap the call to
vc-backend with ignore-errors (see the patch below), I still see the
error message, and running with debug-on-error enters the debugger:

  ./emacs --batch --eval "(let ((debug-on-error t)) (setf vc-git-program \"git-is-not-installed\") (find-file-noselect \"README\"))"
  Debugger entered--Lisp error: (file-missing "Searching for program" "No such file or directory" "git-is-not-installed")
    call-process("git-is-not-installed" nil (t nil) nil "--no-pager" "ls-files" "-c" "-z" "--" "src/README")
    apply(call-process "git-is-not-installed" nil (t nil) nil ("--no-pager" "ls-files" "-c" "-z" "--" "src/README"))
    process-file("git-is-not-installed" nil (t nil) nil "--no-pager" "ls-files" "-c" "-z" "--" "src/README")
    apply(process-file "git-is-not-installed" nil (t nil) nil "--no-pager" "ls-files" ("-c" "-z" "--" "src/README"))
    vc-git--call((t nil) "ls-files" "-c" "-z" "--" "src/README")
    apply(vc-git--call (t nil) "ls-files" ("-c" "-z" "--" "src/README"))
    vc-git--out-ok("ls-files" "-c" "-z" "--" "src/README")
    vc-git-registered("d:/gnu/git/emacs/trunk/src/README")
    vc-git-registered("d:/gnu/git/emacs/trunk/src/README")
    apply(vc-git-registered "d:/gnu/git/emacs/trunk/src/README")
    vc-call-backend(Git registered "d:/gnu/git/emacs/trunk/src/README")
    #f(compiled-function (b) #<bytecode 0x1b9bb8e1e734a02c>)(Git)
    mapc(#f(compiled-function (b) #<bytecode 0x1b9bb8e1e734a02c>) (RCS CVS SVN SCCS SRC Bzr Git Hg))
    vc-registered("d:/gnu/git/emacs/trunk/src/README")
    vc-backend("d:/gnu/git/emacs/trunk/src/README")
    vc-refresh-state()
    run-hooks(find-file-hook)
    after-find-file(nil t)
    find-file-noselect-1(#<buffer README> "d:/gnu/git/emacs/trunk/src/README" nil nil "d:/gnu/git/emacs/trunk/src/README" (8444249302351936 1817014650))
    find-file-noselect("README")
    (let ((debug-on-error t)) (setf vc-git-program "git-is-not-installed") (find-file-noselect "README"))
    eval((let ((debug-on-error t)) (setf vc-git-program "git-is-not-installed") (find-file-noselect "README")) t)
    command-line-1(("--eval" "(let ((debug-on-error t)) (setf vc-git-program \"gi..."))
    command-line()
    normal-top-level()

It looks like condition-case is not working in this case?  Or what am
I missing?

Stefan, can you help?

Here's the patch I installed to try this, and it still produces the
backtrace shown above, which clearly tells the error was signaled by
subroutines called by vc-backend:

diff --git a/lisp/vc/vc-hooks.el b/lisp/vc/vc-hooks.el
index a4de0a6..0715236 100644
--- a/lisp/vc/vc-hooks.el
+++ b/lisp/vc/vc-hooks.el
@@ -799,7 +799,7 @@ vc-refresh-state
     (add-hook 'vc-mode-line-hook #'vc-mode-line nil t)
     (let (backend)
       (cond
-       ((setq backend (with-demoted-errors "VC refresh error: %S"
+       ((setq backend (ignore-errors
                         (vc-backend buffer-file-name)))
         ;; Let the backend setup any buffer-local things he needs.
         (vc-call-backend backend 'find-file-hook)

bug#65763: Error opening a file from a Git working directory if Git is not installed

[Dmitry Gutov]

On 10/09/2023 09:26, Eli Zaretskii wrote:
> diff --git a/lisp/vc/vc-hooks.el b/lisp/vc/vc-hooks.el
> index a4de0a6..0715236 100644
> --- a/lisp/vc/vc-hooks.el
> +++ b/lisp/vc/vc-hooks.el
> @@ -799,7 +799,7 @@ vc-refresh-state
>       (add-hook 'vc-mode-line-hook #'vc-mode-line nil t)
>       (let (backend)
>         (cond
> -       ((setq backend (with-demoted-errors "VC refresh error: %S"
> +       ((setq backend (ignore-errors
>                           (vc-backend buffer-file-name)))
>           ;; Let the backend setup any buffer-local things he needs.
>           (vc-call-backend backend 'find-file-hook)

This seems to work better:

diff --git a/lisp/vc/vc-hooks.el b/lisp/vc/vc-hooks.el
index a4de0a6e791..53271d9d3c2 100644
--- a/lisp/vc/vc-hooks.el
+++ b/lisp/vc/vc-hooks.el
@@ -799,8 +799,9 @@ vc-refresh-state
      (add-hook 'vc-mode-line-hook #'vc-mode-line nil t)
      (let (backend)
        (cond
-       ((setq backend (with-demoted-errors "VC refresh error: %S"
-                        (vc-backend buffer-file-name)))
+       ((setq backend (let (debug-on-error)
+                        (with-demoted-errors "VC refresh error: %S"
+                          (vc-backend buffer-file-name))))
          ;; Let the backend setup any buffer-local things he needs.
          (vc-call-backend backend 'find-file-hook)
  	;; Compute the state and put it in the mode line.

bug#65763: Error opening a file from a Git working directory if Git is not installed

[Eli Zaretskii]

> Date: Sun, 10 Sep 2023 16:21:00 +0300
> Cc: pogonyshev@gmail.com, 65763@debbugs.gnu.org
> From: Dmitry Gutov <dmitry@gutov.dev>
> 
> On 10/09/2023 09:26, Eli Zaretskii wrote:
> > diff --git a/lisp/vc/vc-hooks.el b/lisp/vc/vc-hooks.el
> > index a4de0a6..0715236 100644
> > --- a/lisp/vc/vc-hooks.el
> > +++ b/lisp/vc/vc-hooks.el
> > @@ -799,7 +799,7 @@ vc-refresh-state
> >       (add-hook 'vc-mode-line-hook #'vc-mode-line nil t)
> >       (let (backend)
> >         (cond
> > -       ((setq backend (with-demoted-errors "VC refresh error: %S"
> > +       ((setq backend (ignore-errors
> >                           (vc-backend buffer-file-name)))
> >           ;; Let the backend setup any buffer-local things he needs.
> >           (vc-call-backend backend 'find-file-hook)
> 
> This seems to work better:

That'd be fine by me, but I'd still want to understand why
ignore-errors didn't work.

Thanks.

bug#65763: Error opening a file from a Git working directory if Git is not installed

[Paul Pogonyshev]

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

-       ((setq backend (with-demoted-errors "VC refresh error: %S"
-                        (vc-backend buffer-file-name)))
+       ((setq backend (let (debug-on-error)
+                        (with-demoted-errors "VC refresh error: %S"
+                          (vc-backend buffer-file-name))))

From my experience, you should always comment such things. Or add an
automated test ensuring it. Or even better, do both. It is extremely
non-obvious why this is needed at all and someone can easily undo your
changes a few years later.

Paul

On Sun, 10 Sept 2023 at 15:40, Eli Zaretskii <eliz@gnu.org> wrote:

> > Date: Sun, 10 Sep 2023 16:21:00 +0300
> > Cc: pogonyshev@gmail.com, 65763@debbugs.gnu.org
> > From: Dmitry Gutov <dmitry@gutov.dev>
> >
> > On 10/09/2023 09:26, Eli Zaretskii wrote:
> > > diff --git a/lisp/vc/vc-hooks.el b/lisp/vc/vc-hooks.el
> > > index a4de0a6..0715236 100644
> > > --- a/lisp/vc/vc-hooks.el
> > > +++ b/lisp/vc/vc-hooks.el
> > > @@ -799,7 +799,7 @@ vc-refresh-state
> > >       (add-hook 'vc-mode-line-hook #'vc-mode-line nil t)
> > >       (let (backend)
> > >         (cond
> > > -       ((setq backend (with-demoted-errors "VC refresh error: %S"
> > > +       ((setq backend (ignore-errors
> > >                           (vc-backend buffer-file-name)))
> > >           ;; Let the backend setup any buffer-local things he needs.
> > >           (vc-call-backend backend 'find-file-hook)
> >
> > This seems to work better:
>
> That'd be fine by me, but I'd still want to understand why
> ignore-errors didn't work.
>
> Thanks.
>

[-- Attachment #2: Type: text/html, Size: 2521 bytes --]

bug#65763: Error opening a file from a Git working directory if Git is not installed

[Eli Zaretskii]

> From: Paul Pogonyshev <pogonyshev@gmail.com>
> Date: Sun, 10 Sep 2023 19:36:47 +0200
> Cc: Dmitry Gutov <dmitry@gutov.dev>, monnier@iro.umontreal.ca, 65763@debbugs.gnu.org
> 
> -       ((setq backend (with-demoted-errors "VC refresh error: %S"
> -                        (vc-backend buffer-file-name)))
> +       ((setq backend (let (debug-on-error)
> +                        (with-demoted-errors "VC refresh error: %S"
> +                          (vc-backend buffer-file-name))))
> 
> From my experience, you should always comment such things. Or add an automated test ensuring it.
> Or even better, do both. It is extremely non-obvious why this is needed at all and someone can easily
> undo your changes a few years later.

Yes, if this (or anything like it) goes in, we will comment it.  We
just haven't yet decided whether this will be the solution.

bug#65763: Error opening a file from a Git working directory if Git is not installed

[Eli Zaretskii]

> Date: Wed, 6 Sep 2023 15:35:33 +0300
> Cc: 65763@debbugs.gnu.org
> From: Dmitry Gutov <dmitry@gutov.dev>
> 
> On 06/09/2023 15:13, Eli Zaretskii wrote:
> > Well, we could then protect the execution of the problematic form "by
> > hand" by using condition-case-unless-debug.  Dmitry, WDYT?
> 
> Maybe the solution is to use the straight condition-case rather than 
> condition-case-unless-debug? Because otherwise as long as 
> condition-case-unless-debug is used, we would always have this problem.

Sorry, I forgot that debug-on-error affects
condition-case-unless-debug as well.

> Rewriting with-demoted-errors is not an option, of course, but we could 
> create a special, shorted version of it for vc.

Yes, something like that.  Shouldn't be hard, I think.

> Another option, though, is to rewrite the ERT tests in question: e.g. to 
> bind vc-handled-backends to nil, or to some other value if the presence 
> of certain VC programs is known and expected in advance.

Yes, but that needs to be done in every test...