Dan, I was just Friday thinking that we needed to add such a thing.
Thanks for adding it!
However, I'm not thrilled about the all-or-nothing approach you
inherited from 'svnadmin setlog' in the hook bypassing stuff. There are
many times where an admin would like to bypass the pre-revprop-change
hook (because, as an admin, he or she has a right to not get bounced by
any rules present in that hook) but still wants to have the
post-revprop-change hook fire. The same reasoning explains why I added
both '--use-pre-commit-hook' and '--use-post-commit-hook' as individual
options to 'svnadmin load'. I want that level of control in this case, too.
Have you an opinion on the matter, or were you simply shooting for
low-hanging expansion of 'setlog' to something more generic?
dlr@tigris.org wrote:
> Author: dlr
> Date: Mon Oct 2 14:33:25 2006
> New Revision: 21736
>
> Log:
> Add an 'svnadmin setrevprop' command. This new generic command could
> theoretically replace 'svnadmin setlog'.
>
> Alternately, we could rename this command 'setprop', if there's a
> desire that 'svnadmin' could be used eventually to set normal
> properties (meaning that it would be making commits).
>
>
> * subversion/svnadmin/main.c
> (set_revprop): A helper function which contains the guts of
> 'setrevprop' and 'setlog', factored out of the previous 'setlog'
> implementation.
>
> (subcommand_setlog): Delegate to set_revprop().
>
> (subcommand_setrevprop): Add new sub-command function, and to
> svn_opt_subcommand_t list.
>
> (cmd_table): Add help for "setrevprop".
>
> * subversion/tests/cmdline/svnadmin_tests.py
> (setrevprop): A new test for 'setlog' and 'setrevprop', which
> bypasses hook scripts.
>
> (test_list): Add setrevprop to the list.
>
>
> Modified:
> trunk/subversion/svnadmin/main.c
> trunk/subversion/tests/cmdline/svnadmin_tests.py
>
> Modified: trunk/subversion/svnadmin/main.c
> URL: http://svn.collab.net/viewvc/svn/trunk/subversion/svnadmin/main.c?pathrev=21736&r1=21735&r2=21736
> ==============================================================================
> --- trunk/subversion/svnadmin/main.c (original)
> +++ trunk/subversion/svnadmin/main.c Mon Oct 2 14:33:25 2006
> @@ -194,6 +194,7 @@
> subcommand_recover,
> subcommand_rmlocks,
> subcommand_rmtxns,
> + subcommand_setrevprop,
> subcommand_setlog,
> subcommand_verify;
>
> @@ -386,6 +387,19 @@
> "Delete the named transaction(s).\n"),
> {'q'} },
>
> + {"setrevprop", subcommand_setrevprop, {0}, N_
> + ("usage: svnadmin setrevprop REPOS_PATH -r REVISION NAME FILE\n\n"
> + "Set the property NAME on revision REVISION to the contents of FILE. Use\n"
> + "--bypass-hooks to avoid triggering the revision-property-related hooks\n"
> + "(for example, if you do not want an email notification sent\n"
> + "from your post-revprop-change hook, or because the modification of\n"
> + "revision properties has not been enabled in the pre-revprop-change\n"
> + "hook).\n\n"
> + "NOTE: Revision properties are not versioned (e.g. no revision history\n"
> + "is maintained), so this command will permanently overwrite the previous\n"
> + "value for the property.\n"),
> + {'r', svnadmin__bypass_hooks} },
> +
> {"setlog", subcommand_setlog, {0}, N_
> ("usage: svnadmin setlog REPOS_PATH -r REVISION FILE\n\n"
> "Set the log-message on revision REVISION to the contents of FILE. Use\n"
> @@ -911,41 +925,25 @@
> }
>
>
> -/* This implements `svn_opt_subcommand_t'. */
> +/* A helper for the 'setrevprop' and 'setlog' commands. */
> static svn_error_t *
> -subcommand_setlog(apr_getopt_t *os, void *baton, apr_pool_t *pool)
> +set_revprop(const char *prop_name, const char *filename,
> + struct svnadmin_opt_state *opt_state, apr_pool_t *pool)
> {
> - struct svnadmin_opt_state *opt_state = baton;
> svn_repos_t *repos;
> + svn_string_t *prop_value = svn_string_create("", pool);
> svn_stringbuf_t *file_contents;
> const char *filename_utf8;
> - apr_array_header_t *args;
> - svn_string_t *log_contents = svn_string_create("", pool);
>
> - if (opt_state->start_revision.kind != svn_opt_revision_number)
> - return svn_error_createf(SVN_ERR_CL_ARG_PARSING_ERROR, NULL,
> - _("Missing revision"));
> - else if (opt_state->end_revision.kind != svn_opt_revision_unspecified)
> - return svn_error_createf(SVN_ERR_CL_ARG_PARSING_ERROR, NULL,
> - _("Only one revision allowed"));
> -
> - SVN_ERR(svn_opt_parse_all_args(&args, os, pool));
> -
> - if (args->nelts != 1)
> - return svn_error_createf(SVN_ERR_CL_ARG_PARSING_ERROR, NULL,
> - _("Exactly one file argument required"));
> -
> - SVN_ERR(svn_utf_cstring_to_utf8(&filename_utf8,
> - APR_ARRAY_IDX(args, 0, const char *),
> - pool));
> + SVN_ERR(svn_utf_cstring_to_utf8(&filename_utf8, filename, pool));
> filename_utf8 = svn_path_internal_style(filename_utf8, pool);
> +
> SVN_ERR(svn_stringbuf_from_file(&file_contents, filename_utf8, pool));
>
> - log_contents->data = file_contents->data;
> - log_contents->len = file_contents->len;
> + prop_value->data = file_contents->data;
> + prop_value->len = file_contents->len;
>
> - SVN_ERR(svn_subst_translate_string(&log_contents, log_contents,
> - NULL, pool));
> + SVN_ERR(svn_subst_translate_string(&prop_value, prop_value, NULL, pool));
>
> /* Open the filesystem */
> SVN_ERR(open_repos(&repos, opt_state->repository_path, pool));
> @@ -956,14 +954,14 @@
> {
> svn_fs_t *fs = svn_repos_fs(repos);
> SVN_ERR(svn_fs_change_rev_prop
> - (fs, opt_state->start_revision.value.number,
> - SVN_PROP_REVISION_LOG, log_contents, pool));
> + (fs, opt_state->start_revision.value.number,
> + prop_name, prop_value, pool));
> }
> else
> {
> SVN_ERR(svn_repos_fs_change_rev_prop2
> (repos, opt_state->start_revision.value.number,
> - NULL, SVN_PROP_REVISION_LOG, log_contents, NULL, NULL, pool));
> + NULL, prop_name, prop_value, NULL, NULL, pool));
> }
>
> return SVN_NO_ERROR;
> @@ -972,6 +970,59 @@
>
> /* This implements `svn_opt_subcommand_t'. */
> static svn_error_t *
> +subcommand_setrevprop(apr_getopt_t *os, void *baton, apr_pool_t *pool)
> +{
> + struct svnadmin_opt_state *opt_state = baton;
> + apr_array_header_t *args;
> +
> + if (opt_state->start_revision.kind != svn_opt_revision_number)
> + return svn_error_createf(SVN_ERR_CL_ARG_PARSING_ERROR, NULL,
> + _("Missing revision"));
> + else if (opt_state->end_revision.kind != svn_opt_revision_unspecified)
> + return svn_error_createf(SVN_ERR_CL_ARG_PARSING_ERROR, NULL,
> + _("Only one revision allowed"));
> +
> + SVN_ERR(svn_opt_parse_all_args(&args, os, pool));
> +
> + if (args->nelts != 2)
> + return svn_error_createf(SVN_ERR_CL_ARG_PARSING_ERROR, NULL,
> + _("Exactly one property name and one file "
> + "argument required"));
> +
> + return set_revprop(APR_ARRAY_IDX(args, 0, const char *),
> + APR_ARRAY_IDX(args, 1, const char *),
> + opt_state, pool);
> +}
> +
> +
> +/* This implements `svn_opt_subcommand_t'. */
> +static svn_error_t *
> +subcommand_setlog(apr_getopt_t *os, void *baton, apr_pool_t *pool)
> +{
> + struct svnadmin_opt_state *opt_state = baton;
> + apr_array_header_t *args;
> +
> + if (opt_state->start_revision.kind != svn_opt_revision_number)
> + return svn_error_createf(SVN_ERR_CL_ARG_PARSING_ERROR, NULL,
> + _("Missing revision"));
> + else if (opt_state->end_revision.kind != svn_opt_revision_unspecified)
> + return svn_error_createf(SVN_ERR_CL_ARG_PARSING_ERROR, NULL,
> + _("Only one revision allowed"));
> +
> + SVN_ERR(svn_opt_parse_all_args(&args, os, pool));
> +
> + if (args->nelts != 1)
> + return svn_error_createf(SVN_ERR_CL_ARG_PARSING_ERROR, NULL,
> + _("Exactly one file argument required"));
> +
> + return set_revprop(SVN_PROP_REVISION_LOG,
> + APR_ARRAY_IDX(args, 0, const char *),
> + opt_state, pool);
> +}
> +
> +
> +/* This implements `svn_opt_subcommand_t'. */
> +static svn_error_t *
> subcommand_verify(apr_getopt_t *os, void *baton, apr_pool_t *pool)
> {
> struct svnadmin_opt_state *opt_state = baton;
>
> Modified: trunk/subversion/tests/cmdline/svnadmin_tests.py
> URL: http://svn.collab.net/viewvc/svn/trunk/subversion/tests/cmdline/svnadmin_tests.py?pathrev=21736&r1=21735&r2=21736
> ==============================================================================
> --- trunk/subversion/tests/cmdline/svnadmin_tests.py (original)
> +++ trunk/subversion/tests/cmdline/svnadmin_tests.py Mon Oct 2 14:33:25 2006
> @@ -355,8 +355,47 @@
> if contents1 != contents2:
> print "Error: db/format file contents do not match after hotcopy"
> raise svntest.Failure
> -
>
> +#----------------------------------------------------------------------
> +
> +def setrevprop(sbox):
> + "'setlog' and 'setrevprop', bypassing hooks'"
> + sbox.build()
> +
> + # Try a simple log modification, without bypassing hook scripts.
> + iota_path = os.path.join(sbox.wc_dir, "iota")
> + output, errput = svntest.main.run_svnadmin("setlog", sbox.repo_dir,
> + "-r0", "--bypass-hooks",
> + iota_path)
> + if errput:
> + print "Error: 'setlog' failed"
> + raise svntest.Failure
> +
> + # Verify that the revprop value matches what we set when retrieved
> + # through the client.
> + svntest.actions.run_and_verify_svn(None,
> + [ "This is the file 'iota'.\n", "\n" ],
> + [], "propget", "--revprop", "-r0",
> + "svn:log", sbox.wc_dir)
> +
> + # Try an author log modification, this time bypassing hook scripts.
> + foo_path = os.path.join(sbox.wc_dir, "foo")
> + fp = open(foo_path, "w")
> + fp.write("foo")
> + fp.close()
> +
> + output, errput = svntest.main.run_svnadmin("setrevprop", sbox.repo_dir,
> + "-r0", "--bypass-hooks",
> + "svn:author", foo_path)
> + if errput:
> + print "Error: 'setrevprop' failed"
> + raise svntest.Failure
> +
> + # Verify that the revprop value matches what we set when retrieved
> + # through the client.
> + svntest.actions.run_and_verify_svn(None, [ "foo\n" ], [], "propget",
> + "--revprop", "-r0", "svn:author",
> + sbox.wc_dir)
>
>
> ########################################################################
> @@ -373,6 +412,7 @@
> dump_quiet,
> hotcopy_dot,
> hotcopy_format,
> + setrevprop,
> ]
>
> if __name__ == '__main__':
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: svn-unsubscribe@subversion.tigris.org
> For additional commands, e-mail: svn-help@subversion.tigris.org
>
--
C. Michael Pilato <cmpilato@collab.net>
CollabNet <> www.collab.net <> Distributed Development On Demand
Received on Mon Oct 2 23:53:57 2006