[Top][All Lists]

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

bug#24082: [PATCH] Use ‘cvs update’ instead ‘cvs status’ for CVS *vc-dir

From: Dmitry Gutov
Subject: bug#24082: [PATCH] Use ‘cvs update’ instead ‘cvs status’ for CVS *vc-dir* buffers
Date: Sat, 8 Oct 2016 04:01:22 +0300
User-agent: Mozilla/5.0 (X11; Linux x86_64; rv:49.0) Gecko/20100101 Thunderbird/49.0

On 07.10.2016 22:29, Göktuğ Kayaalp wrote:
On 2016-10-07 02:25:24 AM +0300, Dmitry Gutov <address@hidden> wrote:
Hi! Sorry for the long wait.

No problem, though actually I'm the one who caused the long delay b/c I
was too late to send in copyright papers, sorry :)

We could have done the review earlier. But anyway...

On a computer with CVS installed, these commands should create a working
directory with which the bug can be reproduced:

Thank you, that works.

The output for the ‘cvs status’ command does not provide adequate
information to reliably and simply construct the full path to each
file.  The output from the abovementioned command in the checkout the
commands I listed created is like follows:

| cvs status: Examining .
| ===================================================================
| File: testfil                 Status: Locally Modified
|    Working revision: Fri Oct  7 18:46:53 2016
|    Repository revision: /tmp/cvsroot/test/testfil,v
|    Sticky Tag:                (none)
|    Sticky Date:               (none)
|    Sticky Options:    (none)
| cvs status: Examining subdir
| ===================================================================
| File: subfil                  Status: Locally Modified
|    Working revision: Fri Oct  7 18:46:53 2016
|    Repository revision: /tmp/cvsroot/test/subdir/subfil,v
|    Sticky Tag:                (none)
|    Sticky Date:               (none)
|    Sticky Options:    (none)

The code was trying to rememeber the ‘cvs status: Examining <directory>’
line in order to reconstruct the path, a method that's fragile and
which requires a lot of regexp magic.

Maybe the problem here is that the output we have to deal with does not contain "Examining subdir" because the current implementation of vc-cvs-dir-status-files passes each individual files name to 'cvs -f status'.

And 'cvs -f status testfil subdir/subfil' does not output that line. I think the most reasonable approach for that solution would be to parse the "Repository revision" lines.

Apparently, vc-cvs-dir-status-files was always broken for this usage, and the problem has surfaced now when we've reimplemented dir-status on top of dir-status-files.

  ‘cvs update’, instead has a very
filter-friendly and determinable output syntax also imitated by other
version control software (output for the same checkout):

That looks reasonable. However, it looks like a significant change, so I'm not sure it's appropriate for Emacs 25.2. We'd like to get *a* fix into 25.2, however.

I actually sort-a-kind-a fixed the ‘cvs status’ version initially, but
it's waay slower than using update.  I don't have that fix anymore, but
as I said, using ‘cvs update’ is more robust, simpler, and faster.  In
order to fix the ‘cvs status’ method, I read the relevant files from the
CVS/ subdir of the checkout, reconstruct the absolute module path using
info from files therewithin, than subtract that from the third column of
the ‘ Repository version:...’ lines for each file to find its relative
path to the checkout's root directory, ending up with a complex and
fragile hack, that's also slow and incomplete.

Couldn't you subtract those values from default-directory instead?

Well, I'm unaware of both the history of the command and why the related
Elisp was commented out, though I do not think that the output format
changed in a long time, or that anything else really changed in CVS
recently :)  I recommend asking someone more knowledgeable than me

OK, let's ask Dan.

In the meantime, here's a simple fix we can consider. This would still adhere to the dir-status-files protocol, but it's probably slower than ideal:

diff --git a/lisp/vc/vc-cvs.el b/lisp/vc/vc-cvs.el
index a2499a2..e949f30 100644
--- a/lisp/vc/vc-cvs.el
+++ b/lisp/vc/vc-cvs.el
@@ -1073,7 +1073,7 @@ vc-cvs-dir-status-files
     (if (and (not files) local (not (eq local 'only-file)))
        (vc-cvs-dir-status-heuristic dir update-function)
       (if (not files) (setq files (vc-expand-dirs (list dir) 'CVS)))
-      (vc-cvs-command (current-buffer) 'async files "-f" "status")
+      (vc-cvs-command (current-buffer) 'async dir "-f" "status")
       ;; Alternative implementation: use the "update" command instead of
       ;; the "status" command.
       ;; (vc-cvs-command (current-buffer) 'async

reply via email to

[Prev in Thread] Current Thread [Next in Thread]