On Fri, Aug 07, 2009 at 06:44:24PM +0200, Daniel Näslund wrote:
>
> Ping!
> This patch hasn't received any attention in a couple of days.
>
> Sorry about the diff headers. Tigris concatenates some line in the diffs
> for me. (I blame tigris for everything :-))
>
> Mvh
> Daniel
>
> > Thanks! My python is a bit rusty (hrm now I know some).
> >
> > Make check passed but with two failures in basic_tests.py. When skipping
> > no summary was printed before. That was caused by theese lines in
> > subversion/libsvn_client/update.c:
> >
> > 344 err = svn_client__update_internal(&result_rev, path, revision, depth,
> > 345 depth_is_sticky, ignore_externals,
> > 346 allow_unver_obstructions,
> > 347 &sleep, TRUE, ctx, subpool);
> > 348 if (err && err->apr_err != SVN_ERR_WC_NOT_DIRECTORY)
> > 349 {
> > 350 return svn_error_return(err);
> > 351 }
> > 352 else if (err)
> > 353 {
> > 354 /* SVN_ERR_WC_NOT_DIRECTORY: it's not versioned */
> > 355 svn_error_clear(err);
> > 356 err = SVN_NO_ERROR;
> > 357 result_rev = SVN_INVALID_REVNUM;
> > 358 if (ctx->notify_func2)
> > 359 (*ctx->notify_func2)(ctx->notify_baton2,
> > 360 svn_wc_create_notify(path,
> > 361 svn_wc_notify_skip,
> > 362 subpool), subpool);
> > 363 }
> >
> > A svn_wc_notify_skip was sent after the svn_wc_notify_update_completed.
> > But now when the summary is not printed in response to an update
> > completed notification but after the call to svn_client_update3 a
> > summary will be printed even if the only action is skipping one
> > directory. Since the summary includes a line for skips I think that the
> > test should be changed and has done so. The patch now passes make
> > check.
>
> [[[
> Fix issue #3342: Summary of conflicts printed at end of up/sw/merge
> * subversion/svn/merge-cmd.c
> (svn_cl__merge): Call svn_cl__print_conflict_stats.
>
> * subversion/svn/cl.h
> (svn_cl__print_conflict_stats): Declare.
>
> * subversion/svn/update-cmd.c
> (svn_cl__update): Call svn_cl__print_conflict_stats.
>
> * subversion/svn/switch-cmd.c
> (svn_cl__switch): Call svn_cl__print_conflict_stats.
>
> * subversion/svn_notify.c
> (svn_cl__print_conflict_stats): Changed name from print_conflict_stats.
>
> * subversion/svn_notify.c
> (notify): Remove references to print_conflict_stats. Do not clear
> counters for conflicts.
>
> * subversion/tests/cmdline/basic_tests.py
> (basic_update): Changed tests involving skipping to include summary.
> ]]]
>
> Mvh
> Daniel Näslund
> Index: subversion/tests/cmdline/basic_tests.py
> ==================================================================--- subversion/tests/cmdline/basic_tests.py (revision 38504)
> +++ subversion/tests/cmdline/basic_tests.py (arbetskopia)
> @@ -198,7 +198,8 @@
> # path, are skipped and do not raise an error
> xx_path = os.path.join(wc_dir, 'xx', 'xx')
> exit_code, out, err = svntest.actions.run_and_verify_svn(
> - "update xx/xx", ["Skipped '"+xx_path+"'\n"], [],
> + "update xx/xx",
> + ["Skipped '"+xx_path+"'\nSummary of conflicts:\n Skipped paths: 1\n"], [],
This won't work.
You need to put each line into a separate element of the list:
exit_code, out, err = svntest.actions.run_and_verify_svn(
"update xx/xx",
["Skipped '"+xx_path+"'\n", "Summary of conflicts:\n",
" Skipped paths: 1\n"],
[], 'update', xx_path)
When I run this test with that change, I get:
update xx/xx
EXPECTED STDOUT:
ACTUAL STDOUT:
Summary of conflicts in external item:
Skipped paths: 134639479
EXCEPTION: SVNLineUnequal
Looks like something isn't initialised somewhere. Can you investigate?
> 'update', xx_path)
> exit_code, out, err = svntest.actions.run_and_verify_svn(
> "update xx/xx", [], [],
> Index: subversion/svn/merge-cmd.c
> ==================================================================--- subversion/svn/merge-cmd.c (revision 38504)
> +++ subversion/svn/merge-cmd.c (arbetskopia)
> @@ -340,6 +340,8 @@
> pool);
> }
>
> + SVN_ERR(svn_cl__print_conflict_stats(ctx->notify_baton2, pool));
> +
> if (err && (! opt_state->reintegrate))
> return svn_cl__may_need_force(err);
>
> Index: subversion/svn/cl.h
> ==================================================================--- subversion/svn/cl.h (revision 38504)
> +++ subversion/svn/cl.h (arbetskopia)
> @@ -537,6 +537,12 @@
> svn_boolean_t suppress_final_line,
> apr_pool_t *pool);
>
> +/* Print conflict stats accumulated in NOTIFY_BATON.
> + * Return any error encountered during printing.
> + * Do all allocations in POOL.*/
> +svn_error_t *
> +svn_cl__print_conflict_stats(void *notify_baton, apr_pool_t *pool);
> +
>
> /*** Log message callback stuffs. ***/
>
> Index: subversion/svn/update-cmd.c
> ==================================================================--- subversion/svn/update-cmd.c (revision 38504)
> +++ subversion/svn/update-cmd.c (arbetskopia)
> @@ -88,10 +88,14 @@
> depth_is_sticky = FALSE;
> }
>
> - return svn_client_update3(NULL, targets,
> + SVN_ERR(svn_client_update3(NULL, targets,
> &(opt_state->start_revision),
> depth, depth_is_sticky,
> opt_state->ignore_externals,
> opt_state->force,
> - ctx, pool);
> + ctx, pool));
Parameter indentation here is off.
> +
> + SVN_ERR(svn_cl__print_conflict_stats(ctx->notify_baton2, pool));
> +
> + return SVN_NO_ERROR;
> }
> Index: subversion/svn/switch-cmd.c
> ==================================================================--- subversion/svn/switch-cmd.c (revision 38504)
> +++ subversion/svn/switch-cmd.c (arbetskopia)
> @@ -161,8 +161,12 @@
> }
>
> /* Do the 'switch' update. */
> - return svn_client_switch2(NULL, target, switch_url, &peg_revision,
> + SVN_ERR(svn_client_switch2(NULL, target, switch_url, &peg_revision,
> &(opt_state->start_revision), depth,
> depth_is_sticky, opt_state->ignore_externals,
> - opt_state->force, ctx, pool);
> + opt_state->force, ctx, pool));
Same.
> +
> + SVN_ERR(svn_cl__print_conflict_stats(ctx->notify_baton2, pool));
> +
> + return SVN_NO_ERROR;
> }
> Index: subversion/svn/notify.c
> ==================================================================--- subversion/svn/notify.c (revision 38504)
> +++ subversion/svn/notify.c (arbetskopia)
> @@ -69,12 +69,10 @@
> };
>
>
> -/* Print conflict stats accumulated in notify baton NB.
> - * Return any error encountered during printing.
> - * Do all allocations in POOL.*/
> -static svn_error_t *
> -print_conflict_stats(struct notify_baton *nb, apr_pool_t *pool)
> +svn_error_t *
> +svn_cl__print_conflict_stats(void *notify_baton, apr_pool_t *pool)
> {
> + struct notify_baton *nb = notify_baton;
> const char *header;
> unsigned int text_conflicts;
> unsigned int prop_conflicts;
> @@ -449,27 +447,15 @@
> }
> }
>
> - if ((err = print_conflict_stats(nb, pool)))
> - goto print_error;
> -
> if (nb->in_external)
> {
> nb->in_external = FALSE;
> - nb->ext_text_conflicts = nb->ext_prop_conflicts
> - = nb->ext_tree_conflicts = nb->ext_skipped_paths = 0;
> if ((err = svn_cmdline_printf(pool, "\n")))
> goto print_error;
> }
> - else
> - nb->text_conflicts = nb->prop_conflicts
> - = nb->tree_conflicts = nb->skipped_paths = 0;
> break;
>
> case svn_wc_notify_merge_completed:
> - if ((err = print_conflict_stats(nb, pool)))
> - goto print_error;
> - nb->text_conflicts = nb->prop_conflicts
> - = nb->tree_conflicts = nb->skipped_paths = 0;
> break;
I'd remove the svn_wc_notify_merge_completed case entirely
because it does not differ from the default case.
Stefan
>
> case svn_wc_notify_status_external:
------------------------------------------------------
http://subversion.tigris.org/ds/viewMessage.do?dsForumId=462&dsMessageId=2381406
Received on 2009-08-07 19:49:30 CEST