GNU bug report logs - #4179
vc-merge with svn: syntax error in revision arg.

Previous Next

Full log

Message #27 received at 4179 <at> debbugs.gnu.org (full text, mbox):

From: Dmitry Gutov <dgutov <at> yandex.ru>
To: Lars Ingebrigtsen <larsi <at> gnus.org>, Dan Nicolaescu <dann <at> ics.uci.edu>
Cc: Dieter Wilhelm <dieter <at> duenenhof-wilhelm.de>,
 Stefan Monnier <monnier <at> IRO.UMontreal.CA>, 4179 <at> debbugs.gnu.org
Subject: Re: bug#4179: vc-merge with svn: syntax error in revision arg.
Date: Sun, 29 Aug 2021 03:50:04 +0300

On 27.08.2021 06:06, Lars Ingebrigtsen wrote:
> Dan Nicolaescu <dann <at> ics.uci.edu> writes:
> 
>> We have this code in `vc-merge'
>>
>>      (if (string= first-revision "")
>>          (setq status (vc-call-backend backend 'merge-news file))
>>        (if (not (vc-find-backend-function backend 'merge))
>>          (error "Sorry, merging is not implemented for %s" backend)
>>          (if (not (vc-branch-p first-revision))
>>              (setq second-revision
>>                      (read-string "Second revision: "
>>                           (concat (vc-branch-part first-revision) ".")))
>>             ;; We want to merge an entire branch.  Set revisions
>>             ;; accordingly, so that vc-BACKEND-merge understands us.
>>             (setq second-revision first-revision)
>>             ;; first-revision must be the starting point of the branch
>>            (setq first-revision (vc-branch-part first-revision)))
>>
>> `vc-branch-p' can be made VC backend specific.
>> But what should we do with `vc-branch-part'?
>> What should be the new structure of this code?
> 
> (I'm going through old bug reports that unfortunately weren't
> resolved at the time.)
> 
> This code has changed a lot in the 12 years since this was discussed --
> does anybody know whether the originally reported problem has been fixed
> or not?

Hard for me to say: it's SVN. Someone who has easy access to such a 
server should try the original scenario.

But there have indeed been some changes, including commit d17bae903902. 
Which mentions fixing "a layering violation that caused bad behavior 
with SVN".

> Also:
> 
> ;; functions that operate on RCS revision numbers.  This code should
> ;; also be moved into the backends.  It stays for now, however, since
> ;; it is used in code below.
> (defun vc-branch-p (rev)
>    "Return t if REV is a branch revision."
>    (not (eq nil (string-match "\\`[0-9]+\\(\\.[0-9]+\\.[0-9]+\\)*\\'" rev))))
> 
> ;;;###autoload
> (defun vc-branch-part (rev)
>    "Return the branch part of a revision number REV."
>    (let ((index (string-match "\\.[0-9]+\\'" rev)))
>      (when index
>        (substring rev 0 index))))
> 
> The comment there is wrong -- it's no longer used in vc.el.  Perhaps
> these two functions should be moved to vc-rcs.el (and renamed (with
> obsolete aliases))?   (They're only used in vc-rcs.el and vc-vcs.el.)

Yes, they should. Also, vc-merge should call (vc-find-backend-function 
backend 'merge-file) instead of (vc-find-backend-function backend 
'merge). They only seem equivalent for now by lucky accident. I'd do 
that change myself now, but it's similarly hard for me to quickly test 
any of the affected backends, and this operation is not covered by 
vc-tests.el.

Previous Next

GNU bug tracking system
Copyright (C) 1999 Darren O. Benham, 1997,2003 nCipher Corporation Ltd, 1994-97 Ian Jackson.