Re: opencvs - fix revision lookups for branches
Yes please. As noted in older thread that XXX block in rcs.c produced side
effects with cvs annotate.
https://marc.info/?l=openbsd-tech=144757775319206=2
On Wed, Jun 22, 2016 at 05:20:01PM +0200, Joris Vink wrote:
> On Wed, Jun 22, 2016 at 09:07:03AM -0600, Todd C. Miller wrote:
> > On Wed, 22 Jun 2016 12:21:56 +0200, Joris Vink wrote:
> > > Index: rcs.c
> > > ===
> > > RCS file: /cvs/src/usr.bin/cvs/rcs.c,v
> > > retrieving revision 1.313
> > > diff -u -p -r1.313 rcs.c
> > > --- rcs.c 5 Nov 2015 09:48:21 - 1.313
> > > +++ rcs.c 22 Jun 2016 09:52:04 -
> > > @@ -1796,17 +1796,13 @@ rcs_rev_getlines(RCSFILE *rfp, RCSNUM *f
> > >
> > > again:
> > > for (;;) {
> > > + if (rdp == NULL)
> > > + break;
> >
> > Wouldn't this be easier to read as:
> >
> > while (rdp != NULL) {
>
> Yes, updated diff below.
>
> .joris
>
> Index: rcs.c
> ===
> RCS file: /cvs/src/usr.bin/cvs/rcs.c,v
> retrieving revision 1.313
> diff -u -p -r1.313 rcs.c
> --- rcs.c 5 Nov 2015 09:48:21 - 1.313
> +++ rcs.c 22 Jun 2016 15:13:14 -
> @@ -1795,18 +1795,11 @@ rcs_rev_getlines(RCSFILE *rfp, RCSNUM *f
> goto done;
>
> again:
> - for (;;) {
> + while (rdp != NULL) {
> if (rdp->rd_next->rn_len != 0) {
> trdp = rcs_findrev(rfp, rdp->rd_next);
> if (trdp == NULL)
> fatal("failed to grab next revision");
> - } else {
> - /*
> - * XXX Fail, although the caller does not always do the
> - * right thing (eg cvs diff when the tree is ahead of
> - * the repository).
> - */
> - break;
> }
>
> if (rdp->rd_tlen == 0) {
> @@ -1857,7 +1850,7 @@ again:
> }
>
> next:
> - if (!rcsnum_differ(rdp->rd_num, frev))
> + if (rdp == NULL || !rcsnum_differ(rdp->rd_num, frev))
> done = 1;
>
> if (RCSNUM_ISBRANCHREV(frev) && done != 1) {
> @@ -2045,6 +2038,7 @@ rcs_rev_getbuf(RCSFILE *rfp, RCSNUM *rev
> struct rcs_delta *rdp;
> struct rcs_lines *lines;
> struct rcs_line *lp, *nlp;
> + char version[RCSNUM_MAXSTR];
> BUF *bp;
>
> rdp = NULL;
> @@ -2057,8 +2051,12 @@ rcs_rev_getbuf(RCSFILE *rfp, RCSNUM *rev
> expmode = rcs_kwexp_get(rfp);
>
> if (!(expmode & RCS_KWEXP_NONE)) {
> - if ((rdp = rcs_findrev(rfp, rev)) == NULL)
> - fatal("could not fetch revision");
> + if ((rdp = rcs_findrev(rfp, rev)) == NULL) {
> + rcsnum_tostr(rev, version, sizeof(version));
> + fatal("could not find desired version %s in %s",
> + version, rfp->rf_path);
> + }
> +
> expand = 1;
> }
> }
>
Re: opencvs - fix revision lookups for branches
On Wed, Jun 22, 2016 at 09:07:03AM -0600, Todd C. Miller wrote:
> On Wed, 22 Jun 2016 12:21:56 +0200, Joris Vink wrote:
> > Index: rcs.c
> > ===
> > RCS file: /cvs/src/usr.bin/cvs/rcs.c,v
> > retrieving revision 1.313
> > diff -u -p -r1.313 rcs.c
> > --- rcs.c 5 Nov 2015 09:48:21 - 1.313
> > +++ rcs.c 22 Jun 2016 09:52:04 -
> > @@ -1796,17 +1796,13 @@ rcs_rev_getlines(RCSFILE *rfp, RCSNUM *f
> >
> > again:
> > for (;;) {
> > + if (rdp == NULL)
> > + break;
>
> Wouldn't this be easier to read as:
>
> while (rdp != NULL) {
Yes, updated diff below.
.joris
Index: rcs.c
===
RCS file: /cvs/src/usr.bin/cvs/rcs.c,v
retrieving revision 1.313
diff -u -p -r1.313 rcs.c
--- rcs.c 5 Nov 2015 09:48:21 - 1.313
+++ rcs.c 22 Jun 2016 15:13:14 -
@@ -1795,18 +1795,11 @@ rcs_rev_getlines(RCSFILE *rfp, RCSNUM *f
goto done;
again:
- for (;;) {
+ while (rdp != NULL) {
if (rdp->rd_next->rn_len != 0) {
trdp = rcs_findrev(rfp, rdp->rd_next);
if (trdp == NULL)
fatal("failed to grab next revision");
- } else {
- /*
-* XXX Fail, although the caller does not always do the
-* right thing (eg cvs diff when the tree is ahead of
-* the repository).
-*/
- break;
}
if (rdp->rd_tlen == 0) {
@@ -1857,7 +1850,7 @@ again:
}
next:
- if (!rcsnum_differ(rdp->rd_num, frev))
+ if (rdp == NULL || !rcsnum_differ(rdp->rd_num, frev))
done = 1;
if (RCSNUM_ISBRANCHREV(frev) && done != 1) {
@@ -2045,6 +2038,7 @@ rcs_rev_getbuf(RCSFILE *rfp, RCSNUM *rev
struct rcs_delta *rdp;
struct rcs_lines *lines;
struct rcs_line *lp, *nlp;
+ char version[RCSNUM_MAXSTR];
BUF *bp;
rdp = NULL;
@@ -2057,8 +2051,12 @@ rcs_rev_getbuf(RCSFILE *rfp, RCSNUM *rev
expmode = rcs_kwexp_get(rfp);
if (!(expmode & RCS_KWEXP_NONE)) {
- if ((rdp = rcs_findrev(rfp, rev)) == NULL)
- fatal("could not fetch revision");
+ if ((rdp = rcs_findrev(rfp, rev)) == NULL) {
+ rcsnum_tostr(rev, version, sizeof(version));
+ fatal("could not find desired version %s in %s",
+ version, rfp->rf_path);
+ }
+
expand = 1;
}
}
Re: opencvs - fix revision lookups for branches
On Wed, 22 Jun 2016 12:21:56 +0200, Joris Vink wrote:
> This diff below fixes a serious issue in opencvs when
> checking out revisions from a branch.
>
> Properly perform a revision lookup so update -r actually
> works again, as a bonus throw a more correct error when
> the revision could not be found.
>
> .joris
>
> Index: rcs.c
> ===
> RCS file: /cvs/src/usr.bin/cvs/rcs.c,v
> retrieving revision 1.313
> diff -u -p -r1.313 rcs.c
> --- rcs.c 5 Nov 2015 09:48:21 - 1.313
> +++ rcs.c 22 Jun 2016 09:52:04 -
> @@ -1796,17 +1796,13 @@ rcs_rev_getlines(RCSFILE *rfp, RCSNUM *f
>
> again:
> for (;;) {
> + if (rdp == NULL)
> + break;
Wouldn't this be easier to read as:
while (rdp != NULL) {
> +
> if (rdp->rd_next->rn_len != 0) {
> trdp = rcs_findrev(rfp, rdp->rd_next);
> if (trdp == NULL)
> fatal("failed to grab next revision");
> - } else {
> - /*
> - * XXX Fail, although the caller does not always do the
> - * right thing (eg cvs diff when the tree is ahead of
> - * the repository).
> - */
> - break;
> }
>
> if (rdp->rd_tlen == 0) {
opencvs - fix revision lookups for branches
Hi,
This diff below fixes a serious issue in opencvs when
checking out revisions from a branch.
Properly perform a revision lookup so update -r actually
works again, as a bonus throw a more correct error when
the revision could not be found.
.joris
Index: rcs.c
===
RCS file: /cvs/src/usr.bin/cvs/rcs.c,v
retrieving revision 1.313
diff -u -p -r1.313 rcs.c
--- rcs.c 5 Nov 2015 09:48:21 - 1.313
+++ rcs.c 22 Jun 2016 09:52:04 -
@@ -1796,17 +1796,13 @@ rcs_rev_getlines(RCSFILE *rfp, RCSNUM *f
again:
for (;;) {
+ if (rdp == NULL)
+ break;
+
if (rdp->rd_next->rn_len != 0) {
trdp = rcs_findrev(rfp, rdp->rd_next);
if (trdp == NULL)
fatal("failed to grab next revision");
- } else {
- /*
-* XXX Fail, although the caller does not always do the
-* right thing (eg cvs diff when the tree is ahead of
-* the repository).
-*/
- break;
}
if (rdp->rd_tlen == 0) {
@@ -1857,7 +1853,7 @@ again:
}
next:
- if (!rcsnum_differ(rdp->rd_num, frev))
+ if (rdp == NULL || !rcsnum_differ(rdp->rd_num, frev))
done = 1;
if (RCSNUM_ISBRANCHREV(frev) && done != 1) {
@@ -2045,6 +2041,7 @@ rcs_rev_getbuf(RCSFILE *rfp, RCSNUM *rev
struct rcs_delta *rdp;
struct rcs_lines *lines;
struct rcs_line *lp, *nlp;
+ char version[RCSNUM_MAXSTR];
BUF *bp;
rdp = NULL;
@@ -2057,8 +2054,12 @@ rcs_rev_getbuf(RCSFILE *rfp, RCSNUM *rev
expmode = rcs_kwexp_get(rfp);
if (!(expmode & RCS_KWEXP_NONE)) {
- if ((rdp = rcs_findrev(rfp, rev)) == NULL)
- fatal("could not fetch revision");
+ if ((rdp = rcs_findrev(rfp, rev)) == NULL) {
+ rcsnum_tostr(rev, version, sizeof(version));
+ fatal("could not find desired version %s in %s",
+ version, rfp->rf_path);
+ }
+
expand = 1;
}
}
