From mboxrd@z Thu Jan 1 00:00:00 1970 Path: news.gmane.org!.POSTED!not-for-mail From: =?UTF-8?Q?G=C3=B6ktu=C4=9F?= Kayaalp Newsgroups: gmane.emacs.bugs Subject: bug#24082: [PATCH] Use =?UTF-8?Q?=E2=80=98cvs_?= =?UTF-8?Q?update=E2=80=99?= instead =?UTF-8?Q?=E2=80=98cvs_?= =?UTF-8?Q?status=E2=80=99?= for CVS *vc-dir* buffers Date: Fri, 07 Oct 2016 22:29:14 +0300 Message-ID: <87y420arg5.fsf@xi.bootis> References: <874m7crwv4.fsf@xi.bootis> NNTP-Posting-Host: blaine.gmane.org Mime-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable X-Trace: blaine.gmane.org 1475867804 21225 195.159.176.226 (7 Oct 2016 19:16:44 GMT) X-Complaints-To: usenet@blaine.gmane.org NNTP-Posting-Date: Fri, 7 Oct 2016 19:16:44 +0000 (UTC) Cc: 24082@debbugs.gnu.org To: Dmitry Gutov Original-X-From: bug-gnu-emacs-bounces+geb-bug-gnu-emacs=m.gmane.org@gnu.org Fri Oct 07 21:16:39 2016 Return-path: Envelope-to: geb-bug-gnu-emacs@m.gmane.org Original-Received: from lists.gnu.org ([208.118.235.17]) by blaine.gmane.org with esmtp (Exim 4.84_2) (envelope-from ) id 1bsacx-00029k-HM for geb-bug-gnu-emacs@m.gmane.org; Fri, 07 Oct 2016 21:16:16 +0200 Original-Received: from localhost ([::1]:38022 helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1bsacw-0002gz-2D for geb-bug-gnu-emacs@m.gmane.org; Fri, 07 Oct 2016 15:16:14 -0400 Original-Received: from eggs.gnu.org ([2001:4830:134:3::10]:35559) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1bsaco-0002gh-Iy for bug-gnu-emacs@gnu.org; Fri, 07 Oct 2016 15:16:08 -0400 Original-Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1bsack-00053L-9h for bug-gnu-emacs@gnu.org; Fri, 07 Oct 2016 15:16:05 -0400 Original-Received: from debbugs.gnu.org ([208.118.235.43]:41873) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1bsack-000539-69 for bug-gnu-emacs@gnu.org; Fri, 07 Oct 2016 15:16:02 -0400 Original-Received: from Debian-debbugs by debbugs.gnu.org with local (Exim 4.84_2) (envelope-from ) id 1bsacj-0004t4-VR for bug-gnu-emacs@gnu.org; Fri, 07 Oct 2016 15:16:01 -0400 X-Loop: help-debbugs@gnu.org Resent-From: =?UTF-8?Q?G=C3=B6ktu=C4=9F?= Kayaalp Original-Sender: "Debbugs-submit" Resent-CC: bug-gnu-emacs@gnu.org Resent-Date: Fri, 07 Oct 2016 19:16:01 +0000 Resent-Message-ID: Resent-Sender: help-debbugs@gnu.org X-GNU-PR-Message: followup 24082 X-GNU-PR-Package: emacs X-GNU-PR-Keywords: confirmed Original-Received: via spool by 24082-submit@debbugs.gnu.org id=B24082.147586773118741 (code B ref 24082); Fri, 07 Oct 2016 19:16:01 +0000 Original-Received: (at 24082) by debbugs.gnu.org; 7 Oct 2016 19:15:31 +0000 Original-Received: from localhost ([127.0.0.1]:48063 helo=debbugs.gnu.org) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1bsacF-0004sC-G9 for submit@debbugs.gnu.org; Fri, 07 Oct 2016 15:15:31 -0400 Original-Received: from relay4-d.mail.gandi.net ([217.70.183.196]:57397) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1bsacD-0004s4-V6 for 24082@debbugs.gnu.org; Fri, 07 Oct 2016 15:15:30 -0400 Original-Received: from mfilter6-d.gandi.net (mfilter6-d.gandi.net [217.70.178.135]) by relay4-d.mail.gandi.net (Postfix) with ESMTP id E6E19172094; Fri, 7 Oct 2016 21:15:28 +0200 (CEST) X-Virus-Scanned: Debian amavisd-new at mfilter6-d.gandi.net Original-Received: from relay4-d.mail.gandi.net ([IPv6:::ffff:217.70.183.196]) by mfilter6-d.gandi.net (mfilter6-d.gandi.net [::ffff:10.0.15.180]) (amavisd-new, port 10024) with ESMTP id TYhUWcBWCTMS; Fri, 7 Oct 2016 21:15:26 +0200 (CEST) X-Originating-IP: 88.235.171.29 Original-Received: from localhost (unknown [88.235.171.29]) (Authenticated sender: self@gkayaalp.com) by relay4-d.mail.gandi.net (Postfix) with ESMTPSA id A584417209B; Fri, 7 Oct 2016 21:15:25 +0200 (CEST) In-Reply-To: (message from Dmitry Gutov on Fri, 7 Oct 2016 02:25:24 +0300) X-BeenThere: debbugs-submit@debbugs.gnu.org X-Mailman-Version: 2.1.18 Precedence: list X-detected-operating-system: by eggs.gnu.org: GNU/Linux 2.2.x-3.x [generic] X-Received-From: 208.118.235.43 X-BeenThere: bug-gnu-emacs@gnu.org List-Id: "Bug reports for GNU Emacs, the Swiss army knife of text editors" List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: bug-gnu-emacs-bounces+geb-bug-gnu-emacs=m.gmane.org@gnu.org Original-Sender: "bug-gnu-emacs" Xref: news.gmane.org gmane.emacs.bugs:124173 Archived-At: On 2016-10-07 02:25:24 AM +0300, Dmitry Gutov 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 :) > > On 28.08.2016 23:17, G=C3=B6ktu=C4=9F Kayaalp wrote: >> Hi, attached is a patch that fixes bug#24082. > > Could you help me reproduce the issue locally first? > > Either by pointing to a publicly available CVS repo with submodules, or=20 > by providing a sequence of commands that would create such repo locally=20 > (we do something like that in vc-tests.el). On a computer with CVS installed, these commands should create a working directory with which the bug can be reproduced: $ cd /tmp $ mkdir cvsroot $ cvs -d /tmp/cvsroot init $ mkdir -p test/subdir $ touch test/testfil test/subdir/subfil $ cd test/ $ cvs -d /tmp/cvsroot/ import $(basename $(pwd)) $(whoami) start $ cd ../ $ mv test test.bkp $ cvs -d /tmp/cvsroot/ co test $ echo test > test/testfil=20 $ echo test > test/subdir/subfil=20 Then, in Emacs, run [ C-x v d /tmp/test RET ], and on =E2=80=98subfil=E2=80= =99, hit =E2=80=98=3D=E2=80=99 (i.e. ask for a diff). >> The reason of the bug was that, in function =E2=80=98vc-cvs-after-dir-st= atus=E2=80=99, >> the CVS status line =E2=80=98cvs status: Examining =E2=80=99 was ex= cluded when the >> function narrows to match, and when it tries to set the local variable >> =E2=80=98subdir=E2=80=99, as it does not find this line, it skips settin= g it. As >> =E2=80=98subdir=E2=80=99 defaults to =E2=80=98default-directory=E2=80=99= , which is previously set to >> repo root (i.e. the argument to function =E2=80=98vc-dir=E2=80=99, when = =E2=80=98subdir=E2=80=99 is used >> to construct the relative path to file, concatenating it with the >> already-known file base name, it returns the basename, i.e. in >> the form =E2=80=98name.ext=E2=80=99, with no directory path. This becau= se it constructs >> the relative path like =E2=80=98(file-relative-name basename subdir)=E2= =80=99. > > Could we just fix that, to address this specific bug? The output for the =E2=80=98cvs status=E2=80=99 command does not provide ad= equate 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 . | =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D | File: testfil Status: Locally Modified |=20 | Working revision: 1.1.1.1 Fri Oct 7 18:46:53 2016 | Repository revision: 1.1.1.1 /tmp/cvsroot/test/testfil,v | Sticky Tag: (none) | Sticky Date: (none) | Sticky Options: (none) |=20 | cvs status: Examining subdir | =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D | File: subfil Status: Locally Modified |=20 | Working revision: 1.1.1.1 Fri Oct 7 18:46:53 2016 | Repository revision: 1.1.1.1 /tmp/cvsroot/test/subdir/subfil,v | Sticky Tag: (none) | Sticky Date: (none) | Sticky Options: (none) `---- The code was trying to rememeber the =E2=80=98cvs status: Examining =E2=80=99 line in order to reconstruct the path, a method that's fragile and which requires a lot of regexp magic. =E2=80=98cvs update=E2=80=99, instea= d has a very filter-friendly and determinable output syntax also imitated by other version control software (output for the same checkout): ,---- | $ cvs -fn update -dP | cvs update: Updating . | M testfil | cvs update: Updating subdir | M subdir/subfil `---- The =E2=80=98-n=E2=80=99 argument makes sure the =E2=80=98cvs update=E2=80= =99 command does not alter in any way the working directory (i.e. the checkout), and -f surpresses the user CVS config (~/.cvsrc). I actually sort-a-kind-a fixed the =E2=80=98cvs status=E2=80=99 version ini= tially, but it's waay slower than using update. I don't have that fix anymore, but as I said, using =E2=80=98cvs update=E2=80=99 is more robust, simpler, and = faster. In order to fix the =E2=80=98cvs status=E2=80=99 method, I read the relevant f= iles 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 =E2=80=98 Repository version:...=E2=80=99 lines for each file to find i= ts relative path to the checkout's root directory, ending up with a complex and fragile hack, that's also slow and incomplete. >> The patch uses =E2=80=98cvs update=E2=80=99 command instead. The implem= entation was >> already there, commented out. I enabled and corrected it. The result >> is more correct than the =E2=80=98cvs status=E2=80=99 approach, i.e. inc= ludes >> unregistered and missing, and is faster compared to =E2=80=98cvs status= =E2=80=99 way. > > If that change is mostly unrelated to the reported bug, could you send=20 > it as two separate patches? That actually is the fix. =E2=80=98cvs status=E2=80=99 output does not len= d itself to being computed in a simple, reliable manner. The patched function's first half was the implementation of an incomplete and buggy parser for the abovementioned command, and the other half was commented out code to parse the output of =E2=80=98cvs update=E2=80=99. > Further, do you have any inkling why the 'cvs update' implementation was= =20 > commented out? Did it require a particular recent version of CVS, maybe? > > We can ask Dan if you don't. > 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 as I can't see why it was commented out. The related commits are: 798dafb4 and 769303ae, both don't say anything about why. --=20 =C4=B0. G=C3=B6ktu=C4=9F Kayaalp. http://gkayaalp.com/