regression in C-x v = in latest versions

regression in C-x v = in latest versions

[Paul Pogonyshev]

Hi,

With latest Emacs versions (yesterday SVN) I get very annoying behaviour
of C-x v = on SVN repository.  Now it doesn't use offline SVN copy of
modified files anymore, instead it always asks the server to diff.

E.g.  I open up an SVN-managed file, and type C-o C-x v =.  Previously,
I'd get instant difference of one added empty line, against offline
clean copy.  Now instead I get

	Running svn diff -r NNN FILENAME in the background...

in the echo area.  I guess the problem is in specifying the exact
revision number.

Paul

Re: regression in C-x v = in latest versions

[Paul Pogonyshev]

I wrote:
> With latest Emacs versions (yesterday SVN) I get very annoying behaviour
> of C-x v = on SVN repository.  Now it doesn't use offline SVN copy of
> modified files anymore, instead it always asks the server to diff.
> 
> E.g.  I open up an SVN-managed file, and type C-o C-x v =.  Previously,
> I'd get instant difference of one added empty line, against offline
> clean copy.  Now instead I get
> 
> 	Running svn diff -r NNN FILENAME in the background...
> 
> in the echo area.  I guess the problem is in specifying the exact
> revision number.

I understand that bugs in tetris are more important, but can anyone
look at this?  I still have the same problem with today's CVS.

Paul

Re: regression in C-x v = in latest versions

[Richard Stallman]

Thanks for reminding people about this bug.  Please keep reminding people
every week or two.

Re: regression in C-x v = in latest versions

[Nick Roberts]

 > Thanks for reminding people about this bug.  Please keep reminding people
 > every week or two.

Rather than adding to our spam why not just put this bug in FOR-RELEASE?

-- 
Nick                                           http://www.inet.net.nz/~nickrob

Re: regression in C-x v = in latest versions

[Richard Stallman]

    Rather than adding to our spam why not just put this bug in FOR-RELEASE?

I put bugs in FOR-RELEASE after a couple of reminders.  But I continue
the reminders as well, because people people are not paying enough
attention to these bugs.

What do I need to do -- turn off installation in the trunk until
these bugs are fixed in Emacs 22?

Re: regression in C-x v = in latest versions

[Chong Yidong]

Richard Stallman <rms@gnu.org> writes:

>     Rather than adding to our spam why not just put this bug in FOR-RELEASE?
>
> I put bugs in FOR-RELEASE after a couple of reminders.  But I continue
> the reminders as well, because people people are not paying enough
> attention to these bugs.
>
> What do I need to do -- turn off installation in the trunk until
> these bugs are fixed in Emacs 22?

I suggest putting bugs directly in FOR-RELEASE when you send the first
reminder.  Since Emacs is maintained by volunteers, who work on fixing
bugs when they have the time to do so, it would be helpful to have a
central list of things that need doing.

Re: regression in C-x v = in latest versions

[Richard Stallman]

    I suggest putting bugs directly in FOR-RELEASE when you send the first
    reminder.  Since Emacs is maintained by volunteers, who work on fixing
    bugs when they have the time to do so, it would be helpful to have a
    central list of things that need doing.

I can do it, but since I am already rather overloaded, would
someone else like to volunteer to do this?

Re: regression in C-x v = in latest versions

[Nick Roberts]

 > I put bugs in FOR-RELEASE after a couple of reminders.  But I continue
 > the reminders as well, because people people are not paying enough
 > attention to these bugs.

Generally, given that emacs-devel is an anarchy, I'm amazed how well things do
get fixed.  I think sometimes we need to find new developers to fix bugs rather
than harrass existing ones.  Perhaps it is worth adding a note in CONTRIBUTE
about FOR-RELEASE.

 > What do I need to do -- turn off installation in the trunk until
 > these bugs are fixed in Emacs 22?

I can't speak for others but this appproach won't encourage me to fix more
bugs.

-- 
Nick                                           http://www.inet.net.nz/~nickrob


*** CONTRIBUTE	27 Jul 2007 09:22:04 +1200	1.10
--- CONTRIBUTE	26 Aug 2007 10:36:10 +1200	
*************** After you have downloaded the CVS source
*** 64,69 ****
--- 64,72 ----
  INSTALL.CVS for build instructions (they differ to some extent from a
  normal build).
  
+ The file FOR-RELEASE in the admin directory lists bugs that must be
+ fixed before the next release.
+ 
  Ref: http://savannah.gnu.org/projects/emacs

Re: regression in C-x v = in latest versions

[Glenn Morris]

Paul Pogonyshev wrote:

> I understand that bugs in tetris are more important, but can anyone
> look at this?  I still have the same problem with today's CVS.

This problem (only in the CVS trunk, therefore not relevant for Emacs
22.2, by the way) was introduced by Eric Raymond's 2007-07-18 change
to VC. The (rather ominous) cvs log entry reads only:

    Put the lower half (the back-end) of NewVC in place. This commit
    makes only the minimum changes needed to get the old vc.el logic
    working with the new back ends.

You may get better response by complaining to him directly, since he
seem to be independent of these lists.

Anyway, it's caused by the removal of this code from vc-svn-diff:

(if (and oldvers (equal oldvers (vc-workfile-version file)))   
  ;; Use nil rather than the current revision because svn
  ;; handles it better (i.e. locally).   
  (setq oldvers nil))

"file" has now been replaced by "files", a list. vc-workfile-version
can easily be generalized to accept a list of files as input, so
here's a possible patch. Though I don't understand how this is
supposed to work now.


*** vc-hooks.el	22 Aug 2007 20:18:34 -0700	1.206
--- vc-hooks.el	25 Aug 2007 13:34:14 -0700	
***************
*** 549,559 ****
  
  (defun vc-workfile-version (file)
    "Return the repository version from which FILE was checked out.
! If FILE is not registered, this function always returns nil."
    (or (vc-file-getprop file 'vc-workfile-version)
        (if (vc-backend file)
            (vc-file-setprop file 'vc-workfile-version
!                            (vc-call workfile-version file)))))
  
  (defun vc-default-registered (backend file)
    "Check if FILE is registered in BACKEND using vc-BACKEND-master-templates."
--- 549,562 ----
  
  (defun vc-workfile-version (file)
    "Return the repository version from which FILE was checked out.
! If FILE is not registered, this function always returns nil.  If
! FILE is a list of files, return a list of repository versions."
!   (if (listp file)
!       (mapcar 'vc-workfile-version file)
      (or (vc-file-getprop file 'vc-workfile-version)
          (if (vc-backend file)
              (vc-file-setprop file 'vc-workfile-version
!                              (vc-call workfile-version file))))))
  
  (defun vc-default-registered (backend file)
    "Check if FILE is registered in BACKEND using vc-BACKEND-master-templates."

*** vc-svn.el	25 Jul 2007 22:29:39 -0700	1.39
--- vc-svn.el	25 Aug 2007 13:33:46 -0700	
***************
*** 399,404 ****
--- 399,413 ----
  
  (defun vc-svn-diff (files &optional oldvers newvers buffer)
    "Get a difference report using SVN between two versions of fileset FILES."
+    (if (and oldvers
+ 	    (catch 'no
+ 	      (dolist (v (vc-workfile-version files))
+ 		(or (equal oldvers v)
+ 		    (throw 'no nil)))
+ 	      t))
+        ;; Use nil rather than the current revision because svn handles it
+        ;; better (i.e. locally).
+        (setq oldvers nil))
    (let* ((switches
  	    (if vc-svn-diff-switches
  		(vc-switches 'SVN 'diff)

Re: regression in C-x v = in latest versions

[Paul Pogonyshev]

Hi, Eric.

In response to my complaint:

  With latest Emacs versions (yesterday SVN) I get very annoying behaviour
  of C-x v = on SVN repository.  Now it doesn't use offline SVN copy of
  modified files anymore, instead it always asks the server to diff.

  E.g.  I open up an SVN-managed file, and type C-o C-x v =.  Previously,
  I'd get instant difference of one added empty line, against offline
  clean copy.  Now instead I get

        Running svn diff -r NNN FILENAME in the background...

  in the echo area.  I guess the problem is in specifying the exact
  revision number.

Glenn Morris pointed to your change and proposed a patch.  Can you please
comment on your initial intensions and whether Glenn's patch is OK?

Paul

Glenn Morris wrote (thanks for reply, BTW):
> This problem (only in the CVS trunk, therefore not relevant for Emacs
> 22.2, by the way) was introduced by Eric Raymond's 2007-07-18 change
> to VC. The (rather ominous) cvs log entry reads only:
> 
>     Put the lower half (the back-end) of NewVC in place. This commit
>     makes only the minimum changes needed to get the old vc.el logic
>     working with the new back ends.
> 
> You may get better response by complaining to him directly, since he
> seem to be independent of these lists.
> 
> Anyway, it's caused by the removal of this code from vc-svn-diff:
> 
> (if (and oldvers (equal oldvers (vc-workfile-version file)))   
>   ;; Use nil rather than the current revision because svn
>   ;; handles it better (i.e. locally).   
>   (setq oldvers nil))
> 
> "file" has now been replaced by "files", a list. vc-workfile-version
> can easily be generalized to accept a list of files as input, so
> here's a possible patch. Though I don't understand how this is
> supposed to work now.
> 
> 
> *** vc-hooks.el	22 Aug 2007 20:18:34 -0700	1.206
> --- vc-hooks.el	25 Aug 2007 13:34:14 -0700	
> ***************
> *** 549,559 ****
>   
>   (defun vc-workfile-version (file)
>     "Return the repository version from which FILE was checked out.
> ! If FILE is not registered, this function always returns nil."
>     (or (vc-file-getprop file 'vc-workfile-version)
>         (if (vc-backend file)
>             (vc-file-setprop file 'vc-workfile-version
> !                            (vc-call workfile-version file)))))
>   
>   (defun vc-default-registered (backend file)
>     "Check if FILE is registered in BACKEND using vc-BACKEND-master-templates."
> --- 549,562 ----
>   
>   (defun vc-workfile-version (file)
>     "Return the repository version from which FILE was checked out.
> ! If FILE is not registered, this function always returns nil.  If
> ! FILE is a list of files, return a list of repository versions."
> !   (if (listp file)
> !       (mapcar 'vc-workfile-version file)
>       (or (vc-file-getprop file 'vc-workfile-version)
>           (if (vc-backend file)
>               (vc-file-setprop file 'vc-workfile-version
> !                              (vc-call workfile-version file))))))
>   
>   (defun vc-default-registered (backend file)
>     "Check if FILE is registered in BACKEND using vc-BACKEND-master-templates."
> 
> *** vc-svn.el	25 Jul 2007 22:29:39 -0700	1.39
> --- vc-svn.el	25 Aug 2007 13:33:46 -0700	
> ***************
> *** 399,404 ****
> --- 399,413 ----
>   
>   (defun vc-svn-diff (files &optional oldvers newvers buffer)
>     "Get a difference report using SVN between two versions of fileset FILES."
> +    (if (and oldvers
> + 	    (catch 'no
> + 	      (dolist (v (vc-workfile-version files))
> + 		(or (equal oldvers v)
> + 		    (throw 'no nil)))
> + 	      t))
> +        ;; Use nil rather than the current revision because svn handles it
> +        ;; better (i.e. locally).
> +        (setq oldvers nil))
>     (let* ((switches
>   	    (if vc-svn-diff-switches
>   		(vc-switches 'SVN 'diff)

Re: regression in C-x v = in latest versions

[Richard Stallman]

    This problem (only in the CVS trunk, therefore not relevant for Emacs
    22.2, by the way) was introduced by Eric Raymond's 2007-07-18 change
    to VC. The (rather ominous) cvs log entry reads only:

	Put the lower half (the back-end) of NewVC in place. This commit
	makes only the minimum changes needed to get the old vc.el logic
	working with the new back ends.

I asked him to provide a proper change log, and he did not do it.
It isn't fair that he is leaving this job to be done by
someone else, but if he won't do it, I think we need someone
else to do it -- for the sake of maintainability.

Stefan, can you do it?  You know the VC code best.

    Anyway, it's caused by the removal of this code from vc-svn-diff:

    (if (and oldvers (equal oldvers (vc-workfile-version file)))   
      ;; Use nil rather than the current revision because svn
      ;; handles it better (i.e. locally).   
      (setq oldvers nil))

    "file" has now been replaced by "files", a list. vc-workfile-version
    can easily be generalized to accept a list of files as input, so
    here's a possible patch. Though I don't understand how this is
    supposed to work now.

I don't understand.  Is this patch your suggested fix, or is it the
patch you say broke things?  Please always be explicit about that.
It is not good to have to guess the answer.

If it is your suggested fix, why do you suggest generalizing
vc-workfile-version?

Stefan, what do you think about how to fix this?

Re: regression in C-x v = in latest versions

[Glenn Morris]

Richard Stallman wrote:

>     (if (and oldvers (equal oldvers (vc-workfile-version file)))   
>       ;; Use nil rather than the current revision because svn
>       ;; handles it better (i.e. locally).   
>       (setq oldvers nil))

esr's removal of the above quoted code caused the problem.

> I don't understand.  Is this patch your suggested fix, or is it the
> patch you say broke things?  Please always be explicit about that.
> It is not good to have to guess the answer.

I really don't see what was unclear about my message. The patch was a
suggested fix.

> If it is your suggested fix, why do you suggest generalizing
> vc-workfile-version?

Because esr's changes seem to have as one of their goals generalizing
VC to be able to handle lists of files (aka filesets) rather than one
file at a time. Eg vc-svn-diff has been changed to handle a list of
files.  This means that the functions it calls need to be changed
likewise.

Re: regression in C-x v = in latest versions

[Richard Stallman]

    > If it is your suggested fix, why do you suggest generalizing
    > vc-workfile-version?

    Because esr's changes seem to have as one of their goals generalizing
    VC to be able to handle lists of files (aka filesets) rather than one
    file at a time. Eg vc-svn-diff has been changed to handle a list of
    files.  This means that the functions it calls need to be changed
    likewise.

These should be two separate questions.  As a first step, could you
fix the bug, without generalizing anything?  Then you could suggest
the other change.

Re: regression in C-x v = in latest versions

[Glenn Morris]

Richard Stallman wrote:

> These should be two separate questions.  As a first step, could you
> fix the bug, without generalizing anything?

You're right. I've installed a simple fix.

Re: regression in C-x v = in latest versions

[Stefan Monnier]

> I asked him to provide a proper change log, and he did not do it.
> It isn't fair that he is leaving this job to be done by
> someone else, but if he won't do it, I think we need someone
> else to do it -- for the sake of maintainability.

> Stefan, can you do it?  You know the VC code best.

I'd rather not do it: this is supposed to be a temporary situation, so if
he's not able to do the maintenance of this intermediate state, I'm unsure
he'll be able to finish the change.


        Stefan

Re: regression in C-x v = in latest versions

[Richard Stallman]

    I'd rather not do it: this is supposed to be a temporary situation, so if
    he's not able to do the maintenance of this intermediate state, I'm unsure
    he'll be able to finish the change.

The need for a valid change log for the change that was already
installed is a permanent need.  Even if he can't do the rest of the
redesign he planned, we still need a proper change log for what was
installed (unless we take it out).  And it is increasingly looking
like ESR is not going to provide that.

Would you please do it?

Re: regression in C-x v = in latest versions

[Chong Yidong]

Richard Stallman <rms@gnu.org> writes:

>     I'd rather not do it: this is supposed to be a temporary situation, so if
>     he's not able to do the maintenance of this intermediate state, I'm unsure
>     he'll be able to finish the change.
>
> The need for a valid change log for the change that was already
> installed is a permanent need.  Even if he can't do the rest of the
> redesign he planned, we still need a proper change log for what was
> installed (unless we take it out).

The alternative, of course, is to revert to the last working version.
If and when, ESR finishes this redesign, it can be checked back in.

Re: regression in C-x v = in latest versions

[Richard Stallman]

    > The need for a valid change log for the change that was already
    > installed is a permanent need.  Even if he can't do the rest of the
    > redesign he planned, we still need a proper change log for what was
    > installed (unless we take it out).

    The alternative, of course, is to revert to the last working version.
    If and when, ESR finishes this redesign, it can be checked back in.

If we need to revert because of problems in the code, then we will.
But I think that isn't necessary -- there are not so many bugs.
I don't want to revert the code merely because of the lack of ChangeLog
data.  It would be better for someone simply to write that.

Re: regression in C-x v = in latest versions

[Dan Nicolaescu]

Richard Stallman <rms@gnu.org> writes:

  >     > The need for a valid change log for the change that was already
  >     > installed is a permanent need.  Even if he can't do the rest of the
  >     > redesign he planned, we still need a proper change log for what was
  >     > installed (unless we take it out).
  > 
  >     The alternative, of course, is to revert to the last working version.
  >     If and when, ESR finishes this redesign, it can be checked back in.
  > 
  > If we need to revert because of problems in the code, then we will.
  > But I think that isn't necessary -- there are not so many bugs.
  > I don't want to revert the code merely because of the lack of ChangeLog
  > data.  It would be better for someone simply to write that.

I rewrote the ChangeLog entry. I hope this helps. 

Re: regression in C-x v = in latest versions

[Stefan Monnier]

> "file" has now been replaced by "files", a list. vc-workfile-version
> can easily be generalized to accept a list of files as input, so
> here's a possible patch. Though I don't understand how this is

I'm not sure it makes sense to make such a change.

OTOH the loop-over-files in vc-svn-diff is probably the right way to do
it indeed (just do the call to vc-workfile-version inside the loop rather
than outside of it).


        Stefan