[A fix for this bug was checked in 1998-03-18. This page contains 3 messages: details on the bug, a preliminary patch, and an update. -kingdon] Date: Mon, 16 Feb 1998 23:18:40 -0800 From: Dan Wilder To: CVS Mailing List Subject: Change in cvs admin -n semantics I'm writing to find out if anybody else is as dismayed as I am. I just tried cvs-1.9.24, and quickly uninstalled it when I located a significant change in the semantics of the cvs admin -nNEW:OLD command, as compared with cvs-1.9. The RCS 5.7 man page says: OPTIONS [ ... ] -nname[:[rev]] Associate the symbolic name name with the branch or revision rev. Delete the symbolic name if both : and rev are omitted; otherwise, print an error message if name is already associated with another number. If rev is symbolic, it is expanded before association. A rev consisting of a branch number followed by a . stands for the current latest revision in the branch. A : with an empty rev stands for the current latest revision on the default branch, nor- mally the trunk. For example, rcs -nname: RCS/* associates name with the current latest revision of all the named RCS files; this contrasts with rcs -nname:$ RCS/* which associates name with the revision numbers extracted from keyword strings in the corresponding working files. This pretty clearly allows "rev" to be symbolic, and it doesn't say anything about transforming a symbolic "rev" to the latest revision on a branch, should "rev" happen to be a symbolic branch tag. Indeed, with cvs-1.9, cvs admin -nNEW_TAG:OLD_TAG makes NEW_TAG a synonym in all respects for OLD_TAG, regardless of the nature of OLD_TAG. The new behavior under cvs-1.9.24 changed, giving entirely different results depending on whether OLD_TAG was a branch tag or not. If OLD_TAG is not a branch tag, the result under cvs-1.9.24 and cvs-1.9 are the same; NEW_TAG is a synonym for OLD_TAG. Under cvs 1.9.24, NEW_TAG is never a branch tag, regardless of the nature of OLD_TAG. Thus, there is no longer any means for producing a synonym for OLD_TAG when it is a branch tag. I happen to have a bunch of scripting that solves some fairly difficult versioning problems easily and efficiently. These scripts depend on the ability to clone, sometimes only temporarily, a branch tag. Which now appears to be entirely absent from cvs. It is true there are other solutions to the sort of problems I face. Unfortunately I'm unlikely to be able to achieve these sorts of solutions in my present context, due to organizational constraints. Does this drastic change in 'cvs admin -n' strike anybody else here as distressing? If so, is there any chance a patch restoring the old semantics might be accepted? -- Dan Wilder Date: Thu, 19 Feb 1998 07:32:46 -0800 From: Dan Wilder To: kingdon@cyclic.com Subject: Re: Apparent regression in cvs admin -n Content-Type: multipart/mixed; boundary=3MwIy2ne0vdjdPXF --3MwIy2ne0vdjdPXF Content-Type: text/plain; charset=us-ascii Jim, I've taken a look at the "Apparent regression in cvs admin -n" problem, and constructed a preliminary fix, which I prefer to beat on for a few days before officially sending it, ChangeLog entry and all. The crux of the matter, unfortunately, appears to lie in the routine RCS_gettag(), which looks to be the approved means of translating a tag to an RCS number. Unfortunately RCS_gettag() returns only revision numbers, not branch numbers. If you pass it a branch tag, it will return the head revision of that branch. The right way to fix it would seem to be making RCS_gettag return the corresponding RCS number when passed a tag, rather than making assumptions about what revision different from the tag is _really_ wanted, and then to provide another routine to extract the head revision (or the branchpoint revision) given a branch tag. This, however, would ripple all over the place. A more conservative approach would be to add an argument to RCS_gettag which would preserve the present behavior if TRUE, and return just the requested information if FALSE. Again, this entails changes in quite a few calls, and I don't know how much leeway the CVS folk have in changing rcs.c, which is presumably supported by the RCS folk, and called from places that are not in CVS. The most conservative approach, and the one I've been pursueing, is to limit the changes in rcs.c, and fix the problem so far as possible, in admin.c. I've put one small change into rcs.c/rcs.h: there is a static routine there called translate_symtag() which does pretty much what I want, though without the error checking of RCS_gettag. I've deleted the "static" declaration to make it accessible from admin.c. In case you care to comment, the _unofficial_ copy of my patches to date are attached. -- Dan Wilder --3MwIy2ne0vdjdPXF Content-Type: text/plain; charset=us-ascii Content-Disposition: attachment; filename=dans --- src/old/rcs.c Wed Feb 18 22:39:51 1998 +++ src/rcs.c Thu Feb 19 00:20:43 1998 @@ -31,7 +31,6 @@ static void free_rcsnode_contents PROTO((RCSNode *)); static void free_rcsvers_contents PROTO((RCSVers *)); static void rcsvers_delproc PROTO((Node * p)); -static char *translate_symtag PROTO((RCSNode *, const char *)); static char *printable_date PROTO((const char *)); static char *escape_keyword_value PROTO ((const char *, int *)); static void expand_keywords PROTO((RCSNode *, RCSVers *, const char *, @@ -2144,7 +2143,7 @@ * Return the version associated with a particular symbolic tag. * Returns NULL or a newly malloc'd string. */ -static char * +char * translate_symtag (rcs, tag) RCSNode *rcs; const char *tag; --- src/old/rcs.h Wed Feb 18 22:39:46 1998 +++ src/rcs.h Wed Feb 18 22:41:23 1998 @@ -183,6 +183,7 @@ int *simple_tag)); char *RCS_getversion PROTO((RCSNode * rcs, char *tag, char *date, int force_tag_match, int *simple_tag)); +char *translate_symtag PROTO((RCSNode *, const char *)); char *RCS_magicrev PROTO((RCSNode *rcs, char *rev)); int RCS_isbranch PROTO((RCSNode *rcs, const char *rev)); int RCS_nodeisbranch PROTO((RCSNode *rcs, const char *tag)); --- src/old/admin.c Wed Feb 18 22:32:49 1998 +++ src/admin.c Thu Feb 19 01:00:23 1998 @@ -676,6 +676,17 @@ tag = xstrdup (arg + 2); *p++ = ':'; + /* 'tag' should be a symtag, not a number */ + if (isdigit (tag[0])) + { + error (0, 0, + "%s: invalid symbol '%s'", + rcs->path, + tag); + status = 1; + free (tag); + continue; + } /* Option `n' signals an error if this tag is already bound. */ if (arg[1] == 'n') { @@ -687,15 +698,53 @@ rcs->path, tag, n->data); status = 1; + free (tag); continue; } } - /* Expand rev if necessary. */ - rev = RCS_gettag (rcs, p, 0, NULL); - RCS_settag (rcs, tag, rev); - if (rev != NULL) - free (rev); + /* Expand branch or revision if necessary. + * FIXME: Won't handle magic branches, + * for example 'cvs admin -nTAG:1.9.0.4' + * correctly. + */ + if (isdigit (p[0])) + { + if (findnode (rcs->versions, p)) + { + RCS_settag (rcs, tag, p); + } + else if (rev = RCS_getbranch (rcs, p, 1)) + { + RCS_settag (rcs, tag, p); + free (rev); + } + else + { + error (0, 0, + "%s: unknown revision %s", + rcs->path, + p); + status = 1; + } + } + else + /* Is a tag not a branch or revision. */ + { + if (rev = translate_symtag (rcs, p)) + { + RCS_settag (rcs, tag, rev); + free (rev); + } + else + { + error (0, 0, + "%s: unknown tag %s", + rcs->path, + p); + status = 1; + } + } free (tag); break; case 's': --3MwIy2ne0vdjdPXF-- Date: Thu, 26 Feb 1998 12:05:46 -0800 From: Dan Wilder To: Jim Kingdon Subject: cvs admin -n problem: status report Jim, Progressing merrily on fixing the cvs admin -n problem. I have code that works quite well, handles the wierd cases and all. Fairly minimal changes in admin.c, no changes to existing code in rcs.c, a few (4) small functions added to rcs.c with prototypes in rcs.h. Test suite for cvs admin -n in sanity.sh is about half done. My approach in the preliminary fix didn't pan out too well, the special cases around branch tags kept bulking up. Too, I've found an analogous problem in cvs update -rsome_branch_tag. Which I don't propose to fix immediately, I've gotta spend some time cleaning up the basement, to appease the spouse. It appears that there may be other problems related to the fact that the existing function that converts a tag to a branch number always returns the tip revision of the branch, not its branch number. Which is why I'd prefer add some small, well-commented, well-defined functions to rcs.c, rather than putting the code inline in admin.c. A style question: I'm seeing two indentation styles, or else one style whose rules I don't quite understand. if (condition) { a bunch of code is one style } if (condition) { some other code is another style } and the related if (condition) single statment; and if (condition) single statement; Are both of these right? Is there a rule for applying one or the other? Or would you prefer that I stick to one and not use the other? Thanks! -- Dan Wilder