Proposed fix for file-saving bug (VC queries remote repository).

[Karl Fogel]

I know the fix I want to make here, but the code looks like it's
intentionally organized the way it is, so I thought I'd check.

First, this is the bug in today's Emacs (38aaf904c7 on master) -- it's
rather high-severity, since it interferes with saving files:

If you try save a modified buffer visiting an SVN-controlled file
("README.md" in this example), but you have no connectivity to the
upstream repository, the save will spin for a while and then error:

  Running svn --non-interactive status -u README.md...FAILED (status 1)

The cause is clear in the code.  Saving the buffer invokes this call
chain:

  (vc-after-save)
    ==> (vc-state-refresh "/path/to/README.md" 'SVN)
      ==> (vc-call-backend 'SVN 'state "/path/to/README.md")
        ==> (vc-svn-state "/path/to/README.md")
          ==> (vc-svn-command t 0 "/path/to/README.md" "status" "-u")

The problem is the "-u" flag there.  As an option to "svn status", "-u"
means "ask the remote repository for any updates".  If you don't have
network access to the remote repository, svn will hang for a while.

In the Emacs Lisp VC code, the problem is that when `vc-call-backend'
dispatches to `vc-svn-state', the latter has no way of receiving the
`localp' flag:

  (defun vc-svn-state (file &optional localp)
    "SVN-specific version of `vc-state'."
    (let (process-file-side-effects)
      (with-temp-buffer
        (cd (file-name-directory file))
        (vc-svn-command t 0 file "status" (if localp "-v" "-u"))
        (vc-svn-parse-status file))))

In the call path, there's not really any way to insert that flag either.

I see two possible solutions here:

  1) Remove the optional `local' flag from `vc-svn-state' and 
     just always pass "-v" to "svn status", never "-u".

  2) Create a new backend action called `local-state' or `quick-state',
     and have `vc-state-refresh' pass that instead of `state'.

I prefer (1), but maybe that's because I don't know where VC is actually
using that "-u" in `vc-svn-state'?  On the other hand, I don't see any
explicit callers of `vc-svn-state' in Emacs, other than
`vc-call-backend', so maybe the "-u" is just obsolete now?

Here's why I think the solution is one of the above two:

The naive patch for this bug would be to add the `local' flag earlier in
the call chain (I've attached an example of the naive patch, just to
show why it doesn't quite work).  The problem is that while we know what
the `local' flag means for `vc-svn-state' in the SVN-specific backend,
we can't just pass it to other backends hoping it will have the same
effect -- in fact, other backends don't even have an optional `local'
argument to their state actions (see `vc-git-state' and `vc-bzr-state'
for example).  So presumably it would just cause an error for other
backends ("wrong number of arguments").

So, how about it?  Can I just remove the non-local option from
`vc-svn-state' and solve this the simple way?  Pretty please? :-)

Best,
-Karl

vc-file-saving-bug-naive-patch.txt
Description: Naive patch (DO NOT APPLY) for VC bug under discussion.