Martin Furter wrote:
>Hello
>
>Here is a redesigned auto-props patch.
>
>It now applies all configured properties matching the file name as
>proposed on the mailing list.
>And it includes a test script now.
>
>I hope the patch is ready to apply now.
>
>
Well, I _do_ have several comments. The general approach looks O.K. to
me, though.
>Thanks
>Martin
>
>----- log message -----
>Automatic properties for 'svn add' and 'svn import' added.
>
>Automatic properties are defined in the file 'config' in the section
>[auto-props] and have the following format:
>
>expression = [propname[=value][;name[=val]]...]
>
>expression is used to match filenames with apr_fnmatch(). All matching
>property definitions are added to the file.
>
>Automatic properties can be enabled in the config in setion [miscellany].
>The entries are 'enable-auto-props-add' and 'enable-auto-props-imp'.
>Additionally they can be enabled/disabled on the commandline using
>--auto-props or --no-auto-props.
>
>
This description of the config file syntax would do much more good in
the README template in svn_config_ensure than here.
>* subversion/include/svn_config.h
>
> (SVN_CONFIG_OPTION_ENABLE_AUTO_PROPS_*, SVN_CONFIG_SECTION_AUTO_PROPS)
> New defines.
>
>* subversion/libsvn_subr/config_file.c
>
> (svn_config_ensure) Config entries added.
>
>* subversion/libsvn_client/client.h
>
> (svn_client__add_auto_props) Prototype added.
>
>* subversion/libsvn_client/commit.c
>
> (import_file) Call to svn_client__add_auto_props added, detection of
> mimetype and executable modified.
>
>* subversion/libsvn_client/add.c
>
> (auto_props_baton_t) New struct, local usage only.
> (auto_props_enumerator) New function, used by svn_client__add_auto_props.
> (svn_client__add_auto_props) New function, adds automatic properties.
> (add_dir_recursive) Call to svn_client__add_auto_props added.
> (svn_client__add) Call to svn_client__add_auto_props added.
>
>* subversion/clients/cmdline/cl.h
>
> (svn_cl__longopt_t) Added svn_cl__enable_autoprops_opt and
> svn_cl__disable_autoprops_opt.
> (svn_cl__opt_state_t) Added enable_autoprops and disable_autoprops.
>
>* subversion/clients/cmdline/main.c
>
> (svn_cl__options, svn_cl__cmd_table, main) Added
> svn_cl__enable_autoprops_opt and svn_cl__disable_autoprops_opt,
> set them in the config in main().
>
>* subversion/tests/clients/cmdline/autoprop_tests.py
>
> New file containing tests for auto-props.
>
>----- end of log message -----
>
>
>----- auto-props patch -----
>Index: subversion/include/svn_config.h
>===================================================================
>--- subversion/include/svn_config.h (revision 7208)
>+++ subversion/include/svn_config.h (working copy)
>@@ -81,7 +81,10 @@
> #define SVN_CONFIG_OPTION_LOG_ENCODING "log-encoding"
> #define SVN_CONFIG_OPTION_USE_COMMIT_TIMES "use-commit-times"
> #define SVN_CONFIG_OPTION_TEMPLATE_ROOT "template-root"
>+#define SVN_CONFIG_OPTION_ENABLE_AUTO_PROPS_ADD "enable-auto-props-add"
>+#define SVN_CONFIG_OPTION_ENABLE_AUTO_PROPS_IMP "enable-auto-props-imp"
> #define SVN_CONFIG_SECTION_TUNNELS "tunnels"
>+#define SVN_CONFIG_SECTION_AUTO_PROPS "auto-props"
>
>
> /*** Configuration Default Values ***/
>Index: subversion/libsvn_subr/config_file.c
>===================================================================
>--- subversion/libsvn_subr/config_file.c (revision 7208)
>+++ subversion/libsvn_subr/config_file.c (working copy)
>@@ -978,7 +978,26 @@
> "### Set use-commit-times to make checkout/update/switch/revert\n"
> "### put last-committed timestamps on every file touched.\n"
> "# use-commit-times = yes\n"
>+ "### Enable automatic properties for 'svn add' (true | false).\n"
>+ "### Automatic properties are defined in the section 'auto-props'.\n"
>+ "# enable-auto-props-add = true\n"
>+ "### Enable automatic properties for 'svn import' (true | false).\n"
>+ "# enable-auto-props-imp = true\n"
> "\n"
>
>
Hm. Why aren't we using APR_EOL_STR in these templates, like in the
README template?
>+ "### Section for configuring automatic properties.\n"
>+ "# [auto-props]\n"
>+ "# *.c = svn:eol-style=native\n"
>+ "# *.cpp = svn:eol-style=native\n"
>+ "# *.h = svn:eol-style=native\n"
>+ "# *.dsp = svn:eol-style=CRLF\n"
>+ "# *.dsw = svn:eol-style=CRLF\n"
>+ "# *.sh = svn:eol-style=native;svn:executable\n"
>+ "# *.txt = svn:eol-style=native\n"
>+ "# *.png = svn:mimetype=image/png\n"
>+ "# *.jpg = svn:mimetype=image/jpeg\n"
>+ "# Makefile = svn:eol-style=native\n"
>+ "\n"
>+ "\n"
> "### See http://subversion.tigris.org/issues/show_bug.cgi?id=668\n"
> "### for what else will soon be customized in this file.\n";
>
>Index: subversion/libsvn_client/client.h
>===================================================================
>--- subversion/libsvn_client/client.h (revision 7208)
>+++ subversion/libsvn_client/client.h (working copy)
>@@ -237,6 +237,38 @@
>
> /*** Add/delete ***/
>
>+/* Method which automatically adds configured properties.
>
>
Whups. It's not a method, it's a function.
>+
>+ path, ctx and pool must always be valid. When adding adm_access must
>+ be valid and editor must be NULL, when importing editor, file_baton,
>+ mimetype and executable must be valid.
>+
>+ First the function checks that auto-props is enabled for add or import.
>+ Then i enumerates the 'auto-props' section and tries to find fnmatches.
>+ All matches found are parsed and the properties added to given path.
>+
>+ The format of the value is [propname[=value][;name[=val]]...]
>+
>+ @param path a filename
>+ @param ctx the client context
>+ @param pool apr memory pool
>+ @param adm_access access baton, used when adding
>+ @param editor editor, used when importing, must be NULL when adding
>+ @param file_baton file baton for editor, used when importing
>+ @param mimetype pointer for returning mimetype, used when importing
>+ @param executable pointer for returning executable, used when importing
>+ @return an svn_error_t or SVN_NO_ERROR when everything went OK
>+*/
>
>
Look at how the other docstrings are written in this file. In private
headers, we use UPPERCASE for variable names, we don't use doxygen
markup, and we describe parameters in the text of the docstring, separately.
Also, I'm a bit concerned about the overloading of the 'editor' param to
differentiate between "svn add" and "svn import". It's dangerous
practice, and confusing. Mush better to use a separate svn_boolean_t
parameter instead, this way there's less danger of an inadvertent change
in behaviour if the semantics of the parameter change. There are far too
many rules here about which parameters must be set in different situations.
About param order: we usually put output parameters first, and pools last.
>+svn_error_t *
>+svn_client__add_auto_props (const char *path,
>+ svn_client_ctx_t *ctx,
>+ apr_pool_t *pool,
>+ svn_wc_adm_access_t *adm_access,
>+ const svn_delta_editor_t *editor,
>+ void *file_baton,
>+ const char **mimetype,
>+ int *executable);
>+
> /* The main logic for client deletion from a working copy. Deletes PATH
> from ADM_ACCESS. If PATH (or any item below a directory PATH) is
> modified the delete will fail and return an error unless FORCE is TRUE.
>Index: subversion/libsvn_client/commit.c
>===================================================================
>--- subversion/libsvn_client/commit.c (revision 7208)
>+++ subversion/libsvn_client/commit.c (working copy)
>@@ -105,8 +105,8 @@
> apr_pool_t *pool)
> {
> void *file_baton;
>- const char *mimetype;
>- svn_boolean_t executable;
>+ const char *mimetype = NULL;
>+ svn_boolean_t executable = FALSE;
>
>
Why add the initialization here? It would be much better if
svn_client__add_auto_props promised to set these to a consistent value.
> unsigned char digest[MD5_DIGESTSIZE];
> const char *text_checksum;
>
>@@ -114,21 +114,27 @@
> SVN_ERR (editor->add_file (edit_path, dir_baton, NULL, SVN_INVALID_REVNUM,
> pool, &file_baton));
>
>+ /* add automatic properties */
>+ SVN_ERR (svn_client__add_auto_props (path, ctx, pool, NULL, editor,
>+ file_baton, &mimetype, &executable));
>+
> /* If the file has a discernable mimetype, add that as a property to
> the file. */
>- SVN_ERR (svn_io_detect_mimetype (&mimetype, path, pool));
>+ if (!mimetype)
>+ SVN_ERR (svn_io_detect_mimetype (&mimetype, path, pool));
> if (mimetype)
> SVN_ERR (editor->change_file_prop (file_baton, SVN_PROP_MIME_TYPE,
> svn_string_create (mimetype, pool),
> pool));
>
> /* If the file is executable, add that as a property to the file. */
>- SVN_ERR (svn_io_is_file_executable (&executable, path, pool));
>+ if (!executable)
>+ SVN_ERR (svn_io_is_file_executable (&executable, path, pool));
> if (executable)
> SVN_ERR (editor->change_file_prop (file_baton, SVN_PROP_EXECUTABLE,
> svn_string_create ("", pool),
> pool));
>-
>+
> if (ctx->notify_func)
> (*ctx->notify_func) (ctx->notify_baton,
> path,
>Index: subversion/libsvn_client/add.c
>===================================================================
>--- subversion/libsvn_client/add.c (revision 7208)
>+++ subversion/libsvn_client/add.c (working copy)
>@@ -23,6 +23,7 @@
> /*** Includes. ***/
>
> #include <string.h>
>+#include <apr_fnmatch.h>
> #include "svn_wc.h"
> #include "svn_client.h"
> #include "svn_string.h"
>@@ -30,12 +31,151 @@
> #include "svn_error.h"
> #include "svn_path.h"
> #include "svn_io.h"
>+#include "svn_config.h"
> #include "client.h"
>
>
>
> /*** Code. ***/
>
>+typedef struct {
>+ char *filename;
>+ apr_pool_t *pool;
>+ apr_hash_t *properties;
>+} auto_props_baton_t;
>+
>+/**
>+ * enumerate through auto-props entries and store value if name matches
>+ *
>+ * @param name a property name
>+ * @param value value of the property
>+ * @param baton a baton of type auto_props_baton_t
>+ * @return FALSE to stop the enumeration, TRUE to continue
>+ */
>
>
Again, the existing practice is to describe parameters (and return
values) inline in the text. Parameter names are tagged with @a. Function
descriptions are sentences, so they start with a capital letter and end
with a period.
>+static svn_boolean_t
>+auto_props_enumerator (const char *name,
>+ const char *value,
>+ void *baton)
>+{
>+ auto_props_baton_t *autoprops = (auto_props_baton_t*)baton;
>+ char *property;
>+ char *last_token;
>+ int len;
>+
>+ /* we need a baton, a name and a value */
>+ if (!baton)
>+ return FALSE;
>+ if (!name || strlen (name) == 0)
>+ return TRUE;
>+ if (!value || strlen (value) == 0)
>+ return TRUE;
>
>
We don't check parameter consistency like this; the applicaiton is
responsible for sending correct parameters to library functions. It's
also a lot easier to find bugs if a program crashes, than trying to
trace why an iteration doesn't finish when it should.
Besides, neither 'name' nor 'value' can be NULL here, and 'name' can't
be empty.
>+ /* check if filename matches and return if it doesn't */
>+ if (apr_fnmatch (name, autoprops->filename, 0) == APR_FNM_NOMATCH)
>+ return TRUE;
>+
>+ /* parse the value */
>+ property = apr_pstrdup (autoprops->pool, value);
>
>
Ouch. You're allocating temporary values from a long-lasting pool. It
might actually be acceptable to use a temporary pool when creating the
baton[*].
>+ property = apr_strtok (property, ";", &last_token);
>+ while (property)
>+ {
>+ char *value;
>+
>+ value = strchr (property, '=');
>+ if (value)
>+ {
>+ *value = 0;
>+ value++;
>+ apr_collapse_spaces (value, value);
>+ }
>+ else
>+ value = "";
>+ apr_collapse_spaces (property, property);
>+ len = strlen (property);
>+ if (len > 0)
>+ {
>+ property = apr_pstrdup (autoprops->pool, property);
>+ value = apr_pstrdup (autoprops->pool, value);
>+ apr_hash_set (autoprops->properties, property, len+1, value );
>
>
Why 'len+1'? There's no value in hashing the '\0' at the end of the
property name.
>+ }
>+ property = apr_strtok (NULL, ";", &last_token);
>+ }
>+ return TRUE;
>+}
>+
>+/* adds automatic properties, for a description look into client.h */
>+svn_error_t *
>+svn_client__add_auto_props (const char *path,
>+ svn_client_ctx_t *ctx,
>+ apr_pool_t *pool,
>+ svn_wc_adm_access_t *adm_access,
>+ const svn_delta_editor_t *editor,
>+ void *file_baton,
>+ const char **mimetype,
>+ int *executable)
>+{
>+ svn_config_t *cfg = NULL;
>+ auto_props_baton_t autoprops;
>
>
[*] here -- since this baton only lives within this function.
>+ const char *cfgvalue = 0;
>+ apr_hash_index_t *hi;
>+ svn_error_t *err;
>+
>+ /* check that we have config */
>+ if (!ctx->config)
>+ return SVN_NO_ERROR;
>+ cfg = apr_hash_get (ctx->config, SVN_CONFIG_CATEGORY_CONFIG,
>+ APR_HASH_KEY_STRING);
>
>
Indentation's wrong.
>+ if (!cfg)
>+ return SVN_NO_ERROR;
>+
>+ /* check that auto props is enabled */
>+ if (editor)
>+ svn_config_get (cfg, &cfgvalue, SVN_CONFIG_SECTION_MISCELLANY,
>+ SVN_CONFIG_OPTION_ENABLE_AUTO_PROPS_IMP, "false");
>+ else
>+ svn_config_get (cfg, &cfgvalue, SVN_CONFIG_SECTION_MISCELLANY,
>+ SVN_CONFIG_OPTION_ENABLE_AUTO_PROPS_ADD, "false");
>+ if (!cfgvalue || strcmp(cfgvalue, "true"))
>
>
We tend to use numeric comparison of the strcmp result; that is,
0 != strcmp (cfgvalue, "true")
Also, cfgvalue can't be NULL here, and you've forgotten the space before
the paren.
>+ return SVN_NO_ERROR;
>+
>+ /* search for auto props */
>+ autoprops.filename = svn_path_basename (path, pool);
>+ autoprops.pool = pool;
>+ autoprops.properties = apr_hash_make (pool);
>+ svn_config_enumerate (cfg, SVN_CONFIG_SECTION_AUTO_PROPS,
>+ auto_props_enumerator, &autoprops);
>+
>+ /* loop through the hashtable and add the properties */
>+ for (hi = apr_hash_first (pool, autoprops.properties); hi;
>+ hi = apr_hash_next (hi))
>
>
Indentation...
>+ {
>+ char *propname;
>+ svn_string_t propvalue;
>+
>+ apr_hash_this (hi, (const void**)&propname, 0, (void**)&propvalue.data);
>+ propvalue.len = strlen (propvalue.data);
>+ if (editor)
>+ {
>+ /* SVN_PROP_MIME_TYPE and SVN_PROP_EXECUTABLE
>+ * are returned to the caller when importing */
>+ if (!strcmp(propname, SVN_PROP_MIME_TYPE))
>
>
Boolean vs. numeric comparison again ...
>+ *mimetype = propvalue.data;
>+ else if (!strcmp (propname, SVN_PROP_EXECUTABLE))
>
>
... and again.
>+ *executable = TRUE;
>+ else
>+ err = editor->change_file_prop (file_baton, propname,
>+ &propvalue, pool);
>+ }
>+ else
>+ /* we're adding, so just set the property */
>+ err = svn_wc_prop_set (propname, &propvalue, path,
>+ adm_access, pool);
>+ }
>+
>+ return SVN_NO_ERROR;
>+}
>+
>+
> static svn_error_t *
> add_dir_recursive (const char *dirname,
> svn_wc_adm_access_t *adm_access,
>@@ -97,9 +237,13 @@
> SVN_ERR (add_dir_recursive (fullpath, dir_access, ctx, subpool));
>
> else if (this_entry.filetype == APR_REG)
>- SVN_ERR (svn_wc_add (fullpath, dir_access, NULL, SVN_INVALID_REVNUM,
>- ctx->cancel_func, ctx->cancel_baton,
>- ctx->notify_func, ctx->notify_baton, subpool));
>+ {
>+ SVN_ERR (svn_wc_add (fullpath, dir_access, NULL, SVN_INVALID_REVNUM,
>+ ctx->cancel_func, ctx->cancel_baton,
>+ ctx->notify_func, ctx->notify_baton, subpool));
>+ SVN_ERR (svn_client__add_auto_props (fullpath, ctx, subpool,
>+ adm_access, NULL, NULL, NULL, NULL));
>
>
Indentation...
>+ }
>
> /* Clean out the per-iteration pool. */
> svn_pool_clear (subpool);
>@@ -147,9 +291,14 @@
> if ((kind == svn_node_dir) && recursive)
> err = add_dir_recursive (path, adm_access, ctx, pool);
> else
>- err = svn_wc_add (path, adm_access, NULL, SVN_INVALID_REVNUM,
>- ctx->cancel_func, ctx->cancel_baton,
>- ctx->notify_func, ctx->notify_baton, pool);
>+ {
>+ err = svn_wc_add (path, adm_access, NULL, SVN_INVALID_REVNUM,
>+ ctx->cancel_func, ctx->cancel_baton,
>+ ctx->notify_func, ctx->notify_baton, pool);
>+ if (!err && kind == svn_node_file)
>+ err = svn_client__add_auto_props (path, ctx, pool, adm_access,
>+ NULL, NULL, NULL, NULL);
>+ }
>
> return err;
> }
>Index: subversion/clients/cmdline/cl.h
>===================================================================
>--- subversion/clients/cmdline/cl.h (revision 7208)
>+++ subversion/clients/cmdline/cl.h (working copy)
>@@ -68,7 +68,9 @@
> svn_cl__editor_cmd_opt,
> svn_cl__old_cmd_opt,
> svn_cl__new_cmd_opt,
>- svn_cl__config_dir_opt
>+ svn_cl__config_dir_opt,
>+ svn_cl__enable_autoprops_opt,
>+ svn_cl__disable_autoprops_opt
>
>
I'd suggest 'svn_cl__autoprops_opt' and 'svn_cl__no_autopropt_opt',
because....
> } svn_cl__longopt_t;
>
>
>@@ -121,6 +123,8 @@
> const char *new_target; /* diff target */
> svn_boolean_t relocate; /* rewrite urls (svn switch) */
> const char * config_dir; /* over-riding configuration directory */
>+ svn_boolean_t enable_autoprops; /* automatic properties (svn add|import) */
>+ svn_boolean_t disable_autoprops; /* automatic properties (svn add|import) */
>
>
... you'd get 'autoprops' and 'no_autoprops' here, and the comments
would line up. :-)
> } svn_cl__opt_state_t;
>
>
>Index: subversion/clients/cmdline/main.c
>===================================================================
>--- subversion/clients/cmdline/main.c (revision 7208)
>+++ subversion/clients/cmdline/main.c (working copy)
>@@ -124,6 +124,10 @@
> "relocate via URL-rewriting"},
> {"config-dir", svn_cl__config_dir_opt, 1,
> "read user configuration files from directory ARG"},
>+ {"auto-props", svn_cl__enable_autoprops_opt, 0,
>+ "enable automatic properties"},
>+ {"no-auto-props", svn_cl__disable_autoprops_opt, 0,
>+ "disable automatic properties"},
> {0, 0, 0, 0}
> };
>
>@@ -151,7 +155,8 @@
> "Put files and directories under revision control, scheduling\n"
> "them for addition to repository. They will be added in next commit.\n"
> "usage: add PATH...\n",
>- {svn_cl__targets_opt, 'N', 'q', svn_cl__config_dir_opt} },
>+ {svn_cl__targets_opt, 'N', 'q', svn_cl__config_dir_opt,
>+ svn_cl__enable_autoprops_opt, svn_cl__disable_autoprops_opt} },
>
> { "cat", svn_cl__cat, {0},
> "Output the content of specified files or URLs.\n"
>@@ -261,7 +266,8 @@
> " If PATH is omitted '.' is assumed. Parent directories are created\n"
> " as necessary in the repository.\n",
> {'m', 'F', 'q', 'N', SVN_CL__AUTH_OPTIONS, svn_cl__force_log_opt,
>- svn_cl__editor_cmd_opt, svn_cl__encoding_opt, svn_cl__config_dir_opt} },
>+ svn_cl__editor_cmd_opt, svn_cl__encoding_opt, svn_cl__config_dir_opt,
>+ svn_cl__enable_autoprops_opt, svn_cl__disable_autoprops_opt} },
>
> { "info", svn_cl__info, {0},
> "Display info about a resource.\n"
>@@ -841,6 +847,30 @@
> svn_path_canonicalize (opt_arg,
> pool));
> break;
>+ case svn_cl__enable_autoprops_opt:
>+ if (opt_state.disable_autoprops)
>+ {
>+ err = svn_error_create (SVN_ERR_CL_MUTUALLY_EXCLUSIVE_ARGS, NULL,
>+ "--auto-props and --no-autoprops are "
>+ "mutually exclusive.");
>
>
Whups, typo; should be '--no-auto-props'.
>+ svn_handle_error (err, stderr, FALSE);
>+ svn_pool_destroy (pool);
>+ return EXIT_FAILURE;
>+ }
>+ opt_state.enable_autoprops = TRUE;
>+ break;
>+ case svn_cl__disable_autoprops_opt:
>+ if (opt_state.enable_autoprops)
>+ {
>+ err = svn_error_create (SVN_ERR_CL_MUTUALLY_EXCLUSIVE_ARGS, NULL,
>+ "--auto-props and --no-autoprops are "
>
>
Same typo again.
>+ "mutually exclusive.");
>+ svn_handle_error (err, stderr, FALSE);
>+ svn_pool_destroy (pool);
>+ return EXIT_FAILURE;
>+ }
>+ opt_state.disable_autoprops = TRUE;
>+ break;
> default:
> /* Hmmm. Perhaps this would be a good place to squirrel away
> opts that commands like svn diff might need. Hmmm indeed. */
>@@ -993,6 +1023,23 @@
> svn_config_set (cfg, SVN_CONFIG_SECTION_HELPERS,
> SVN_CONFIG_OPTION_DIFF3_CMD, opt_state.merge_cmd);
>
>+ /* Update auto-props-enable option for add/import commands */
>+ if (subcommand->cmd_func == svn_cl__add
>+ || subcommand->cmd_func == svn_cl__import)
>+ {
>+ char *cfgopt = (subcommand->cmd_func == svn_cl__add)
>+ ? SVN_CONFIG_OPTION_ENABLE_AUTO_PROPS_ADD
>+ : SVN_CONFIG_OPTION_ENABLE_AUTO_PROPS_IMP;
>+ if (opt_state.enable_autoprops)
>+ {
>+ svn_config_set (cfg, SVN_CONFIG_SECTION_MISCELLANY, cfgopt, "true");
>+ }
>+ if (opt_state.disable_autoprops)
>+ {
>+ svn_config_set (cfg, SVN_CONFIG_SECTION_MISCELLANY, cfgopt, "false");
>+ }
>+ }
>+
> /* Set the log message callback function. Note that individual
> subcommands will populate the ctx.log_msg_baton */
> ctx.log_msg_func = svn_cl__get_log_message;
>Index: subversion/tests/clients/cmdline/autoprop_tests.py
>===================================================================
>--- subversion/tests/clients/cmdline/autoprop_tests.py (revision 0)
>+++ subversion/tests/clients/cmdline/autoprop_tests.py (revision 0)
>@@ -0,0 +1,468 @@
>+#!/usr/bin/env python
>+#
>+# autoprop_tests.py: testing automatic properties
>+#
>+# Subversion is a tool for revision control.
>+# See http://subversion.tigris.org for more information.
>+#
>+# ====================================================================
>+# Copyright (c) 2000-2003 CollabNet. All rights reserved.
>+#
>+# This software is licensed as described in the file COPYING, which
>+# you should have received as part of this distribution. The terms
>+# are also available at http://subversion.tigris.org/license-1.html.
>+# If newer versions of this license are posted there, you may use a
>+# newer version instead, at your option.
>+#
>+######################################################################
>+
>+# General modules
>+import string, sys, re, os, os.path, shutil
>+
>+# Our testing module
>+import svntest
>+
>+
>+# (abbreviation)
>+Skip = svntest.testcase.Skip
>+XFail = svntest.testcase.XFail
>+Item = svntest.wc.StateItem
>+
>+
>+# Helper functions
>+def check_prop(name, path, exp_out):
>+ """Verify that property NAME on PATH has a value of EXP_OUT"""
>+ # Not using run_svn because binary_mode must be set
>+ out, err = svntest.main.run_command(svntest.main.svn_binary, None, 1,
>+ 'pg', '--strict', name, path,
>+ '--config-dir', svntest.main.config_dir)
>
>
Is there any reason why you can't use svntest.main.run_svn here instead?
>+ if out != exp_out:
>+ print "svn pg --strict", name, "output does not match expected."
>
>
There's no need for this; command lines are already logged in verbose mode.
>+ print "Expected standard output: ", exp_out, "\n"
>+ print "Actual standard output: ", out, "\n"
>+ raise svntest.Failure
>+
>+
>+def check_proplist(path, exp_out):
>+ """Verify that property NAME on PATH has a value of EXP_OUT"""
>+ # Not using run_svn because binary_mode must be set
>+ out, err = svntest.main.run_command(svntest.main.svn_binary, None, 1,
>+ 'proplist', path,
>+ '--config-dir', svntest.main.config_dir)
>
>
Ditto...
>+ if len(out) == 0 and len(exp_out) == 0:
>+ # no properties expected and svn didn't output anything so it's ok
>+ return
>+
>+ if len(out) < 1:
>+ print "svn proplist output does not match expected."
>
>
Ditto...
>+ print "Expected standard output: ", exp_out, "\n"
>+ print "Actual standard output: ", out, "\n"
>+ raise svntest.Failure
>+ out2 = []
>+ if len(out) > 1:
>+ for line in out[1:]:
>+ out2 = out2 + [string.strip(line)]
>+ out2.sort()
>+ exp_out.sort()
>+ if out2 != exp_out:
>+ print "svn proplist output does not match expected."
>
>
Ditto...
>+ print "Expected result: ", exp_out, "\n"
>+ print "Actual result: ", out2, "\n"
>+ print "Actual standard output: ", out, "\n"
>+ raise svntest.Failure
>+
>+
>+######################################################################
>+# Tests
>+
>+#----------------------------------------------------------------------
>+
>+def create_config(config_dir, add_flag, imp_flag):
>+ "create config directories and files"
>+
>+ # config file names
>+ cfgfile_cfg = os.path.join(config_dir, 'config')
>+ cfgfile_srv = os.path.join(config_dir, 'server')
>+
>+ # create the directory
>+ if os.path.isdir(config_dir) == 0:
>
>
if not os.path.isdir(config_dir):
>+ os.makedirs(config_dir)
>+
>+ # create the file 'config'
>+ fd = os.open(cfgfile_cfg, os.O_WRONLY|os.O_CREAT)
>
>
Um. Just use the default stream functions here:
cfg = open(cfgfile_cfg, 'w')
cfg.write(...)
...
cfg.close()
>+ os.write(fd, '[miscellany]\n')
>+ os.write(fd, 'enable-auto-props-add = ' + add_flag + '\n')
>+ os.write(fd, 'enable-auto-props-imp = ' + imp_flag + '\n')
>+ os.write(fd, '\n')
>+ os.write(fd, '[auto-props]\n')
>+ os.write(fd, '*.c = cfile=yes\n')
>+ os.write(fd, '*.jpg = jpgfile=ja\n')
>+ os.write(fd, 'fubar* = tarfile=si\n')
>+ os.write(fd, 'foobar.lha = lhafile=da;lzhfile=niet\n')
>+ os.write(fd, '* = auto=oui\n')
>+ os.write(fd, '\n')
>+ os.close(fd)
>+
>+ # create the file 'server'
>+ fd = os.open(cfgfile_srv, os.O_WRONLY|os.O_CREAT)
>+ os.write(fd, '#\n')
>+ os.close(fd)
>+
>+
>+#----------------------------------------------------------------------
>+
>+def create_test_file(dir, name):
>+ "create a test file"
>+
>+ fd = os.open(os.path.join(dir, name), os.O_WRONLY|os.O_CREAT, 0644)
>+ os.write(fd, 'foo\nbar\nbaz\n')
>+ os.close(fd)
>
>
Same here.
>+
>+#----------------------------------------------------------------------
>+
>+def autopros_test(sbox, cmd, cfgtype, paramtype):
>+ "configurable autoprops test"
>
>
A bit of documentation about what the values of 'cfgtype' and 'paramtpe'
mean would be beneficial to the sanity of anyone else who reads this code.
>+ # some directories
>+ wc_dir = sbox.wc_dir
>+ tmp_dir = os.path.abspath('local_tmp')
>+ config_dir = os.path.join(tmp_dir, 'autoprops_config')
>+ repos_url = svntest.main.current_repo_url
>+
>+ # initialize parameters
>+ parameters = []
>+
>+ # add svn command
>+ if cmd == 1:
>+ parameters = parameters + ['import', '--username', main.wc_author,
>+ '--password', main.wc_passwd, '-m', 'bla']
>+ need_svn_up = 1
>+ files_dir = tmp_dir
>+ else:
>+ parameters = parameters + ['add']
>+ need_svn_up = 0
>+ files_dir = wc_dir
>+ parameters = parameters + ['--config-dir', config_dir]
>
>
Hm. How about adding an optional parameter to svntest.main.run_svn that
woudl override the default global config_dir? Then you could use
svntest.main.run_svn everywhere.
>+
>+ # set config flags
>+ if cfgtype == 1:
>+ create_config(config_dir, 'true', 'false')
>+ add_flag = 1
>+ imp_flag = 0
>+ elif cfgtype == 2:
>+ create_config(config_dir, 'false', 'true')
>+ add_flag = 0
>+ imp_flag = 1
>+ else:
>+ create_config(config_dir, 'false', 'false')
>+ add_flag = 0
>+ imp_flag = 0
>+
>+ # add comandline flags
>+ if paramtype == 1:
>+ parameters = parameters + ['--auto-props']
>+ add_flag = 1
>+ imp_flag = 1
>+ elif paramtype == 2:
>+ parameters = parameters + ['--no-auto-props']
>+ add_flag = 0
>+ imp_flag = 0
>+
>+ # create test files
>+ filenames = []
>+ filenames = filenames + ['foo.h']
>+ create_test_file(files_dir, filenames[len(filenames)-1])
>+ filenames = filenames + ['foo.c']
>+ create_test_file(files_dir, filenames[len(filenames)-1])
>+ filenames = filenames + ['foo.jpg']
>+ create_test_file(files_dir, filenames[len(filenames)-1])
>+ filenames = filenames + ['fubar.tar']
>+ create_test_file(files_dir, filenames[len(filenames)-1])
>+ filenames = filenames + ['foobar.lha']
>+ create_test_file(files_dir, filenames[len(filenames)-1])
>+
>+ # add/import the files
>+ for filename in filenames:
>+ filename = os.path.join(files_dir, filename)
>+ if cmd == 1:
>+ tmp_params = parameters + [filename, os.path.join(repos_url, filename)]
>+ else:
>+ tmp_params = parameters + [filename]
>+ out, err = svntest.main.run_command(svntest.main.svn_binary, None, 0, *tmp_params)
>+
>+ # do an svn up if needed
>+ if need_svn_up:
>+ svntest.main.run_command(svntest.main.svn_binary, None, 0, 'update')
>
>
svntest.main.run_svn
>+ #
>+ if cmd == 1:
>+ propflag = imp_flag
>+ else:
>+ propflag = add_flag
>+
>+ # check the properties
>+ if propflag:
>+ filename = os.path.join(wc_dir, 'foo.h' )
>+ check_proplist(filename,['auto'])
>+ check_prop('auto', filename, ['oui'])
>+ filename = os.path.join(wc_dir, 'foo.c' )
>+ check_proplist(filename,['cfile', 'auto'])
>+ check_prop('auto', filename, ['oui'])
>+ check_prop('cfile', filename, ['yes'])
>+ filename = os.path.join(wc_dir, 'foo.jpg' )
>+ check_proplist(filename,['jpgfile', 'auto'])
>+ check_prop('auto', filename, ['oui'])
>+ check_prop('jpgfile', filename, ['ja'])
>+ filename = os.path.join(wc_dir, 'fubar.tar' )
>+ check_proplist(filename,['tarfile', 'auto'])
>+ check_prop('auto', filename, ['oui'])
>+ check_prop('tarfile', filename, ['si'])
>+ filename = os.path.join(wc_dir, 'foobar.lha' )
>+ check_proplist(filename,['lhafile', 'lzhfile', 'auto'])
>+ check_prop('auto', filename, ['oui'])
>+ check_prop('lhafile', filename, ['da'])
>+ check_prop('lzhfile', filename, ['niet'])
>+ else:
>+ filename = os.path.join(wc_dir, 'foo.h' )
>+ check_proplist(filename,[])
>+ filename = os.path.join(wc_dir, 'foo.c' )
>+ check_proplist(filename,[])
>+ filename = os.path.join(wc_dir, 'foo.jpg' )
>+ check_proplist(filename,[])
>+ filename = os.path.join(wc_dir, 'fubar.tar' )
>+ check_proplist(filename,[])
>+ filename = os.path.join(wc_dir, 'foobar.lha' )
>+ check_proplist(filename,[])
>+
>+
>+#----------------------------------------------------------------------
>+
>+def autoprops_add_no_none(sbox):
>+ "add: config=no, commandline=none"
>+
>+ # Bootstrap
>+ sbox.build()
>+
>+ # cmd=add, config=no, commandline=none
>+ autopros_test(sbox, 'add', 0, 0)
>+
>+#----------------------------------------------------------------------
>+
>+def autoprops_add_add_none(sbox):
>+ "add: config=add, commandline=none"
>+
>+ # Bootstrap
>+ sbox.build()
>+
>+ # cmd=add, config=add, commandline=none
>+ autopros_test(sbox, 'add', 1, 0)
>+
>+#----------------------------------------------------------------------
>+
>+def autoprops_add_imp_none(sbox):
>+ "add: config=imp, commandline=none"
>+
>+ # Bootstrap
>+ sbox.build()
>+
>+ # cmd=add, config=imp, commandline=none
>+ autopros_test(sbox, 'add', 2, 0)
>+
>+#----------------------------------------------------------------------
>+
>+def autoprops_add_no_yes(sbox):
>+ "add: config=no, commandline=yes"
>+
>+ # Bootstrap
>+ sbox.build()
>+
>+ # cmd=add, config=no, commandline=yes
>+ autopros_test(sbox, 'add', 0, 1)
>+
>+#----------------------------------------------------------------------
>+
>+def autoprops_add_add_yes(sbox):
>+ "add: config=add, commandline=yes"
>+
>+ # Bootstrap
>+ sbox.build()
>+
>+ # cmd=add, config=add, commandline=yes
>+ autopros_test(sbox, 'add', 1, 1)
>+
>+#----------------------------------------------------------------------
>+
>+def autoprops_add_imp_yes(sbox):
>+ "add: config=imp, commandline=yes"
>+
>+ # Bootstrap
>+ sbox.build()
>+
>+ # cmd=add, config=imp, commandline=yes
>+ autopros_test(sbox, 'add', 2, 1)
>+
>+#----------------------------------------------------------------------
>+
>+def autoprops_add_no_no(sbox):
>+ "add: config=no, commandline=no"
>+
>+ # Bootstrap
>+ sbox.build()
>+
>+ # cmd=add, config=no, commandline=no
>+ autopros_test(sbox, 'add', 0, 2)
>+
>+#----------------------------------------------------------------------
>+
>+def autoprops_add_add_no(sbox):
>+ "add: config=add, commandline=no"
>+
>+ # Bootstrap
>+ sbox.build()
>+
>+ # cmd=add, config=add, commandline=no
>+ autopros_test(sbox, 'add', 1, 2)
>+
>+#----------------------------------------------------------------------
>+
>+def autoprops_add_imp_no(sbox):
>+ "add: config=imp, commandline=no"
>+
>+ # Bootstrap
>+ sbox.build()
>+
>+ # cmd=add, config=imp, commandline=no
>+ autopros_test(sbox, 'add', 2, 2)
>+
>+#----------------------------------------------------------------------
>+
>+def autoprops_imp_no_none(sbox):
>+ "import: config=no, commandline=none"
>+
>+ # Bootstrap
>+ sbox.build()
>+
>+ # cmd=import, config=no, commandline=none
>+ autopros_test(sbox, 'import', 0, 0)
>+
>+#----------------------------------------------------------------------
>+
>+def autoprops_imp_add_none(sbox):
>+ "import: config=add, commandline=none"
>+
>+ # Bootstrap
>+ sbox.build()
>+
>+ # cmd=import, config=add, commandline=none
>+ autopros_test(sbox, 'import', 1, 0)
>+
>+#----------------------------------------------------------------------
>+
>+def autoprops_imp_imp_none(sbox):
>+ "import: config=imp, commandline=none"
>+
>+ # Bootstrap
>+ sbox.build()
>+
>+ # cmd=import, config=imp, commandline=none
>+ autopros_test(sbox, 'import', 2, 0)
>+
>+#----------------------------------------------------------------------
>+
>+def autoprops_imp_no_yes(sbox):
>+ "import: config=no, commandline=yes"
>+
>+ # Bootstrap
>+ sbox.build()
>+
>+ # cmd=import, config=no, commandline=yes
>+ autopros_test(sbox, 'import', 0, 1)
>+
>+#----------------------------------------------------------------------
>+
>+def autoprops_imp_add_yes(sbox):
>+ "import: config=add, commandline=yes"
>+
>+ # Bootstrap
>+ sbox.build()
>+
>+ # cmd=import, config=add, commandline=yes
>+ autopros_test(sbox, 'import', 1, 1)
>+
>+#----------------------------------------------------------------------
>+
>+def autoprops_imp_imp_yes(sbox):
>+ "import: config=imp, commandline=yes"
>+
>+ # Bootstrap
>+ sbox.build()
>+
>+ # cmd=import, config=imp, commandline=yes
>+ autopros_test(sbox, 'import', 2, 1)
>+
>+#----------------------------------------------------------------------
>+
>+def autoprops_imp_no_no(sbox):
>+ "import: config=no, commandline=no"
>+
>+ # Bootstrap
>+ sbox.build()
>+
>+ # cmd=import, config=no, commandline=no
>+ autopros_test(sbox, 'import', 0, 2)
>+
>+#----------------------------------------------------------------------
>+
>+def autoprops_imp_add_no(sbox):
>+ "import: config=add, commandline=no"
>+
>+ # Bootstrap
>+ sbox.build()
>+
>+ # cmd=import, config=add, commandline=no
>+ autopros_test(sbox, 'import', 1, 2)
>+
>+#----------------------------------------------------------------------
>+
>+def autoprops_imp_imp_no(sbox):
>+ "import: config=imp, commandline=no"
>+
>+ # Bootstrap
>+ sbox.build()
>+
>+ # cmd=import, config=imp, commandline=no
>+ autopros_test(sbox, 'import', 2, 2)
>+
>+
>+########################################################################
>+# Run the tests
>+
>+
>+# list all tests here, starting with None:
>+test_list = [ None,
>+ autoprops_add_no_none,
>+ autoprops_add_add_none,
>+ autoprops_add_imp_none,
>+ autoprops_add_no_yes,
>+ autoprops_add_add_yes,
>+ autoprops_add_imp_yes,
>+ autoprops_add_no_no,
>+ autoprops_add_add_no,
>+ autoprops_add_imp_no,
>+ autoprops_imp_no_none,
>+ autoprops_imp_add_none,
>+ autoprops_imp_imp_none,
>+ autoprops_imp_no_yes,
>+ autoprops_imp_add_yes,
>+ autoprops_imp_imp_yes,
>+ autoprops_imp_no_no,
>+ autoprops_imp_add_no,
>+ autoprops_imp_imp_no,
>+ ]
>+
>+if __name__ == '__main__':
>+ svntest.main.run_tests(test_list)
>+ # NOTREACHED
>+
>+
>+### End of file.
>
>Property changes on: subversion/tests/clients/cmdline/autoprop_tests.py
>___________________________________________________________________
>Name: svn:executable
> + *
>
>----- end of auto-props patch -----
>
>
--
Brane Čibej <brane_at_xbc.nu> http://www.xbc.nu/brane/
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Received on Mon Sep 29 00:18:00 2003