vc-revert broken for Subversion files

vc-revert broken for Subversion files

[Bob Rogers]

   To reproduce:

   1.  Make a small change to a file registered under Subversion,
"foo.el" for example.

   2.  "emacs -Q foo.el"

   3.  "C-x v u".  The operation will fail with the following message:

	vc-stay-local-p: Symbol's function definition is void: remove-if-not

I am using an emacs built from the trunk about 36 hours ago.  FWIW, this
worked in my previous trunk build two week ago.  vc-revert also still
works with CVS files.

   TIA,

					-- Bob Rogers
					   http://rgrjr.dyndns.org/

Re: vc-revert broken for Subversion files

[timotheus]

[-- Attachment #1.1: Type: text/plain, Size: 667 bytes --]

Bob Rogers <rogers-emacs@rgrjr.dyndns.org> writes:

>    To reproduce:
>
>    1.  Make a small change to a file registered under Subversion,
> "foo.el" for example.
>
>    2.  "emacs -Q foo.el"
>
>    3.  "C-x v u".  The operation will fail with the following message:
>
> 	vc-stay-local-p: Symbol's function definition is void: remove-if-not
>
> I am using an emacs built from the trunk about 36 hours ago.  FWIW, this
> worked in my previous trunk build two week ago.  vc-revert also still
> works with CVS files.
>
>    TIA,
>
> 					-- Bob Rogers
> 					   http://rgrjr.dyndns.org/

First, please report if `vc-svn' is listed with:
    M-x list-load-path-shadows

[-- Attachment #1.2: Type: application/pgp-signature, Size: 188 bytes --]

[-- Attachment #2: Type: text/plain, Size: 142 bytes --]

_______________________________________________
Emacs-devel mailing list
Emacs-devel@gnu.org
http://lists.gnu.org/mailman/listinfo/emacs-devel

Re: vc-revert broken for Subversion files

[Dan Nicolaescu]

Bob Rogers <rogers-emacs@rgrjr.dyndns.org> writes:

  >    To reproduce:
  > 
  >    1.  Make a small change to a file registered under Subversion,
  > "foo.el" for example.
  > 
  >    2.  "emacs -Q foo.el"
  > 
  >    3.  "C-x v u".  The operation will fail with the following message:
  > 
  > 	vc-stay-local-p: Symbol's function definition is void: remove-if-not

This is a problem introduced a few days ago by changes to VC. vc now
uses remove-if-not which is a function in cl-seq.el 
You can work around this by doing: 
M-x load-library RET cl RET
M-x load-library RET cl-seq RET

Hopefully someone will fix the code soon... 

Re: vc-revert broken for Subversion files

[Bob Rogers]

   From: timotheus <timotheus@tstotts.net>
   Date: Sun, 22 Jul 2007 14:22:22 -0400

   Bob Rogers <rogers-emacs@rgrjr.dyndns.org> writes:

   >    To reproduce:
   >
   >    1.  Make a small change to a file registered under Subversion,
   > "foo.el" for example.
   >
   >    2.  "emacs -Q foo.el"
   >
   >    3.  "C-x v u".  The operation will fail with the following message:
   >
   > 	vc-stay-local-p: Symbol's function definition is void: remove-if-not
   >
   > I am using an emacs built from the trunk about 36 hours ago.  FWIW, this
   > worked in my previous trunk build two week ago.  vc-revert also still
   > works with CVS files.
   >
   >    TIA,
   >
   > 					-- Bob Rogers
   > 					   http://rgrjr.dyndns.org/

   First, please report if `vc-svn' is listed with:
       M-x list-load-path-shadows

It is not.  (Could it be otherwise with "emacs -Q"?)

					-- Bob

Re: vc-revert broken for Subversion files

[Thien-Thi Nguyen]

() Dan Nicolaescu <dann@ics.uci.edu>
() Sun, 22 Jul 2007 11:59:37 -0700

   This is a problem introduced a few days ago by changes to VC. vc now
   uses remove-if-not which is a function in cl-seq.el 
   You can work around this by doing: 
   M-x load-library RET cl RET
   M-x load-library RET cl-seq RET

   Hopefully someone will fix the code soon... 

a quick scan shows another problem in the same func where remove-if-not
was introduced (vc-stay-local-p): docstrings sez "return non-nil", but
if FILE is a list, the function returns a symbol, either `yes' or `no'.
furthermore, the docstring makes no mention of the case when FILE is a
list.  furthermore, there is no ChangeLog entry.  this last problem is
very grave, causing the quick scan mentioned at the beginning of this
paragraph to balloon into lots of time-wasting C-x v g digging.  grrr.

below is a patch.  here is a ChangeLog entry:

	* vc-hooks.el (vc-stay-local-p): Fix bug: Avoid remove-if-not.
	Also, when FILE is a list, return non-nil if any of the files
	are should stay local.  Update docstring.

note to whoever really understands the intent of vc-stay-local-p: the
opposite sense for FILE as list may be what you really want; please
review carefully.

thi


____________________________________________________________
*** vc-hooks.el	22 Jul 2007 19:04:10 -0000	1.195
--- vc-hooks.el	22 Jul 2007 21:42:22 -0000
***************
*** 159,167 ****
  
  (defun vc-stay-local-p (file)
    "Return non-nil if VC should stay local when handling FILE.
! This uses the `repository-hostname' backend operation."
    (if (listp file)
!       (if (remove-if-not (lambda (x) (not (vc-stay-local-p x))) file) 'no 'yes)
      (let* ((backend (vc-backend file))
  	   (sym (vc-make-backend-sym backend 'stay-local))
  	   (stay-local (if (boundp sym) (symbol-value sym) t)))
--- 159,169 ----
  
  (defun vc-stay-local-p (file)
    "Return non-nil if VC should stay local when handling FILE.
! This uses the `repository-hostname' backend operation.
! If FILE is a list of files, return non-nil if any of them
! individually should stay local."
    (if (listp file)
!       (delq nil (mapcar 'vc-stay-local-p file))
      (let* ((backend (vc-backend file))
  	   (sym (vc-make-backend-sym backend 'stay-local))
  	   (stay-local (if (boundp sym) (symbol-value sym) t)))

Re: vc-revert broken for Subversion files

[Richard Stallman]

Please install your patch.

    a quick scan shows another problem in the same func where remove-if-not
    was introduced (vc-stay-local-p): docstrings sez "return non-nil", but
    if FILE is a list, the function returns a symbol, either `yes' or `no'.
    furthermore, the docstring makes no mention of the case when FILE is a
    list.  furthermore, there is no ChangeLog entry.  this last problem is
    very grave,

That sounds bad.  Was this part of esr's recent changes?

I see that the change log entries for them are not up to the usual
standards.  Was the change part of this?

    2007-07-18  Eric S. Raymond  <esr@snark.thyrsus.com>

	    * vc-hooks.el: Generalize stay-local-p to operate on lists of
	    files.  Change two keybindings to point to new function names.
	    * vc-arch.el, vc-bzr.el, vc-cvs.el, vc-hg.el, vc-mcvs.el, vc-rcs.el,
	    vc-sccs.el, vc-svn.el: These now implement the NewVC-fileset.
	    * vc.el: Adapted for NewVC-fileset, but no functional changes yet.

Re: vc-revert broken for Subversion files

[Richard Stallman]

       * vc-hooks.el: Generalize stay-local-p to operate on lists of
       files.  Change two keybindings to point to new function names.
       * vc-arch.el, vc-bzr.el, vc-cvs.el, vc-hg.el, vc-mcvs.el, vc-rcs.el,
       vc-sccs.el, vc-svn.el: These now implement the NewVC-fileset.
       * vc.el: Adapted for NewVC-fileset, but no functional changes yet.

    yes.

    unfortunately, even w/ this ChangeLog entry, i am unable to determine the
    intent (sense) of the "generalization".  my doubt is as follows:

I have asked him to put in a clear and detailed change log entry.

    when applied to many files, i can imagine two equally valid policies:
    (a) if any file of the list is stay-local-p, enable optimizations.
    (b) if all files of the list are stay-local-p, enable optimizations.

    as is (just checked in), the code supports (a) but not (b).  could
    maintainers of client code give this some thought, and confirm or revise
    the code accordingly?  it comes down to "any" vs "all" semantics, of
    which i'm slightly paranoid, having been bitten in a previous life...

    the more i think about it, the more (b) would make sense (AND gate),
    since optimizations may not be possible if any one file in the list is
    NOT stay-local-p.  this boolean stuff is (simply ;-) weird...

Could you write to him and Stefan to discuss the question?

Re: vc-revert broken for Subversion files

[Stefan Monnier]

>     when applied to many files, i can imagine two equally valid policies:
>     (a) if any file of the list is stay-local-p, enable optimizations.
>     (b) if all files of the list are stay-local-p, enable optimizations.

stay-local-p is used to determine not whether to use some optimizations but
whether to use some heuristic rather than the real thing.

I.e. (vc-stay-local-p FILE) should be non-nil if FILE's repository is on
a far away server, i.e. if querying the status of FILE will take a long
time.  This is the used to avoid querying the server when it's not
absolutely needed, and also triggers the use of async operations where
supported.

So I think (a) is right, because if any of the files are on a distant
repository, the resulting command will take a long time, even if all the
other files's repositories are nearby.


        Stefan

Re: vc-revert broken for Subversion files

[Thien-Thi Nguyen]

() Stefan Monnier <monnier@iro.umontreal.ca>
() Wed, 25 Jul 2007 15:49:41 -0400

   So I think (a) is right, because if any of the files are on a
   distant repository, the resulting command will take a long
   time, even if all the other files's repositories are nearby.

thanks for explaining the intent.  i'll leave things alone, now.

thi