bug#7850: 23.2.91; emacs runs svn status -v after every file save

bug#7850: 23.2.91; emacs runs svn status -v after every file save

[Steve Revilak]

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

This bug report will be sent to the Free Software Foundation,
not to your local site managers!
Please write in English if possible, because the Emacs maintainers
usually do not have translators to read other languages for them.

Your report will be posted to the bug-gnu-emacs@gnu.org mailing list
and the gnu.emacs.bug news group, and at http://debbugs.gnu.org.

Please describe exactly what actions triggered the bug
and the precise symptoms of the bug.  If you can, give
a recipe starting from `emacs -Q':

---------------------------------

I would like to report a minor bug in emacs 23.2.91.  When editing a
file that is kept in svn version control, emacs runs "svn status -V
FILE" each time the file's buffer is saved.  Other vc backends do not
exhibit this behavior.  For example, the git backend does not run "git
status" on save, and the cvs backend does not run "cvs status" on
save.

The invocation of "svn status -V" does not cause functional problems.
Instead, it makes save-buffer seem sluggish when editing files in
subversion version control, because a shell command is run after each
file save.

Steps to reproduce:

  + create a file called svn.txt, which was checked out of a subversion
    repository.  

  + emacs -Q -nw

  + evaluate the expression "(setq vc-command-messages t)" in the
    *scratch* buffer.

  + C-x C-f /path/to/svn.txt RET

  + edit the buffer, then C-x C-s to save the buffer

In the *Messages* buffer, this produces

   Saving file /home/srevilak/tmp/src/svn.txt...
   Wrote /home/srevilak/tmp/src/svn.txt
   Running svn status -v svn.txt in foreground...
   Running svn status -v svn.txt...OK = 0

  + optionally, repeat this process with a file name git.txt (kept
    under git version control), or cvs.txt (kept under cvs version
    control).  Verify that saving git.txt (or cvs.txt) does NOT produce
    vc-command-messages in the *Messages* buffer.


I have two builds of emacs-23.2.91: one on OpenSUSE 11.3, and other on
Mac OS X 10.6.  Both exhibit the same behavior (with svn 1.6).

I've checked before-save-hook and after-save-hook, and both are nil.

If there is additional information that would be helpful, I would be
happy to provide it.  I would also be happy to test patches.

I realize that the latest pretest is emacs 23.2.92.  I've been waiting
to see the outcome of
http://lists.gnu.org/archive/html/emacs-devel/2011-01/msg00477.html
before installing the latest 23.2.92.

Kind regards.

Steve Revilak

---------------------------------

If Emacs crashed, and you have the Emacs process in the gdb debugger,
please include the output from the following gdb commands:
     `bt full' and `xbacktrace'.
For information about debugging Emacs, please read the file
/usr/local/emacs-23.2.91/share/emacs/23.2.91/etc/DEBUG.


In GNU Emacs 23.2.91.1 (x86_64-unknown-linux-gnu, GTK+ Version 2.20.1)
  of 2011-01-01 on sunny
Windowing system distributor `The X.Org Foundation', version 11.0.10800000
configured using `configure  '--prefix=/usr/local/emacs-23.2.91' 'CFLAGS=-g -O2 -fno-optimize-sibling-calls''

Important settings:
   value of $LC_ALL: nil
   value of $LC_COLLATE: C
   value of $LC_CTYPE: nil
   value of $LC_MESSAGES: nil
   value of $LC_MONETARY: nil
   value of $LC_NUMERIC: nil
   value of $LC_TIME: nil
   value of $LANG: en_US.UTF-8
   value of $XMODIFIERS: @im=local
   locale-coding-system: utf-8-unix
   default enable-multibyte-characters: t

Major mode: Shell

Minor modes in effect:
   shell-dirtrack-mode: t
   diff-auto-refine-mode: t
   display-time-mode: t
   tooltip-mode: t
   mouse-wheel-mode: t
   menu-bar-mode: t
   file-name-shadow-mode: t
   global-font-lock-mode: t
   font-lock-mode: t
   blink-cursor-mode: t
   auto-encryption-mode: t
   auto-compression-mode: t
   column-number-mode: t
   line-number-mode: t

Recent input:
<up> <up> <up> <up> <up> <up> <up> <up> <up> <up> <up> 
<up> <up> <up> <up> <up> <up> <up> <up> <up> <up> <up> 
<up> <up> <up> <up> <down> <down> C-a <tab> <down> 
C-c C-c d <return> <tab> <down> C-c C-c s <return> 
<tab> <down> C-c C-c d d <return> <up> C-c C-c s r 
<return> C-x C-s <down> <tab> C-x C-s <escape> > C-a 
<tab> M-> <return> <return> <return> C-c C-x C-r C-SPC 
<down> <down> <down> <down> <down> <down> <down> <down> 
<down> <down> <down> <down> <down> <down> <down> <down> 
<down> <down> <down> <down> <down> <down> <down> <down> 
<down> <down> <down> C-w <backspace> <backspace> <backspace> 
C-x C-s M-x v c - d i <tab> r <return> <return> <down> 
<down> <down> <down> <down> <down> <down> <down> m 

Recent messages:
Making completion list...
Running git update-index --refresh . in background... done
Running git diff-index --relative -z -M HEAD -- . in background... done
Running git ls-files -z -o --directory --no-empty-directory --exclude-standard -- . in background... done
Mark set
Press C-c C-c when you are done editing.
Enter a change comment.  Type C-c C-c when done
Checking in /home/srevilak/filename-redacted.org...
Running git commit -m ad... --only -- filename-redacted.org...OK = 0
Checking in /home/srevilak/filename-redacted.org...done

Load-path shadows:
None found.

Features:
(shadow sort mail-extr message sendmail ecomplete rfc822 mml mml-sec
password-cache mm-decode mm-bodies mm-encode mailcap mail-parse rfc2231
rfc2047 rfc2045 qp ietf-drums mailabbrev nnheader gnus-util netrc
mm-util mail-prsvr gmm-utils wid-edit mailheader canlock hashcash
mail-utils emacsbug tabify org-table ansi-color shell comint log-edit
ring pcvs-util add-log diff-mode vc-bzr sha1 hex-util vc-sccs vc-svn
vc-cvs vc-rcs vc-dir ewoc vc vc-dispatcher help-mode view rect org-clock
diary-lib diary-loaddefs cal-iso vc-git org-wl org-w3m org-vm org-rmail
org-mhe org-mew org-irc org-jsinfo org-infojs org-html org-exp
org-exp-blocks org-info org-gnus org-bibtex org-bbdb regexp-opt cal-menu
calendar cal-loaddefs org-agenda org byte-opt warnings bytecomp
byte-compile advice help-fns advice-preload org-footnote org-src
org-list org-faces org-compat org-macs easymenu time-date noutline
outline easy-mmode edmacro kmacro paren time tooltip ediff-hook vc-hooks
lisp-float-type mwheel x-win x-dnd font-setting tool-bar dnd fontset
image fringe lisp-mode register page menu-bar rfn-eshadow timer select
scroll-bar mldrag 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 loaddefs button minibuffer faces cus-face files
text-properties overlay md5 base64 format env code-pages mule custom
widget hashtable-print-readable backquote make-network-process dbusbind
system-font-setting font-render-setting gtk x-toolkit x multi-tty emacs)

[-- Attachment #2: Type: application/pgp-signature, Size: 198 bytes --]

bug#7850: 23.2.91; emacs runs svn status -v after every file save

[Glenn Morris]

Steve Revilak wrote:

> When editing a file that is kept in svn version control, emacs runs
> "svn status -V FILE" each time the file's buffer is saved. Other vc
> backends do not exhibit this behavior. For example, the git backend
> does not run "git status" on save, and the cvs backend does not run
> "cvs status" on save.

This can be changed by altering the definition of vc-svn-state-heuristic.
Simply copying the CVS one seems fine (?). I don't think this is an
appropriate change for the coming Emacs 23.3 though.


*** lisp/vc-svn.el	2011-01-02 23:50:46 +0000
--- lisp/vc-svn.el	2011-01-21 02:28:37 +0000
***************
*** 156,162 ****
  
  (defun vc-svn-state-heuristic (file)
    "SVN-specific state heuristic."
!   (vc-svn-state file 'local))
  
  ;; FIXME it would be better not to have the "remote" argument,
  ;; but to distinguish the two output formats based on content.
--- 156,170 ----
  
  (defun vc-svn-state-heuristic (file)
    "SVN-specific state heuristic."
!   ;; If the file has not changed since checkout, consider it `up-to-date'.
!   ;; Otherwise consider it `edited'.  Copied from vc-cvs-state-heuristic.
!   (let ((checkout-time (vc-file-getprop file 'vc-checkout-time))
!         (lastmod (nth 5 (file-attributes file))))
!     (cond
!      ((equal checkout-time lastmod) 'up-to-date)
!      ((string= (vc-working-revision file) "0") 'added)
!      ((null checkout-time) 'unregistered)
!      (t 'edited))))
  
  ;; FIXME it would be better not to have the "remote" argument,
  ;; but to distinguish the two output formats based on content.

bug#7850: 23.2.91; emacs runs svn status -v after every file save

[Glenn Morris]

Though this wouldn't handle modified properties (cf bug#7861).
Perhaps one should parse .svn/entries (?) like the bzr heuristic does.
I guess since svn is generally fast, no-one's bothered to do this.

bug#7850: 23.2.91; emacs runs svn status -v after every file save

[Chong Yidong]

Glenn Morris <rgm@gnu.org> writes:

> Though this wouldn't handle modified properties (cf bug#7861).
> Perhaps one should parse .svn/entries (?) like the bzr heuristic does.

What are the implications if vc-svn-state-heuristic does not detect
modified properties?

bug#7850: 23.2.91; emacs runs svn status -v after every file save

[Steve Revilak]

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

> Though this wouldn't handle modified properties (cf bug#7861).
> Perhaps one should parse .svn/entries (?) like the bzr heuristic does.
> I guess since svn is generally fast, no-one's bothered to do this.

Glenn,

Thanks for looking into this.  When I wrote 7850, I wasn't thinking of
svn properties.  Once you bring properties into the picture, running
"svn status -v" makes more sense.


>What are the implications if vc-svn-state-heuristic does not detect
>modified properties?

http://debbugs.gnu.org/cgi/bugreport.cgi?bug=7861 might answer that
question.  As far as svn is concerned, properties are separate from
the contents of files, and can change independently.  Changing file
properties does not change the file mtime.

Here is a small example, where a property changes, but the file
contents do not.

~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
# foo.txt has a modified property
$ svn status
  M      foo.txt

# svn diff shows that only properties have changed
$ svn diff foo.txt 

Property changes on: foo.txt
___________________________________________________________________
Modified: svn:keywords
    - Id Revision Date Author URL
    + Id
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

I am surprised that vc-dir does not notice this change (this is the
subject of 7861).  I am equally surprised that

   C-x C-f foo.txt
   C-x v v

tells me "Fileset is up-to-date".  So, you really cannot use emacs to
commit a file that has only property changes.  This also implies that
emacs is not getting much benefit from running svn status after file
saves.

Would it make sense to run "svn status" during vc actions, but not
during file saves?

One thing I noticed while writing this example: when I change a
property on foo.txt, svn 1.6 creates a .svn/props/foo.txt.svn-work,
which contains a description of the property change.  If I commit (or
revert) foo.txt, then svn deletes .svn/props/foo.txt.svn-work.  This
might be a cheap way to test for properties changes.

Steve


[-- Attachment #2: Type: application/pgp-signature, Size: 198 bytes --]

bug#7850: 23.2.91; emacs runs svn status -v after every file save

[Glenn Morris]

Version: 24.4

Glenn Morris wrote:

> This can be changed by altering the definition of vc-svn-state-heuristic.
> Simply copying the CVS one seems fine (?). I don't think this is an
> appropriate change for the coming Emacs 23.3 though.
>
>
> *** lisp/vc-svn.el	2011-01-02 23:50:46 +0000
> --- lisp/vc-svn.el	2011-01-21 02:28:37 +0000
> ***************
> *** 156,162 ****
>   
>   (defun vc-svn-state-heuristic (file)
>     "SVN-specific state heuristic."
> !   (vc-svn-state file 'local))
>   
>   ;; FIXME it would be better not to have the "remote" argument,
>   ;; but to distinguish the two output formats based on content.
> --- 156,170 ----
>   
>   (defun vc-svn-state-heuristic (file)
>     "SVN-specific state heuristic."
> !   ;; If the file has not changed since checkout, consider it `up-to-date'.
> !   ;; Otherwise consider it `edited'.  Copied from vc-cvs-state-heuristic.
> !   (let ((checkout-time (vc-file-getprop file 'vc-checkout-time))
> !         (lastmod (nth 5 (file-attributes file))))
> !     (cond
> !      ((equal checkout-time lastmod) 'up-to-date)
> !      ((string= (vc-working-revision file) "0") 'added)
> !      ((null checkout-time) 'unregistered)
> !      (t 'edited))))
>   
>   ;; FIXME it would be better not to have the "remote" argument,
>   ;; but to distinguish the two output formats based on content.

I was asked to commit this, so I looked at it again and concluded that
the fact that it does not handle svn properties is irrelevant.

AFAICS (I find vc.el rather opaque), it is _only_ called from
vc-state-refresh in vc-after-save. When a file is first visited,
vc-svn-registered is called, which explicitly calls "svn status".

I think that the only consequence of this change is that eg adding a
character, deleting it, and then saving the file will now mark the
buffer as VC modified. Previously, it did not. Presumably CVS has always
behaved like this, and I don't recall seeing any complaints about that.