You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@subversion.apache.org by Daniel Rall <dl...@collab.net> on 2007/09/28 20:45:56 UTC

[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__':