bug#4599: 23.1.50; VC (CVS) fails to show directory status

bug#4599: 23.1.50; VC (CVS) fails to show directory status

[Tim Van Holder]

As of this morning's CVS build, C-x v d for a CVS-controlled directory
results in the following error message, leaving the message line with
the [waiting] marker:

error in process sentinel: Wrong type argument: arrayp, nil

*Messages* also has

error in process sentinel: vc-dir-node-directory: Wrong type argument: arrayp, nil

bug#4599: 23.1.50; VC (CVS) fails to show directory status

[Dan Nicolaescu]

Tim Van Holder <tim.vanholder@gmail.com> writes:

  > As of this morning's CVS build, C-x v d for a CVS-controlled directory
  > results in the following error message, leaving the message line with
  > the [waiting] marker:
  > 
  > error in process sentinel: Wrong type argument: arrayp, nil
  > 
  > *Messages* also has
  > 
  > error in process sentinel: vc-dir-node-directory: Wrong type argument: arrayp, nil

I can't try this at the moment, but I am pretty sure that this change is to blame:

+2009-09-29  Stefan Monnier  <address@hidden>
+
+       * vc-hooks.el (vc-dir-buffers): New var.
+       (vc-state-refresh): New function.
+       (vc-state): Use it.
+       (vc-after-save): Always ask the backend to recompute the new state.
+       Always call vc-dir if necessary, using vc-dir-buffers.
+       * vc-dir.el (vc-dir-prepare-status-buffer, vc-dir-resynch-file):
+       Use vc-dir-buffers.
+       (vc-dir-mode): Use vc-dir-buffers rather than after-save-hook.
+       (vc-dir-prepare-status-buffer, vc-dir-update)
+       (vc-dir-resync-directory-files, vc-dir-resynch-file, vc-dir-mode):
+       Don't call expand-file-name on default-directory.

in particular the removal or the expand-file-name calls is incorrect.
At least ~ needs to be expanded, otherwise things just do not work.

bug#4599: 23.1.50; VC (CVS) fails to show directory status

[Stefan Monnier]

> +2009-09-29  Stefan Monnier  <address@hidden>
> +
> +       * vc-hooks.el (vc-dir-buffers): New var.
> +       (vc-state-refresh): New function.
> +       (vc-state): Use it.
> +       (vc-after-save): Always ask the backend to recompute the new state.
> +       Always call vc-dir if necessary, using vc-dir-buffers.
> +       * vc-dir.el (vc-dir-prepare-status-buffer, vc-dir-resynch-file):
> +       Use vc-dir-buffers.
> +       (vc-dir-mode): Use vc-dir-buffers rather than after-save-hook.
> +       (vc-dir-prepare-status-buffer, vc-dir-update)
> +       (vc-dir-resync-directory-files, vc-dir-resynch-file, vc-dir-mode):
> +       Don't call expand-file-name on default-directory.

> in particular the removal or the expand-file-name calls is incorrect.
> At least ~ needs to be expanded, otherwise things just do not work.

While it might be the immediate cause of the problem, I think the
removal of expand-file-name calls might still be right: the code that
sets default-directory to a value starting with "~/" might be the one
that should be changed.


        Stefan

bug#4599: 23.1.50; VC (CVS) fails to show directory status

[Stefan Monnier]

>> in particular the removal or the expand-file-name calls is incorrect.
>> At least ~ needs to be expanded, otherwise things just do not work.
> While it might be the immediate cause of the problem, I think the
> removal of expand-file-name calls might still be right: the code that
> sets default-directory to a value starting with "~/" might be the one
> that should be changed.

The patch below indeed seems to fix the problem.


        Stefan


=== modified file 'lisp/files.el'
--- lisp/files.el	2009-09-30 14:51:08 +0000
+++ lisp/files.el	2009-10-01 02:15:33 +0000
@@ -648,7 +648,12 @@
   ;; Put the name into directory syntax now,
   ;; because otherwise expand-file-name may give some bad results.
   (setq dir (file-name-as-directory dir))
-  (setq dir (abbreviate-file-name (expand-file-name dir)))
+  ;; We used to additionally call abbreviate-file-name here, for an
+  ;; unknown reason.  Problem is that most buffers are setup
+  ;; without going through cd-absolute and don't call
+  ;; abbreviate-file-name on their default-directory, so the few that
+  ;; do end up using a superficially different directory.
+  (setq dir (expand-file-name dir))
   (if (not (file-directory-p dir))
       (if (file-exists-p dir)
 	  (error "%s is not a directory" dir)

bug#4599: 23.1.50; VC (CVS) fails to show directory status

[Tim Van Holder]

2009/10/1 Stefan Monnier <monnier@iro.umontreal.ca>:
> The patch below indeed seems to fix the problem.
>
> [snip]

Yes, this morning's CVS build works fine again.

I still have a couple outstanding issues with VC (CVS backend):

- Sometimes after checking in (either C-x v v in a file, or from
vc-dir), I get "point before start of properties". The commit will
have succeeded at that point (so it's not a big deal as such), but the
mode line of a buffer visiting a checked-in file will not be updated
to indicate its "unmodified" status (i.e. will have "CVS:1.blah"
instead of "CVS-1.blah").
- Some operations, when done on a directory name in vc-dir fail; for
example, I just pressed '+' when on a directory name (containing only
'need-update' files), and got the error "vc-cvs-merge-news: Couldn't
analyze cvs update result". The *vc* buffer has normal CVS output at
that point ("cvs update: Updating ." followed by a bunch of P lines);
my guess it's that first line that confuses VC. Again, the operation
does indeed succeed, so it's not a big deal.

I'll report these as separate bugs once they annoy me enough.

bug#4599: 23.1.50; VC (CVS) fails to show directory status

[Stefan Monnier]

> - Sometimes after checking in (either C-x v v in a file, or from
> vc-dir), I get "point before start of properties". The commit will

I've received a few reports of "point before start of properties" (not
necessarily linked to VC, most likely it's a much lower-level bug), but
none that I could reproduce.  If you can find a way to reproduce it,
that would be very helpful.

> - Some operations, when done on a directory name in vc-dir fail; for
> example, I just pressed '+' when on a directory name (containing only
> 'need-update' files), and got the error "vc-cvs-merge-news: Couldn't
> analyze cvs update result". The *vc* buffer has normal CVS output at
> that point ("cvs update: Updating ." followed by a bunch of P lines);
> my guess it's that first line that confuses VC. Again, the operation
> does indeed succeed, so it's not a big deal.

Hmm... looks like vc-cvs-merge-news needs a lot of work,


        Stefan

bug#4599: 23.1.50; VC (CVS) fails to show directory status

[Dan Nicolaescu]

Stefan Monnier <monnier@iro.umontreal.ca> writes:

  > > - Some operations, when done on a directory name in vc-dir fail; for
  > > example, I just pressed '+' when on a directory name (containing only
  > > 'need-update' files), and got the error "vc-cvs-merge-news: Couldn't
  > > analyze cvs update result". The *vc* buffer has normal CVS output at
  > > that point ("cvs update: Updating ." followed by a bunch of P lines);
  > > my guess it's that first line that confuses VC. Again, the operation
  > > does indeed succeed, so it's not a big deal.
  > 
  > Hmm... looks like vc-cvs-merge-news needs a lot of work,

And `vc-update' too, the fact that it loops through the files and
updates them one by one is not the best idea, it should probably
delegate this to a VC-backend specific function.