all messages for Emacs-related lists mirrored at yhetil.org
 help / color / mirror / code / Atom feed
* bug#4179: vc-merge with svn: syntax error in revision arg.
@ 2009-08-17 17:34 Dieter Wilhelm
  2009-08-20  2:59 ` Dan Nicolaescu
  0 siblings, 1 reply; 9+ messages in thread
From: Dieter Wilhelm @ 2009-08-17 17:34 UTC (permalink / raw)
  To: bug-gnu-emacs


Please write in English if possible, because the Emacs maintainers
usually do not have translators to read other languages for them.

Your bug report will be posted to the bug-gnu-emacs@gnu.org mailing list,
and to the gnu.emacs.bug news group.

Please describe exactly what actions triggered the bug
and the precise symptoms of the bug:
------------------------------------------------------------

Hi,

when running `vc-merge' for a subversion controlled file (this happens
also with Emacs 23.1.50 by the way) with an integer argument (I'd like
to specify two svn revision numbers), I'm tumbling into this:

svn: Syntax error in revision argument ':10'

(What I'd like to submit to svn is this: svn merge -r '10:8' ...)

vc-merge (in vc.el) thinks that an integer is a branch ID instead of a
svn revision # because of the following function:

(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))))

Thanks for your good work

       Dieter

------------------------------------------------------------
If Emacs crashed, and you have the Emacs process in the gdb debugger,
please include the output from the following gdb commands:
    `bt full' and `xbacktrace'.
If you would like to further debug the crash, please read the file
/usr/share/emacs/22.2/etc/DEBUG for instructions.


In GNU Emacs 22.2.1 (i486-pc-linux-gnu, GTK+ Version 2.12.11)
 of 2008-11-09 on raven, modified by Debian
Windowing system distributor `The X.Org Foundation', version 11.0.10402000
configured using `configure  '--build=i486-linux-gnu' '--host=i486-linux-gnu' '--prefix=/usr' '--sharedstatedir=/var/lib' '--libexecdir=/usr/lib' '--localstatedir=/var/lib' '--infodir=/usr/share/info' '--mandir=/usr/share/man' '--with-pop=yes' '--enable-locallisppath=/etc/emacs22:/etc/emacs:/usr/local/share/emacs/22.2/site-lisp:/usr/local/share/emacs/site-lisp:/usr/share/emacs/22.2/site-lisp:/usr/share/emacs/site-lisp:/usr/share/emacs/22.2/leim' '--with-x=yes' '--with-x-toolkit=gtk' '--with-toolkit-scroll-bars' 'build_alias=i486-linux-gnu' 'host_alias=i486-linux-gnu' 'CFLAGS=-DDEBIAN -g -O2' 'LDFLAGS=-g' 'CPPFLAGS=''

Important settings:
  value of $LC_ALL: nil
  value of $LC_COLLATE: nil
  value of $LC_CTYPE: nil
  value of $LC_MESSAGES: nil
  value of $LC_MONETARY: nil
  value of $LC_NUMERIC: nil
  value of $LC_TIME: nil
  value of $LANG: en_US.UTF-8
  locale-coding-system: utf-8
  default-enable-multibyte-characters: t

Major mode: Fundamental

Minor modes in effect:
  shell-dirtrack-mode: t
  display-time-mode: t
  show-paren-mode: t
  tooltip-mode: t
  mouse-wheel-mode: t
  file-name-shadow-mode: t
  global-font-lock-mode: t
  font-lock-mode: t
  blink-cursor-mode: t
  unify-8859-on-encoding-mode: t
  utf-translate-cjk-mode: t
  auto-compression-mode: t
  column-number-mode: t
  line-number-mode: t
  transient-mark-mode: t
  abbrev-mode: t

Recent input:
<switch-frame> <switch-frame> <switch-frame> <switch-frame> 
<switch-frame> <switch-frame> <switch-frame> <f4> C-x 
C-f d i s c C-r C-s C-s <return> y C-x v l C-x v m 
1 0 <return> M-x r e p o <tab> r t <tab> <return>

Recent messages:
Loading mule-util...done
Marking holidays...done
Marking diary entries...done
For information about GNU Emacs and the GNU system, type C-h C-a.
Please type y, n, or !: 
Running svn in the background... done
Loading log-view...done
vc-do-command: Running svn...FAILED (status 1)
Making completion list...
Loading emacsbug...done






^ permalink raw reply	[flat|nested] 9+ messages in thread

* bug#4179: vc-merge with svn: syntax error in revision arg.
  2009-08-17 17:34 bug#4179: vc-merge with svn: syntax error in revision arg Dieter Wilhelm
@ 2009-08-20  2:59 ` Dan Nicolaescu
  2009-08-25 18:06   ` Stefan Monnier
  0 siblings, 1 reply; 9+ messages in thread
From: Dan Nicolaescu @ 2009-08-20  2:59 UTC (permalink / raw)
  To: Dieter Wilhelm; +Cc: 4179

Dieter Wilhelm <dieter@duenenhof-wilhelm.de> writes:

  > Hi,
  > 
  > when running `vc-merge' for a subversion controlled file (this happens
  > also with Emacs 23.1.50 by the way) with an integer argument (I'd like
  > to specify two svn revision numbers), I'm tumbling into this:
  > 
  > svn: Syntax error in revision argument ':10'
  > 
  > (What I'd like to submit to svn is this: svn merge -r '10:8' ...)
  > 
  > vc-merge (in vc.el) thinks that an integer is a branch ID instead of a
  > svn revision # because of the following function:
  > 
  > (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))))

Good catch!  This function is from the time when RCS style revision
numbers where the only thing supported, here's what the comment in the
code says:

;; 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.


Stefan, how would you go about making these functions backend specific:

(defun vc-trunk-p (rev)
(defun vc-branch-p (rev)
(defun vc-branch-part (rev)
(defun vc-minor-part (rev)

It seems to me that only `vc-branch-p' needs to be backend specific, the
rest are only used by RCS and CVS. 

vc-default-{previous,next}-revision use them, but they can go into
vc-rcs (or vc-cvs) and make one use the other (there's a precedent,
vc-cvs-comment-history uses an RCS backend function...).

Do you see an elegant way of dealing with this?





^ permalink raw reply	[flat|nested] 9+ messages in thread

* bug#4179: vc-merge with svn: syntax error in revision arg.
  2009-08-20  2:59 ` Dan Nicolaescu
@ 2009-08-25 18:06   ` Stefan Monnier
  2009-08-26 18:04     ` Dan Nicolaescu
  0 siblings, 1 reply; 9+ messages in thread
From: Stefan Monnier @ 2009-08-25 18:06 UTC (permalink / raw)
  To: Dan Nicolaescu; +Cc: Dieter Wilhelm, 4179

> Stefan, how would you go about making these functions backend specific:

> (defun vc-trunk-p (rev)
> (defun vc-branch-p (rev)
> (defun vc-branch-part (rev)
> (defun vc-minor-part (rev)

Not sure: for some backends (svn, bzr, darcs at least), they simply
don't make any sense.

> It seems to me that only `vc-branch-p' needs to be backend specific, the
> rest are only used by RCS and CVS.

That sounds right.

> vc-default-{previous,next}-revision use them, but they can go into
> vc-rcs (or vc-cvs) and make one use the other (there's a precedent,
> vc-cvs-comment-history uses an RCS backend function...).

> Do you see an elegant way of dealing with this?

Move them to vc-rcs.


        Stefan





^ permalink raw reply	[flat|nested] 9+ messages in thread

* bug#4179: vc-merge with svn: syntax error in revision arg.
  2009-08-25 18:06   ` Stefan Monnier
@ 2009-08-26 18:04     ` Dan Nicolaescu
  2021-08-27  3:06       ` Lars Ingebrigtsen
  0 siblings, 1 reply; 9+ messages in thread
From: Dan Nicolaescu @ 2009-08-26 18:04 UTC (permalink / raw)
  To: Stefan Monnier; +Cc: Dieter Wilhelm, 4179

Stefan Monnier <monnier@IRO.UMontreal.CA> writes:

  > > vc-default-{previous,next}-revision use them, but they can go into
  > > vc-rcs (or vc-cvs) and make one use the other (there's a precedent,
  > > vc-cvs-comment-history uses an RCS backend function...).
  > 
  > > Do you see an elegant way of dealing with this?
  > 
  > Move them to vc-rcs.

I did that.

  > > Stefan, how would you go about making these functions backend specific:
  > 
  > > (defun vc-trunk-p (rev)
  > > (defun vc-branch-p (rev)
  > > (defun vc-branch-part (rev)
  > > (defun vc-minor-part (rev)
  > 
  > Not sure: for some backends (svn, bzr, darcs at least), they simply
  > don't make any sense.
  > 
  > > It seems to me that only `vc-branch-p' needs to be backend specific, the
  > > rest are only used by RCS and CVS.
  > 
  > That sounds right.

Also moved + renamed vc-trunk-p and vc-minor-part to vc-rcs.el, they are
only used in that file now.

All the above turned out to be a good cleanup, but unfortunately
not necessarily related to the problem in this bug report.

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?





^ permalink raw reply	[flat|nested] 9+ messages in thread

* bug#4179: vc-merge with svn: syntax error in revision arg.
  2009-08-26 18:04     ` Dan Nicolaescu
@ 2021-08-27  3:06       ` Lars Ingebrigtsen
  2021-08-27 20:11         ` H. Dieter Wilhelm
  2021-08-29  0:50         ` Dmitry Gutov
  0 siblings, 2 replies; 9+ messages in thread
From: Lars Ingebrigtsen @ 2021-08-27  3:06 UTC (permalink / raw)
  To: Dan Nicolaescu; +Cc: Dieter Wilhelm, 4179, Stefan Monnier, Dmitry Gutov

Dan Nicolaescu <dann@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?

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.)

I've added Dmitry to the CCs; perhaps he has some comments.

-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no





^ permalink raw reply	[flat|nested] 9+ messages in thread

* bug#4179: vc-merge with svn: syntax error in revision arg.
  2021-08-27  3:06       ` Lars Ingebrigtsen
@ 2021-08-27 20:11         ` H. Dieter Wilhelm
  2021-08-29  0:50         ` Dmitry Gutov
  1 sibling, 0 replies; 9+ messages in thread
From: H. Dieter Wilhelm @ 2021-08-27 20:11 UTC (permalink / raw)
  To: Lars Ingebrigtsen; +Cc: 4179, Dan Nicolaescu, Stefan Monnier, Dmitry Gutov

Lars Ingebrigtsen <larsi@gnus.org> writes:

> (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?

Sorry, I can't tell, started that year with git (had to start...) and
lost sight of the svn issue.

Thank you for your detective work, though

      Dieter
-- 
Best wishes
H. Dieter Wilhelm
Zwingenberg, Germany





^ permalink raw reply	[flat|nested] 9+ messages in thread

* bug#4179: vc-merge with svn: syntax error in revision arg.
  2021-08-27  3:06       ` Lars Ingebrigtsen
  2021-08-27 20:11         ` H. Dieter Wilhelm
@ 2021-08-29  0:50         ` Dmitry Gutov
  2021-08-29 20:42           ` Lars Ingebrigtsen
  1 sibling, 1 reply; 9+ messages in thread
From: Dmitry Gutov @ 2021-08-29  0:50 UTC (permalink / raw)
  To: Lars Ingebrigtsen, Dan Nicolaescu; +Cc: Dieter Wilhelm, Stefan Monnier, 4179

On 27.08.2021 06:06, Lars Ingebrigtsen wrote:
> Dan Nicolaescu <dann@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.





^ permalink raw reply	[flat|nested] 9+ messages in thread

* bug#4179: vc-merge with svn: syntax error in revision arg.
  2021-08-29  0:50         ` Dmitry Gutov
@ 2021-08-29 20:42           ` Lars Ingebrigtsen
  2021-08-29 21:09             ` Dmitry Gutov
  0 siblings, 1 reply; 9+ messages in thread
From: Lars Ingebrigtsen @ 2021-08-29 20:42 UTC (permalink / raw)
  To: Dmitry Gutov; +Cc: Dieter Wilhelm, Dan Nicolaescu, Stefan Monnier, 4179

Dmitry Gutov <dgutov@yandex.ru> writes:

> 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".

OK; I think we can close this bug report, then.  (Now done.)

>> 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 done.

> 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.

Perhaps open a new bug report for this and see whether anybody
volunteers to implement and test?

-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no





^ permalink raw reply	[flat|nested] 9+ messages in thread

* bug#4179: vc-merge with svn: syntax error in revision arg.
  2021-08-29 20:42           ` Lars Ingebrigtsen
@ 2021-08-29 21:09             ` Dmitry Gutov
  0 siblings, 0 replies; 9+ messages in thread
From: Dmitry Gutov @ 2021-08-29 21:09 UTC (permalink / raw)
  To: Lars Ingebrigtsen; +Cc: Dieter Wilhelm, Dan Nicolaescu, Stefan Monnier, 4179

On 29.08.2021 23:42, Lars Ingebrigtsen wrote:
>> 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.
> Perhaps open a new bug report for this and see whether anybody
> volunteers to implement and test?

Filed bug#50258.





^ permalink raw reply	[flat|nested] 9+ messages in thread

end of thread, other threads:[~2021-08-29 21:09 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2009-08-17 17:34 bug#4179: vc-merge with svn: syntax error in revision arg Dieter Wilhelm
2009-08-20  2:59 ` Dan Nicolaescu
2009-08-25 18:06   ` Stefan Monnier
2009-08-26 18:04     ` Dan Nicolaescu
2021-08-27  3:06       ` Lars Ingebrigtsen
2021-08-27 20:11         ` H. Dieter Wilhelm
2021-08-29  0:50         ` Dmitry Gutov
2021-08-29 20:42           ` Lars Ingebrigtsen
2021-08-29 21:09             ` Dmitry Gutov

Code repositories for project(s) associated with this external index

	https://git.savannah.gnu.org/cgit/emacs.git
	https://git.savannah.gnu.org/cgit/emacs/org-mode.git

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.