Re: [PATCH] - svndumpfilter - add wildcard matchinv

Thank you for the feedback. Here is a new version using apr_fnmatch. Based on my tests, it seems to handle escapes ('\') ok and has '[' matching on top of the previous one. I did not see a case which would require me to handle escapes aditionally from apr_fnmatch .. did you
have any specific functionality in mind that requires this?

Andrei

On Mon, 2006-10-02 at 09:00 -0400, C. Michael Pilato wrote:
> Andrei wrote:
> > This patch adds wildcard matching to the regular "prefix" matching for
> > svndumpfilter. It is my first patch and feedback would be appreciated.
> >
> > There is one place where I am unsure about: the wildcard matching uses
> > "NXOR", so I am not sure if it was designed to only work with path
> > prefixes (it seems to go well if I choose "*.txt" as a pattern for
> > example).
>
> Andrei, thanks for the generally good patch.  I have a few comments
> which you can find inline below.
>
> > Index: subversion/svndumpfilter/main.c
> > ===================================================================
> > --- subversion/svndumpfilter/main.c (revision 21719)
> > +++ subversion/svndumpfilter/main.c (working copy)
> > @@ -97,7 +97,118 @@
> >    svn_stringbuf_appendbytes(*strbuf, "\n", 1);
> >  }
> > 
> > +/* Match a string against a wildcard pattern.
> > + * @param str - [in] the string to check
> > + * @param pat - [in] the pattern. '*' and '?' wildcards are accepted,
> > + *              with '\\' as an escape character
> > + * @return TRUE if pattern match is detected
> > + */
> > +static svn_boolean_t
> > +wildcard_match(const char *str, const char *pat)
> > +{
>
> ...
>
> Could not this whole function be written using apr_fnmatch(), the same
> matching interface we use elsewhere in Subversion?  You could do a
> quicky pass to handle the backslash escape character (a UI quirk I'm not
> totally sold on), and then hand the result off to apr_fnmatch().  Sure
> would simplify the code, and keep this behavior consistent with that of
> other areas of Subversion.
>
> > @@ -864,6 +991,23 @@
> >    baton->renumber_history = apr_hash_make(pool);
> >    baton->last_live_revision = SVN_INVALID_REVNUM;
> > 
> > +  if (opt_state->match_type == NULL) {  /* just use the default */
> > +    baton->match_function = ary_prefix_match;
> > +  } else if (strcmp(opt_state->match_type, MATCH_TYPE_PREFIX) == 0) {
> > +    baton->match_function = ary_prefix_match;
> > +  } else if (strcmp(opt_state->match_type, MATCH_TYPE_WILDCARD) == 0) {
> > +    svn_error_t *pComplexityCheck;
> > +   
> > +    pComplexityCheck = check_wildcard_complexity(baton->prefixes);
> > +    if (pComplexityCheck)
> > +      return pComplexityCheck;
>
> We don't do camel-case.  But if you use apr_fnmatch(), you needn't check
> complexity anyway.
>
Index: subversion/svndumpfilter/main.c
===================================================================
--- subversion/svndumpfilter/main.c (revision 21736)
+++ subversion/svndumpfilter/main.c (working copy)
@@ -20,6 +20,7 @@
 #include <stdlib.h>
 
 #include <apr_file_io.h>
+#include <apr_fnmatch.h>
 
 #include "svn_private_config.h"
 #include "svn_cmdline.h"
@@ -97,7 +98,26 @@
   svn_stringbuf_appendbytes(*strbuf, "\n", 1);
 }
 
+/* Wildcard matching function to compare a node-path with a
+ * specified set of wildcard patterns
+ */
+static svn_boolean_t
+ary_wildcard_match(apr_array_header_t *pfxlist, const char *path)
+{
+  int i;
+  const char *pattern;
 
+  for (i = 0; i < pfxlist->nelts; i++)
+    {
+      pattern = APR_ARRAY_IDX(pfxlist, i, const char *);
+
+      if (apr_fnmatch(pattern, path, 0) == APR_SUCCESS)
+        return TRUE;
+    }
+
+  return FALSE;
+}
+
 /* Prefix matching function to compare node-path with set of prefixes. */
 static svn_boolean_t
 ary_prefix_match(apr_array_header_t *pfxlist, const char *path)
@@ -143,6 +163,8 @@
   svn_boolean_t was_dropped; /* Was this revision dropped? */
 };
 
+typedef svn_boolean_t (*match_function_t)(apr_array_header_t*, const char *);
+
 struct parse_baton_t
 {
   /* Command-line options values. */
@@ -152,6 +174,7 @@
   svn_boolean_t do_renumber_revs;
   svn_boolean_t preserve_revprops;
   apr_array_header_t *prefixes;
+  match_function_t match_function;
 
   /* Input and output streams. */
   svn_stream_t *in_stream;
@@ -460,7 +483,7 @@
     copyfrom_path = svn_path_join("/", copyfrom_path, pool);
 
   /* Shame, shame, shame ... this is NXOR. */
-  nb->do_skip = (ary_prefix_match(pb->prefixes, node_path)
+  nb->do_skip = (pb->match_function(pb->prefixes, node_path)
                  ? pb->do_exclude : (! pb->do_exclude));
 
   /* If we're skipping the node, take note of path, discarding the
@@ -480,7 +503,7 @@
 
       /* Test if this node was copied from dropped source. */
       if (copyfrom_path &&
-          (ary_prefix_match(pb->prefixes, copyfrom_path)
+          (pb->match_function(pb->prefixes, copyfrom_path)
            ? pb->do_exclude : (! pb->do_exclude)))
         {
           /* This node was copied from dropped source.
@@ -767,7 +790,8 @@
     svndumpfilter__renumber_revs,
     svndumpfilter__preserve_revprops,
     svndumpfilter__quiet,
-    svndumpfilter__version
+    svndumpfilter__version,
+    svndumpfilter__match_type
   };
 
 /* Option codes and descriptions.
@@ -792,9 +816,18 @@
      N_("Renumber revisions left after filtering.") },
     {"preserve-revprops",  svndumpfilter__preserve_revprops, 0,
      N_("Don't filter revision properties.") },
+    {"match-type", svndumpfilter__match_type, 1,
+     N_("Default: 'prefix'. Defines how the pattern matching is done\n"
+        "                            "
+        "'prefix'   - match nodes with the fiven prefixes from dumpstream.\n"
+        "                            "
+        "'wildcard' - use '*' and '?' as wildcards (e.g. '*.pdf') .\n"
+       ) },
     {NULL}
   };
 
+#define MATCH_TYPE_PREFIX      "prefix"
+#define MATCH_TYPE_WILDCARD    "wildcard"
 
 /* Array of available subcommands.
  * The entire list must be terminated with an entry of nulls.
@@ -802,16 +835,18 @@
 static const svn_opt_subcommand_desc_t cmd_table[] =
   {
     {"exclude", subcommand_exclude, {0},
-     N_("Filter out nodes with given prefixes from dumpstream.\n"
-        "usage: svndumpfilter exclude PATH_PREFIX...\n"),
+     N_("Filter out nodes matching a given pattern from the dumpstream.\n"
+        "usage: svndumpfilter exclude PATTERN...\n"),
      {svndumpfilter__drop_empty_revs, svndumpfilter__renumber_revs,
-      svndumpfilter__preserve_revprops, svndumpfilter__quiet} },
+      svndumpfilter__preserve_revprops, svndumpfilter__quiet,
+      svndumpfilter__match_type} },
 
     {"include", subcommand_include, {0},
-     N_("Filter out nodes without given prefixes from dumpstream.\n"
-        "usage: svndumpfilter include PATH_PREFIX...\n"),
+     N_("Filter out nodes matching a given pattern from the dumpstream.\n"
+        "usage: svndumpfilter include PATTERN...\n"),
      {svndumpfilter__drop_empty_revs, svndumpfilter__renumber_revs,
-      svndumpfilter__preserve_revprops, svndumpfilter__quiet} },
+      svndumpfilter__preserve_revprops, svndumpfilter__quiet,
+      svndumpfilter__match_type} },
 
     {"help", subcommand_help, {"?", "h"},
      N_("Describe the usage of this program or its subcommands.\n"
@@ -833,6 +868,7 @@
   svn_boolean_t help;                    /* --help or -?        */
   svn_boolean_t renumber_revs;           /* --renumber-revs     */
   svn_boolean_t preserve_revprops;       /* --preserve-revprops */
+  const char *match_type;                /* --match-type        */
   apr_array_header_t *prefixes;          /* mainargs.           */
 };
 
@@ -864,6 +900,17 @@
   baton->renumber_history = apr_hash_make(pool);
   baton->last_live_revision = SVN_INVALID_REVNUM;
 
+  if (opt_state->match_type == NULL) {  /* just use the default */
+    baton->match_function = ary_prefix_match;
+  } else if (strcmp(opt_state->match_type, MATCH_TYPE_PREFIX) == 0) {
+    baton->match_function = ary_prefix_match;
+  } else if (strcmp(opt_state->match_type, MATCH_TYPE_WILDCARD) == 0) {
+    baton->match_function = ary_wildcard_match;
+  } else {
+    return svn_error_create(SVN_ERR_CL_ARG_PARSING_ERROR, NULL,
+                            _("Invalid pattern matching type selected"));
+  }
+
   /* This is non-ideal: We should pass through the version of the
    * input dumpstream.  However, our API currently doesn't allow that.
    * Hardcoding version 2 is acceptable because:
@@ -1084,7 +1131,6 @@
   apr_array_header_t *received_opts;
   int i;
 
-
   /* Initialize the app. */
   if (svn_cmdline_init("svndumpfilter", stderr) != EXIT_SUCCESS)
     return EXIT_FAILURE;
@@ -1168,6 +1214,9 @@
         case svndumpfilter__preserve_revprops:
           opt_state.preserve_revprops = TRUE;
           break;
+        case svndumpfilter__match_type:
+          opt_state.match_type = apr_pstrdup(pool, opt_arg);
+          break;
         default:
           {
             subcommand_help(NULL, NULL, pool);


---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@...
For additional commands, e-mail: dev-help@...