Re: Feature request/ideas

[Frank Hemer]

On Saturday 26 February 2005 21:06, Derek Price wrote:
> Frank Hemer wrote:
> | On Saturday 26 February 2005 00:27, Derek Price wrote:
> |> Frank Hemer wrote: |> I was just glancing at that patch and I
> |> think I can implement |> what Steve did much more succinctly, so
> |> I'm going to take a shot |> at it. The most important thing for
> |> your patch is probably to use |> the naming scheme: `.XXX'. | |
> |> So I'll wait for your commit then.
> |>
> |> Here's a question:  Should the commitids be cached in
> |> CVSROOT/val-tags in some form?  I think so.  What is CVSNT doing
> |> in this regard?
> |
> | Cvsnt seems not to cache the commitids. I don't think it is
> | reasonable to cache here because _every_ commit would be cached,
> | bloating val-tags but not gaining any performance.
>
> Actually, the trick with val-tags is that it is much cheaper to search
> the dbm file than it is to parse and search each and every RCS file
> when looking for a tag that may only exist in a single file.  Even
> with a lot of commits, it will remain cheaper to search the val-tags
> file, I would hazard.

Well, actually the commitid isn't a tag ...
However, I added it in tag.c::tag_check_valid() to play around ...

> Though it's not yet being done, the current recursive RCS file tag
> search could be made more efficient by only parsing the RCS header
> when validating tags.  This won't work with commitids, since they are
> stored with the revision data, adding more reason for adding them to
> val-tags.
>
> It's possible that if DBM search performance ever becomes an issue, we
> could move to some sort of sorted (and indexed? binary?) dbm type that
> is cheaper to search.

Good Idea, this would open ways to implement a real 'rename' and 'move' 
feature ...

I have pasted my current patch state, there is no docu yet, and no sanity.sh 
testing. However, the current sanity.sh is not affected by the patch, and 
still runs ok.

Features: 
1) .commitid.xxx will select the revision when used as a tag
2) appending '.prev' to a numeric revision or a symbolic tag will select the 
previous revision

Probably more testing needs to be done, but I'll wait for feedback ...
Unfortunately my mail-client refuses to switch of line wrapping, but I'll care 
about it another time:-(


Index: src/cvs.h
===================================================================
RCS file: /cvs/ccvs/src/cvs.h,v
retrieving revision 1.330
diff -u -r1.330 cvs.h
--- src/cvs.h   26 Feb 2005 00:27:35 -0000      1.330
+++ src/cvs.h   26 Feb 2005 22:11:07 -0000
@@ -242,6 +242,8 @@
  */
 #define        TAG_HEAD        "HEAD"
 #define        TAG_BASE        "BASE"
+#define TAG_COMMITID    ".commitid."
+#define TAG_PREVIOUS    "prev"
 
 /* Environment variable used by CVS */
 #define        CVSREAD_ENV     "CVSREAD"       /* make files read-only */
Index: src/rcs.c
===================================================================
RCS file: /cvs/ccvs/src/rcs.c,v
retrieving revision 1.330
diff -u -r1.330 rcs.c
--- src/rcs.c   25 Feb 2005 17:54:04 -0000      1.330
+++ src/rcs.c   26 Feb 2005 22:11:12 -0000
@@ -94,6 +94,8 @@
 static void free_rcsnode_contents (RCSNode *);
 static void free_rcsvers_contents (RCSVers *);
 static void rcsvers_delproc (Node * p);
+static char *RCS_findcommitid (RCSNode *, const char *, bool);
+static char *translate_tag (RCSNode *, const char *);
 static char *translate_symtag (RCSNode *, const char *);
 static char *RCS_addbranch (RCSNode *, const char *);
 static char *truncate_revnum_in_place (char *);
@@ -2236,7 +2238,7 @@
         return RCS_head (rcs);
 
     /* If valid tag let translate_symtag say yea or nay. */
-    rev = translate_symtag (rcs, tag);
+    rev = translate_tag (rcs, tag);
 
     if (rev)
         return rev;
@@ -2262,6 +2264,7 @@
             int *simple_tag)
 {
     char *tag;
+    char *version;
 
     if (simple_tag != NULL)
        *simple_tag = 0;
@@ -2282,12 +2285,13 @@
 #endif
            return RCS_head (rcs);
 
-    if (!isdigit ((unsigned char) symtag[0]))
+/*     if (!isdigit ((unsigned char) symtag[0])) */
+    if (!isrevnumonly (symtag))
     {
        char *version;
 
        /* If we got a symbolic tag, resolve it to a numeric */
-       version = translate_symtag (rcs, symtag);
+       version = translate_tag (rcs, symtag);
        if (version != NULL)
        {
            int dots;
@@ -2517,10 +2521,14 @@
     assert (rcs != NULL);
 
     /* numeric revisions are easy -- even number of dots is a branch */
-    if (isdigit ((unsigned char) *rev))
-       return (numdots (rev) & 1) == 0;
+/*     if (isdigit ((unsigned char) *rev)) */
+/*     return (numdots (rev) & 1) == 0; */
+
+/*     version = translate_symtag (rcs, rev); */
+    if (isrevnumonly (rev))
+        return (numdots (rev) & 1) == 0;
 
-    version = translate_symtag (rcs, rev);
+    version = translate_tag (rcs, rev);
     if (version == NULL)
        return 0;
     dots = numdots (version);
@@ -2571,7 +2579,7 @@
        return NULL;
 
     /* now, look for a match in the symbols list */
-    version = translate_symtag (rcs, rev);
+    version = translate_tag (rcs, rev);
     if (version == NULL)
        return NULL;
     dots = numdots (version);
@@ -2742,11 +2750,12 @@
     if (RCS_nodeisbranch (rcs, rev))
        return RCS_getbranch (rcs, rev, 1);
 
-    if (isdigit ((unsigned char) *rev))
+    if (isrevnumonly (rev))
+/*     if (isdigit ((unsigned char) *rev)) */
        num = xstrdup (rev);
     else
     {
-       num = translate_symtag (rcs, rev);
+       num = translate_tag (rcs, rev);
        if (num == NULL)
            return NULL;
     }
@@ -3178,15 +3187,134 @@
 
 
 /*
- * Return the version associated with a particular symbolic tag.
+ * Find the commitid, if prev is true return the
+ * previous revision 
+ */
+static char *
+RCS_findcommitid (RCSNode *rcs, const char *commitid, bool prev)
+{
+    RCSVers *vers, *oldvers;
+    Node *head, *p, *info;
+
+    head = rcs->versions->list;
+    oldvers = NULL;
+    for (p = head->next; p != head; p = p->next)
+    {
+        vers = (RCSVers *) p->data;
+
+       info = findnode (vers->other_delta, "commitid");
+       if (info != NULL)
+           if (!strcmp(info->data, commitid))
+           {
+               RCSVers *newvers = NULL;
+               if (!prev)
+                   return xstrdup(vers->version);
+               if (p->next)
+                   newvers = (RCSVers *)p->next->data;
+               else
+                   newvers = NULL;
+               if (numdots(vers->version)==1)
+                   return newvers?xstrdup(newvers->version):NULL;
+               else
+                   return oldvers?xstrdup(oldvers->version):NULL;
+           }
+       oldvers = vers;
+    }
+    return NULL;
+}
+
+
+
+/* Translate special/symbolic tags into their revision
+ * If tag is symbolic, resolv it. Then check for
+ * tag extensions. If an extension is found, resolv it.
  * Returns NULL or a newly malloc'd string.
  */
 static char *
-translate_symtag (RCSNode *rcs, const char *tag)
+translate_tag (RCSNode *rcs, const char *tag)
 {
+    char * retval = NULL;
     if (rcs->flags & PARTIAL)
        RCS_reparsercsfile (rcs, NULL, NULL);
+    
+    if (tag[0] == '@')
+    {
+        if ( tag[1] == '<')
+       {
+           return RCS_findcommitid(rcs, tag+2, true);
+       }
+       else
+       {
+           return RCS_findcommitid(rcs, tag+1, false);
+       }
+    }
+    else if ( !strncmp (tag, TAG_COMMITID, strlen (TAG_COMMITID)))
+    {
+       char * p;
+        bool prev = false;
+       char * id = xstrdup (tag + strlen (TAG_COMMITID));
+       p = strchr (id, '.');
+       if (p)
+       {
+           *p = '\0';
+           ++p;
+           if (!strcmp (p, TAG_PREVIOUS))
+               prev = true;
+           else
+               error (1, 0, "invalid tag extension: '%s'",p);
+       }
+       else
+       {
+           prev = false;
+       }
+       retval = RCS_findcommitid(rcs, id, prev);
+       free(id);
+    }
+    else
+    {
+        /* extract the revision part of x.x.x.x.sometagext.someothertagext */
+        char * c;
+        char * tmp = xstrdup (tag);
+        c = tmp + 1;
+       while (c = strchr(c,'.'))
+        {
+           if (isalpha ((unsigned char) c[1]))
+               break;
+           ++c;
+       }
+       if (c)
+       {
+           /* there is an extension */
+           *c = '\0';
+           ++c;
+           if (!isdigit( *tmp))
+               retval = translate_symtag (rcs, tmp);
+           else
+               retval = tmp;
+           if ( c && !strcmp(c, TAG_PREVIOUS))
+               retval = previous_rev( rcs, retval);
+           else
+               error (1, 0, "invalid tag extension: '%s'", c);
+       }
+       else
+       {
+           retval = translate_symtag (rcs, tmp);
+       }
+       free (tmp);
+    }
+    return retval;
+}
+
+
 
+
+/*
+ * Return the version associated with a particular symbolic tag.
+ * Returns NULL or a newly malloc'd string.
+ */
+static char *
+translate_symtag (RCSNode *rcs, const char *tag)
+{
     if (rcs->symbols != NULL)
     {
        Node *p;
@@ -3314,18 +3442,37 @@
      */
     if (isalpha ((unsigned char) *tag))
     {
-       for (cp = tag; *cp; cp++)
+        cp = tag;
+       while (*cp)
        {
            if (!isgraph ((unsigned char) *cp))
                error (1, 0, "tag `%s' has non-visible graphic characters",
                       tag);
+           if (*cp == '.')
+           {
+              if (!strncmp (cp+1, TAG_PREVIOUS, strlen (TAG_PREVIOUS)))
+              {
+                  cp += strlen (TAG_PREVIOUS) + 1;
+                  continue;
+              }
+           }
            if (strchr (invalid, *cp))
                error (1, 0, "tag `%s' must not contain the characters `%s'",
                       tag, invalid);
+           ++cp;
        }
     }
     else
+    {
+        if (*tag == '@')
+           /* keep compatible with cvsnt commitid tags */
+           return;
+       if (!strncmp(tag,TAG_COMMITID,strlen (TAG_COMMITID)))
+       {
+           return;
+       }
        error (1, 0, "tag `%s' must start with a letter", tag);
+    }
 }
 
 
@@ -3366,6 +3513,59 @@
 
 
 /*
+ * TRUE if argument has valid syntax for an RCS revision or 
+ * branch number including an extension of the form '.*'.
+ * All previous characters must be digits or dots, first and
+ * last characters thereof must be digits, and no two consecutive 
+ * characters may be dots.
+ *
+ * Intended for classifying things, so this function doesn't 
+ * call error.
+ */
+int 
+RCS_valid_rev_ext (const char *rev)
+{
+    const char * last = rev;
+    if (!isdigit ((unsigned char) *rev))
+        return 0;
+    while (*(++rev))
+    {
+        if (*rev == '.')
+       {
+            if (*last == '.')
+                return 0;
+           continue;
+       }
+       if (!isdigit ((unsigned char) *rev))
+       {
+           do
+           {
+               if (!strncmp (rev, TAG_PREVIOUS, strlen (TAG_PREVIOUS)))
+                   rev += strlen (TAG_PREVIOUS);
+/*             add further extensions here */
+/*             else if (!strncmp (rev,"trunk",5)) */
+/*                 rev += 5; */
+               else
+                   return 0;
+               if (*rev == '.')
+                   ++rev;
+               else if (*rev == 0)
+                   return 1;
+               else
+                   return 0;
+           } while (*rev);
+           return 1;
+       }
+       last = rev;
+    }
+    if (!isdigit ((unsigned char) *last))
+        return 0;
+    return 1;
+}
+
+
+
+/*
  * Return true if RCS revision with TAG is a dead revision.
  */
 int
Index: src/rcs.h
===================================================================
RCS file: /cvs/ccvs/src/rcs.h,v
retrieving revision 1.81
diff -u -r1.81 rcs.h
--- src/rcs.h   23 Feb 2005 01:24:28 -0000      1.81
+++ src/rcs.h   26 Feb 2005 22:11:12 -0000
@@ -216,6 +216,7 @@
 List *RCS_symbols (RCSNode *rcs);
 void RCS_check_tag (const char *tag);
 int RCS_valid_rev (const char *rev);
+int RCS_valid_rev_ext (const char *rev);
 List *RCS_getlocks (RCSNode *rcs);
 void freercsnode (RCSNode ** rnodep);
 char *RCS_getbranch (RCSNode *rcs, const char *tag, int force_tag_match);
Index: src/subr.c
===================================================================
RCS file: /cvs/ccvs/src/subr.c,v
retrieving revision 1.133
diff -u -r1.133 subr.c
--- src/subr.c  25 Feb 2005 04:35:23 -0000      1.133
+++ src/subr.c  26 Feb 2005 22:11:12 -0000
@@ -217,6 +217,32 @@
 
 
 
+/*
+ * Returns 1 if this is a plain revision number without extensions
+ * A plain rev num never has a dot followed by an alpha 
+ */
+int
+isrevnumonly (const char *s)
+{
+    if (!isdigit ((unsigned char) s[0]))
+    {
+        return 0;
+    }
+    else
+    {
+        const char * c = s+1;
+       while (c = strchr(c,'.'))
+        {
+           ++c;
+           if (*c && isalpha ((unsigned char) c[0]))
+               return 0;
+       }
+    }
+    return 1;
+}
+
+
+
 /* Compare revision numbers REV1 and REV2 by consecutive fields.
    Return negative, zero, or positive in the manner of strcmp.  The
    two revision numbers must have the same number of fields, or else
Index: src/subr.h
===================================================================
RCS file: /cvs/ccvs/src/subr.h,v
retrieving revision 1.2
diff -u -r1.2 subr.h
--- src/subr.h  25 Feb 2005 04:35:23 -0000      1.2
+++ src/subr.h  26 Feb 2005 22:11:12 -0000
@@ -26,6 +26,7 @@
 void free_names (int *pargc, char *argv[]);
 void line2argv (int *pargc, char ***argv, char *line, char *sepchars);
 int numdots (const char *s);
+int isrevnumonly (const char *s);
 int compare_revnums (const char *, const char *);
 char *increment_revnum (const char *);
 char *getcaller (void);
Index: src/tag.c
===================================================================
RCS file: /cvs/ccvs/src/tag.c,v
retrieving revision 1.134
diff -u -r1.134 tag.c
--- src/tag.c   26 Feb 2005 00:27:35 -0000      1.134
+++ src/tag.c   26 Feb 2005 22:11:13 -0000
@@ -1442,11 +1442,12 @@
  *   Nothing.
  */
 void
-tag_check_valid (const char *name, int argc, char **argv, int local, int 
aflag,
+tag_check_valid (const char *oriname, int argc, char **argv, int local, int 
aflag,
                  char *repository, bool valid)
 {
     DBM *db;
     char *valtags_filename;
+    char *name;
     int nowrite = 0;
     datum mytag, val;
     struct val_args the_val_args;
@@ -1457,32 +1458,32 @@
     TRACE (TRACE_FUNCTION,
           "tag_check_valid (name=%s, argc=%d, argv=%p, local=%d,\n"
       "                      aflag=%d, repository=%s, valid=%s)",
-          name ? name : "(name)", argc, (void *)argv, local, aflag,
+          oriname ? oriname : "(name)", argc, (void *)argv, local, aflag,
           repository ? repository : "(null)",
           valid ? "true" : "false");
 #else
     TRACE (TRACE_FUNCTION,
           "tag_check_valid (name=%s, argc=%d, argv=%lx, local=%d,\n"
       "                      aflag=%d, repository=%s, valid=%s)",
-          name ? name : "(name)", argc, (unsigned long)argv, local, aflag,
+          oriname ? oriname : "(name)", argc, (unsigned long)argv, local, 
aflag,
           repository ? repository : "(null)",
           valid ? "true" : "false");
 #endif
 
     /* Numeric tags require only a syntactic check.  */
-    if (isdigit ((unsigned char) name[0]))
+    if (isdigit ((unsigned char) oriname[0]))
     {
        /* insert is not possible for numeric revisions */
        assert (!valid);
-       if (RCS_valid_rev (name)) return;
+       if (RCS_valid_rev_ext (oriname)) return;
        else
            error (1, 0, "\
-Numeric tag %s invalid.  Numeric tags should be of the form X[.X]...", name);
+Numeric tag %s invalid.  Numeric tags should be of the form X[.X]...", 
oriname);
     }
 
     /* Special tags are always valid.  */
-    if (strcmp (name, TAG_BASE) == 0
-       || strcmp (name, TAG_HEAD) == 0)
+    if (strcmp (oriname, TAG_BASE) == 0
+       || strcmp (oriname, TAG_HEAD) == 0)
     {
        /* insert is not possible for numeric revisions */
        assert (!valid);
@@ -1492,7 +1493,18 @@
     /* Verify that the tag is valid syntactically.  Some later code once made
      * assumptions about this.
      */
-    RCS_check_tag (name);
+    RCS_check_tag (oriname);
+
+    /* Remove tag extension from commitid 
+     * for not bloating val_tags
+     */
+    name = xstrdup (oriname);
+    if (!strncmp (name, TAG_COMMITID, strlen (TAG_COMMITID)))
+    {
+        char * p = name + strlen (TAG_COMMITID);
+       if (p = strchr (p,'.'))
+           *p = '\0';
+    }
 
     /* FIXME: This routine doesn't seem to do any locking whatsoever
        (and it is called from places which don't have locks in place).
@@ -1530,6 +1542,7 @@
             */
            dbm_close (db);
            free (valtags_filename);
+           free (name);
            return;
        }
        /* FIXME: should check errors somehow (add dbm_error to myndbm.c?).  */
@@ -1579,6 +1592,7 @@
        if (db != NULL)
            dbm_close (db);
        free (valtags_filename);
+       free (name);
        return;
     }
 
@@ -1593,6 +1607,7 @@
        {
            error (0, errno, "warning: cannot create %s", valtags_filename);
            free (valtags_filename);
+           free (name);
            return;
        }
     }
@@ -1604,4 +1619,5 @@
     dbm_close (db);
     free (mytag.dptr);
     free (valtags_filename);
+    free (name);
 }



Regards,
Frank
-- 
- The LinCVS Team -
http:/www.lincvs.com