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

[Karl Fogel]

Should have added:

I expected this bug to be related to this recent thread:

  "Re: master 2a81c5d 1/2: Confine vc-stay-local to CVS, because it was 
unusable in SVN."
  http://lists.gnu.org/archive/html/emacs-devel/2014-12/threads.html#00028

...but when I looked over the thread, it actually didn't seem that
related.  However, the fact that vc-stay-local got pushed down into the
CVS backend indicates that the `local' flag remaining in `vc-svn-state'
might just be an oversight.

I'm happy to remove the flag, and the "-u" functionality that it
suppresses, unless anyone objects.

-K

>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? :-)