[issue #2880] 'svn commit --changelist NO_SUCH_CHANGELIST' should warn

I think we could've fixed this in the command-line client by examining
the commit_info returned by svn_client_commit4(), rather than handling
this in Subversion's core libraries.

As it stands, you get back a SVN_ERR_UNKNOWN_CHANGELIST with some
other error wrapped around it (which is kinda yucky); one must use
svn_error_root_cause() to examine the error chain to determine whether
or not to downplay the error to a warning.

On Fri, 28 Sep 2007, dlr@tigris.org wrote:

> Author: dlr
> Date: Fri Sep 28 13:36:13 2007
> New Revision: 26841
>
> Log:
> Fix issue #2880, 'svn commit --changelist NO_SUCH_CHANGELIST' should warn.
>
> * subversion/include/svn_client.h
> (svn_client_commit4): Add blurb about when
> SVN_ERR_UNKNOWN_CHANGELIST is returned.
>
> * subversion/include/svn_error_codes.h
> (SVN_ERR_UNKNOWN_CHANGELIST): Add new error code.
>
> * subversion/libsvn_client/commit_util.c
> (svn_client__harvest_committables): If none of the paths we examined
> were in the changelist, return SVN_ERR_UNKNOWN_CHANGELIST.
>
> * subversion/svn/commit-cmd.c
> Include svn_error.h and svn_error_codes.h.
> (svn_cl__commit): Transform SVN_ERR_UNKNOWN_CHANGELIST from a hard
> error into a warning.
>
> * subversion/tests/cmdline/commit_tests.py
> (changelist_near_conflict): Rename from changelist(), and remove
> test for an unknwown changelist.
> (no_such_changelist): Factor new test out of old changelist() test,
> now checking for specific output.
> (test_list): Switch changelist_near_conflict test back to to PASS,
> and add no_such_changelist test (also PASS).
>
>
> Modified:
> trunk/subversion/include/svn_client.h
> trunk/subversion/include/svn_error_codes.h
> trunk/subversion/libsvn_client/commit_util.c
> trunk/subversion/svn/commit-cmd.c
> trunk/subversion/tests/cmdline/commit_tests.py
>
> Modified: trunk/subversion/include/svn_client.h
> URL: http://svn.collab.net/viewvc/svn/trunk/subversion/include/svn_client.h?pathrev=26841&r1=26840&r2=26841
> ==============================================================================
> --- trunk/subversion/include/svn_client.h (original)
> +++ trunk/subversion/include/svn_client.h Fri Sep 28 13:36:13 2007
> @@ -1570,7 +1570,9 @@
> * on items that are committed; that is, don't commit anything unless
> * it's a member of changelist @a changelist_name. After the commit
> * completes successfully, remove changelist associations from the
> - * targets, unless @a keep_changelist is set.
> + * targets, unless @a keep_changelist is set. If no items are
> + * committed, return an error with @c SVN_ERR_UNKNOWN_CHANGELIST as
> + * its root cause.
> *
> * Use @a pool for any temporary allocations.
> *
>
> Modified: trunk/subversion/include/svn_error_codes.h
> URL: http://svn.collab.net/viewvc/svn/trunk/subversion/include/svn_error_codes.h?pathrev=26841&r1=26840&r2=26841
> ==============================================================================
> --- trunk/subversion/include/svn_error_codes.h (original)
> +++ trunk/subversion/include/svn_error_codes.h Fri Sep 28 13:36:13 2007
> @@ -1083,6 +1083,10 @@
> SVN_ERR_MISC_CATEGORY_START + 23,
> "Iteration terminated before completion")
>
> + SVN_ERRDEF(SVN_ERR_UNKNOWN_CHANGELIST,
> + SVN_ERR_MISC_CATEGORY_START + 24,
> + "Unknown changelist")
> +
> /* command-line client errors */
>
> SVN_ERRDEF(SVN_ERR_CL_ARG_PARSING_ERROR,
>
> Modified: trunk/subversion/libsvn_client/commit_util.c
> URL: http://svn.collab.net/viewvc/svn/trunk/subversion/libsvn_client/commit_util.c?pathrev=26841&r1=26840&r2=26841
> ==============================================================================
> --- trunk/subversion/libsvn_client/commit_util.c (original)
> +++ trunk/subversion/libsvn_client/commit_util.c Fri Sep 28 13:36:13 2007
> @@ -857,6 +857,11 @@
> }
> while (i < targets->nelts);
>
> + if (changelist_name && apr_hash_count(*committables) == 0)
> + /* None of the paths we examined were in the changelist. */
> + return svn_error_createf(SVN_ERR_UNKNOWN_CHANGELIST, NULL,
> + _("Unknown changelist '%s'"), changelist_name);
> +
> /* Make sure that every path in danglers is part of the commit. */
> SVN_ERR(svn_iter_apr_hash(NULL,
> danglers, validate_dangler, *committables, pool));
>
> Modified: trunk/subversion/svn/commit-cmd.c
> URL: http://svn.collab.net/viewvc/svn/trunk/subversion/svn/commit-cmd.c?pathrev=26841&r1=26840&r2=26841
> ==============================================================================
> --- trunk/subversion/svn/commit-cmd.c (original)
> +++ trunk/subversion/svn/commit-cmd.c Fri Sep 28 13:36:13 2007
> @@ -24,6 +24,8 @@
>
> #include <apr_general.h>
>
> +#include "svn_error.h"
> +#include "svn_error_codes.h"
> #include "svn_wc.h"
> #include "svn_client.h"
> #include "svn_path.h"
> @@ -39,6 +41,7 @@
> void *baton,
> apr_pool_t *pool)
> {
> + svn_error_t *err;
> svn_cl__opt_state_t *opt_state = ((svn_cl__cmd_baton_t *) baton)->opt_state;
> svn_client_ctx_t *ctx = ((svn_cl__cmd_baton_t *) baton)->ctx;
> apr_array_header_t *targets;
> @@ -96,16 +99,27 @@
> ctx->revprop_table = opt_state->revprop_table;
>
> /* Commit. */
> - SVN_ERR(svn_cl__cleanup_log_msg
> - (ctx->log_msg_baton3,
> - svn_client_commit4(&commit_info,
> - targets,
> - opt_state->depth,
> - no_unlock,
> - opt_state->keep_changelist,
> - opt_state->changelist,
> - ctx,
> - pool)));
> + err = svn_client_commit4(&commit_info,
> + targets,
> + opt_state->depth,
> + no_unlock,
> + opt_state->keep_changelist,
> + opt_state->changelist,
> + ctx,
> + pool);
> + if (err)
> + {
> + svn_error_t *root_err = svn_error_root_cause(err);
> + if (root_err->apr_err == SVN_ERR_UNKNOWN_CHANGELIST)
> + {
> + /* Convert a hard error about an unknown changelist into a
> + soft warning. */
> + svn_handle_warning2(stdout, root_err, "svn: ");
> + svn_error_clear(err);
> + err = SVN_NO_ERROR;
> + }
> + }
> + SVN_ERR(svn_cl__cleanup_log_msg(ctx->log_msg_baton3, err));
> if (commit_info && ! opt_state->quiet)
> SVN_ERR(svn_cl__print_commit_info(commit_info, pool));
>
>
> Modified: trunk/subversion/tests/cmdline/commit_tests.py
> URL: http://svn.collab.net/viewvc/svn/trunk/subversion/tests/cmdline/commit_tests.py?pathrev=26841&r1=26840&r2=26841
> ==============================================================================
> --- trunk/subversion/tests/cmdline/commit_tests.py (original)
> +++ trunk/subversion/tests/cmdline/commit_tests.py Fri Sep 28 13:36:13 2007
> @@ -2553,7 +2553,7 @@
>
> #----------------------------------------------------------------------
>
> -def changelist(sbox):
> +def changelist_near_conflict(sbox):
> "'svn commit --changelist=foo' above a conflict"
>
> sbox.build()
> @@ -2587,14 +2587,24 @@
> "--changelist=" + changelist_name,
> "-m", "msg", wc_dir)
>
> +
> +#----------------------------------------------------------------------
> +
> +def no_such_changelist(sbox):
> + "'svn commit --changelist=not-found' should warn"
> +
> + sbox.build()
> + wc_dir = sbox.wc_dir
> +
> # Attempt to commit a non-existent changelist.
> expected_output = svntest.wc.State(wc_dir, {})
> - svntest.actions.run_and_verify_commit(wc_dir,
> - expected_output,
> - expected_status,
> - ".+", None, None, None, None,
> - "--changelist=does-not-exist",
> - "-m", "msg", wc_dir)
> + expected_status = svntest.actions.get_virginal_state(wc_dir, 1)
> + svntest.actions.run_and_verify_svn("Attempt to commit a changelist with no "
> + "relevant paths should warn",
> + ".*Unknown changelist 'not-found'", [],
> + "commit", "--changelist=not-found",
> + "-m", "msg", wc_dir)
> +
>
> ########################################################################
> # Run the tests
> @@ -2654,7 +2664,8 @@
> start_commit_hook_test,
> pre_commit_hook_test,
> versioned_log_message,
> - XFail(changelist),
> + changelist_near_conflict,
> + no_such_changelist,
> ]
>
> if __name__ == '__main__':