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 Näslund <da...@longitudo.com> on 2009/08/08 15:48:07 UTC

Re: [PATCH v4] - the right patch!

Hi again!

The patch I submitted earlier was an old one. Really sorry. I need some
smart way of organazing my patches, mercurial patch queues perhaps?

I have run make check today with no failures.
  
[[[
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.
]]]


> > 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)

Yep, it's a list. I'm learning python bit by bit.
 
> 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?

It was caused by this line in update-cmd.c:

  if (! opt_state->quiet)
    svn_cl__get_notifier(&ctx->notify_func2, &ctx->notify_baton2,
                         FALSE, FALSE, FALSE, pool);

I added a check if we are operating quietly.

  if (! opt_state->quiet)
    SVN_ERR(svn_cl__print_conflict_stats(ctx->notify_baton2, pool));

> > Index: subversion/svn/update-cmd.c
> > ==================================================================--- subversion/svn/update-cmd.c	(revision 38504)
> > 
> > -  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.

Fixed

> 
> > Index: subversion/svn/switch-cmd.c
> > ==================================================================--- subversion/svn/switch-cmd.c	(revision 38504)
> > +  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.

Fixed
 
> > Index: subversion/svn/notify.c
> > ==================================================================--- subversion/svn/notify.c	(revision 38504)
> > 
> >      case svn_wc_notify_merge_completed:
> >          break;
> 
> I'd remove the svn_wc_notify_merge_completed case entirely
> because it does not differ from the default case.
 
Done.

Mvh
Daniel