You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@subversion.apache.org by Julian Foad <ju...@btopenworld.com> on 2008/06/11 12:27:37 UTC

[PATCH] Simplify walk-entries code by not special-casing non-recursive case

The attached patch simplifies code that walks a WC, by always using the
"walk_entries" function, and no longer special-casing the single-file
(or non-recursive directory) cases. The "walk_entries" function can
handle those cases itself.

This patch is NOT FINISHED: two merge tests fail, presumably due to some
intentional difference between the single-item case and the recursive
cases. I'll investigate and fix if there are no objections.

Does anyone see a flaw in this approach?

Thanks,
- Julian



Re: [PATCH] Simplify walk-entries code by not special-casing non-recursive case

Posted by "Rui, Guo" <ti...@mail.ustc.edu.cn>.
I would like to merge this to my branch, once it is ready. Great job.

Rui, Guo

On Thu, Jun 12, 2008 at 03:55:21AM -0400, Karl Fogel wrote:
> Julian Foad <ju...@btopenworld.com> writes:
> > The attached patch simplifies code that walks a WC, by always using the
> > "walk_entries" function, and no longer special-casing the single-file
> > (or non-recursive directory) cases. The "walk_entries" function can
> > handle those cases itself.
> >
> > This patch is NOT FINISHED: two merge tests fail, presumably due to some
> > intentional difference between the single-item case and the recursive
> > cases. I'll investigate and fix if there are no objections.
> >
> > Does anyone see a flaw in this approach?
> 
> Not only do I not see a flaw in this approach, I see a big improvement
> to our code...
> 
> -K
> 
> > Simplify code that walks a WC by always using the "walk_entries" function,
> > removing the special case processing of the single-file or non-recursive
> > cases. The "walk_entries" function can handle those cases itself.
> >
> > There is no functional change.
> >
> > In one case, the behaviour was intentionally slightly different when
> > processing a single target, and in this case the special behaviour is
> > now implemented as an option in the generic code path.
> >
> > ### FAIL:  merge_tests.py 3 20
> >
> > * subversion/libsvn_client/info.c
> >   (crawl_entries): Don't special-case a single file.
> >
> > * subversion/libsvn_client/merge.c
> >   (get_mergeinfo_paths): Don't special-case a single file.
> >
> > * subversion/libsvn_client/prop_commands.c
> >   (propset_walk_baton): Add an "ignore_illegal_targets" flag.
> >   (propset_walk_cb): Obey the "ignore_illegal_targets" flag.
> >   (svn_client_propset3): Don't special case a single file; instead, just
> >     set the "ignore_illegal_targets" flag appropriately.
> >   (svn_client__get_prop_from_wc): Don't special-case depth=empty.
> >   (svn_client_proplist3): Don't special-case depth=empty.
> >
> > * subversion/libsvn_wc/adm_ops.c
> >   (svn_wc_resolved_conflict3): Don't special-case depth=empty.
> >
> > Index: subversion/libsvn_client/info.c
> > ===================================================================
> > --- subversion/libsvn_client/info.c	(revision 31702)
> > +++ subversion/libsvn_client/info.c	(working copy)
> > @@ -257,35 +257,20 @@ crawl_entries(const char *wcpath,
> >                apr_pool_t *pool)
> >  {
> >    svn_wc_adm_access_t *adm_access;
> > -  const svn_wc_entry_t *entry;
> >    int adm_lock_level = SVN_WC__LEVELS_TO_LOCK_FROM_DEPTH(depth);
> > +  struct found_entry_baton fe_baton;
> >  
> >    SVN_ERR(svn_wc_adm_probe_open3(&adm_access, NULL, wcpath, FALSE, 
> >                                   adm_lock_level, ctx->cancel_func, 
> >                                   ctx->cancel_baton, pool));
> > -  SVN_ERR(svn_wc__entry_versioned(&entry, wcpath, adm_access, FALSE, pool));
> >  
> > -
> > -  if (entry->kind == svn_node_file)
> > -    {
> > -      if (SVN_WC__CL_MATCH(changelist_hash, entry))
> > -        {
> > -          svn_info_t *info;
> > -          SVN_ERR(build_info_from_entry(&info, entry, pool));
> > -          return receiver(receiver_baton, wcpath, info, pool);
> > -        }
> > -    }
> > -  else if (entry->kind == svn_node_dir)
> > -    {
> > -      struct found_entry_baton fe_baton;
> > -      fe_baton.changelist_hash = changelist_hash;
> > -      fe_baton.receiver = receiver;
> > -      fe_baton.receiver_baton = receiver_baton;
> > -      SVN_ERR(svn_wc_walk_entries3(wcpath, adm_access,
> > -                                   &entry_walk_callbacks, &fe_baton,
> > -                                   depth, FALSE, ctx->cancel_func,
> > -                                   ctx->cancel_baton, pool));
> > -    }
> > +  fe_baton.changelist_hash = changelist_hash;
> > +  fe_baton.receiver = receiver;
> > +  fe_baton.receiver_baton = receiver_baton;
> > +  SVN_ERR(svn_wc_walk_entries3(wcpath, adm_access,
> > +                               &entry_walk_callbacks, &fe_baton,
> > +                               depth, FALSE, ctx->cancel_func,
> > +                               ctx->cancel_baton, pool));
> >  
> >    return SVN_NO_ERROR;
> >  }
> > Index: subversion/libsvn_client/merge.c
> > ===================================================================
> > --- subversion/libsvn_client/merge.c	(revision 31702)
> > +++ subversion/libsvn_client/merge.c	(working copy)
> > @@ -3392,15 +3392,11 @@ get_mergeinfo_paths(apr_array_header_t *
> >    /* Cover cases 1), 2), 6), and 7) by walking the WC to get all paths which
> >       have mergeinfo and/or are switched or are absent from disk or is the
> >       target of the merge. */
> > -  if (entry->kind == svn_node_file)
> > -    SVN_ERR(walk_callbacks.found_entry(merge_cmd_baton->target, entry, &wb,
> > -                                       pool));
> > -  else
> > -    SVN_ERR(svn_wc_walk_entries3(merge_cmd_baton->target, adm_access,
> > -                                 &walk_callbacks, &wb, depth, TRUE,
> > -                                 merge_cmd_baton->ctx->cancel_func,
> > -                                 merge_cmd_baton->ctx->cancel_baton,
> > -                                 pool));
> > +  SVN_ERR(svn_wc_walk_entries3(merge_cmd_baton->target, adm_access,
> > +                               &walk_callbacks, &wb, depth, TRUE,
> > +                               merge_cmd_baton->ctx->cancel_func,
> > +                               merge_cmd_baton->ctx->cancel_baton,
> > +                               pool));
> >  
> >    /* CHILDREN_WITH_MERGEINFO must be in depth first order, but
> >       svn_wc_walk_entries3() relies on svn_wc_entries_read() which means the
> > Index: subversion/libsvn_client/prop_commands.c
> > ===================================================================
> > --- subversion/libsvn_client/prop_commands.c	(revision 31702)
> > +++ subversion/libsvn_client/prop_commands.c	(working copy)
> > @@ -86,6 +86,8 @@ struct propset_walk_baton
> >    const svn_string_t *propval;  /* The value to set. */
> >    svn_wc_adm_access_t *base_access;  /* Access for the tree being walked. */
> >    svn_boolean_t force;  /* True iff force was passed. */
> > +  svn_boolean_t ignore_illegal_targets;  /* Don't report failures to set
> > +                                            svn:executable on a dir, etc. */
> >    apr_hash_t *changelist_hash;  /* Keys are changelists to filter on. */
> >    svn_wc_notify_func2_t notify_func;
> >    void *notify_baton;
> > @@ -130,9 +132,11 @@ propset_walk_cb(const char *path,
> >                           path, adm_access, wb->force, pool);
> >    if (err)
> >      {
> > -      if (err->apr_err != SVN_ERR_ILLEGAL_TARGET)
> > +      if (err->apr_err == SVN_ERR_ILLEGAL_TARGET
> > +          && wb->ignore_illegal_targets)
> > +        svn_error_clear(err);
> > +      else
> >          return err;
> > -      svn_error_clear(err);
> >      }
> >  
> >    /* Notify we updated a property value */
> > @@ -388,6 +392,9 @@ svn_client_propset3(svn_commit_info_t **
> >        int adm_lock_level = SVN_WC__LEVELS_TO_LOCK_FROM_DEPTH(depth);
> >        const svn_wc_entry_t *entry;
> >        apr_hash_t *changelist_hash = NULL;
> > +      static const svn_wc_entry_callbacks2_t walk_callbacks
> > +        = { propset_walk_cb, svn_client__default_walker_error_handler };
> > +      struct propset_walk_baton wb;
> >  
> >        if (changelists && changelists->nelts)
> >          SVN_ERR(svn_hash_from_cstring_keys(&changelist_hash, 
> > @@ -399,28 +406,19 @@ svn_client_propset3(svn_commit_info_t **
> >        SVN_ERR(svn_wc__entry_versioned(&entry, target, adm_access, 
> >                                        FALSE, pool));
> >  
> > -      if (depth >= svn_depth_files && entry->kind == svn_node_dir)
> > -        {
> > -          static const svn_wc_entry_callbacks2_t walk_callbacks
> > -            = { propset_walk_cb, svn_client__default_walker_error_handler };
> > -          struct propset_walk_baton wb;
> > -
> > -          wb.base_access = adm_access;
> > -          wb.propname = propname;
> > -          wb.propval = propval;
> > -          wb.force = skip_checks;
> > -          wb.changelist_hash = changelist_hash;
> > -          wb.notify_func = ctx->notify_func2;
> > -          wb.notify_baton = ctx->notify_baton2;
> > -          SVN_ERR(svn_wc_walk_entries3(target, adm_access, &walk_callbacks, 
> > -                                       &wb, depth, FALSE, ctx->cancel_func, 
> > -                                       ctx->cancel_baton, pool));
> > -        }
> > -      else if (SVN_WC__CL_MATCH(changelist_hash, entry))
> > -        {
> > -          SVN_ERR(svn_wc_prop_set2(propname, propval, target,
> > -                                   adm_access, skip_checks, pool));
> > -        }
> > +      wb.base_access = adm_access;
> > +      wb.propname = propname;
> > +      wb.propval = propval;
> > +      wb.force = skip_checks;
> > +      wb.ignore_illegal_targets =
> > +        (depth >= svn_depth_files && entry->kind == svn_node_dir);
> > +      wb.changelist_hash = changelist_hash;
> > +      wb.notify_func = ctx->notify_func2;
> > +      wb.notify_baton = ctx->notify_baton2;
> > +      SVN_ERR(svn_wc_walk_entries3(target, adm_access, &walk_callbacks,
> > +                                   &wb, depth, FALSE, ctx->cancel_func,
> > +                                   ctx->cancel_baton, pool));
> > +
> >        SVN_ERR(svn_wc_adm_close(adm_access));
> >      }
> >  
> > @@ -842,17 +840,10 @@ svn_client__get_prop_from_wc(apr_hash_t 
> >    wb.props = props;
> >  
> >    /* Fetch the property, recursively or for a single resource. */
> > -  if (depth >= svn_depth_files && entry->kind == svn_node_dir)
> > -    {
> > -      SVN_ERR(svn_wc_walk_entries3(target, adm_access,
> > -                                   &walk_callbacks, &wb, depth, FALSE,
> > -                                   ctx->cancel_func, ctx->cancel_baton,
> > -                                   pool));
> > -    }
> > -  else if (SVN_WC__CL_MATCH(changelist_hash, entry))
> > -    {
> > -      SVN_ERR(propget_walk_cb(target, entry, &wb, pool));
> > -    }
> > +  SVN_ERR(svn_wc_walk_entries3(target, adm_access,
> > +                               &walk_callbacks, &wb, depth, FALSE,
> > +                               ctx->cancel_func, ctx->cancel_baton,
> > +                               pool));
> >  
> >    return SVN_NO_ERROR;
> >  }
> > @@ -1236,15 +1227,15 @@ svn_client_proplist3(const char *path_or
> >      {
> >        svn_boolean_t pristine;
> >        int levels_to_lock = SVN_WC__LEVELS_TO_LOCK_FROM_DEPTH(depth);
> > -      const svn_wc_entry_t *entry;
> >        apr_hash_t *changelist_hash = NULL;
> > +      static const svn_wc_entry_callbacks2_t walk_callbacks
> > +        = { proplist_walk_cb, svn_client__default_walker_error_handler };
> > +      struct proplist_walk_baton wb;
> >  
> >        SVN_ERR(svn_wc_adm_probe_open3(&adm_access, NULL, path_or_url,
> >                                       FALSE, levels_to_lock,
> >                                       ctx->cancel_func, ctx->cancel_baton,
> >                                       pool));
> > -      SVN_ERR(svn_wc__entry_versioned(&entry, path_or_url, adm_access, 
> > -                                      FALSE, pool));
> >  
> >        if ((revision->kind == svn_opt_revision_committed)
> >            || (revision->kind == svn_opt_revision_base))
> > @@ -1260,34 +1251,16 @@ svn_client_proplist3(const char *path_or
> >          SVN_ERR(svn_hash_from_cstring_keys(&changelist_hash, 
> >                                             changelists, pool));
> >  
> > -      /* Fetch, recursively or not. */
> > -      if (depth >= svn_depth_files && (entry->kind == svn_node_dir))
> > -        {
> > -          static const svn_wc_entry_callbacks2_t walk_callbacks
> > -            = { proplist_walk_cb, svn_client__default_walker_error_handler };
> > -          struct proplist_walk_baton wb;
> > -
> > -          wb.base_access = adm_access;
> > -          wb.pristine = pristine;
> > -          wb.changelist_hash = changelist_hash;
> > -          wb.receiver = receiver;
> > -          wb.receiver_baton = receiver_baton;
> > -
> > -          SVN_ERR(svn_wc_walk_entries3(path_or_url, adm_access,
> > -                                       &walk_callbacks, &wb, depth, FALSE,
> > -                                       ctx->cancel_func, ctx->cancel_baton,
> > -                                       pool));
> > -        }
> > -      else if (SVN_WC__CL_MATCH(changelist_hash, entry))
> > -        {
> > -          apr_hash_t *hash;
> > -
> > -          SVN_ERR(pristine_or_working_props(&hash, path_or_url, adm_access,
> > -                                            pristine, pool));
> > -          SVN_ERR(call_receiver(path_or_url, hash, 
> > -                                receiver, receiver_baton, pool));
> > -
> > -        }
> > +      /* Fetch the properties. */
> > +      wb.base_access = adm_access;
> > +      wb.pristine = pristine;
> > +      wb.changelist_hash = changelist_hash;
> > +      wb.receiver = receiver;
> > +      wb.receiver_baton = receiver_baton;
> > +      SVN_ERR(svn_wc_walk_entries3(path_or_url, adm_access,
> > +                                   &walk_callbacks, &wb, depth, FALSE,
> > +                                   ctx->cancel_func, ctx->cancel_baton,
> > +                                   pool));
> >  
> >        SVN_ERR(svn_wc_adm_close(adm_access));
> >      }
> > Index: subversion/libsvn_wc/adm_ops.c
> > ===================================================================
> > --- subversion/libsvn_wc/adm_ops.c	(revision 31702)
> > +++ subversion/libsvn_wc/adm_ops.c	(working copy)
> > @@ -2909,20 +2909,9 @@ svn_wc_resolved_conflict3(const char *pa
> >    baton->notify_baton = notify_baton;
> >    baton->conflict_choice = conflict_choice;
> >  
> > -  if (depth == svn_depth_empty)
> > -    {
> > -      const svn_wc_entry_t *entry;
> > -      SVN_ERR(svn_wc__entry_versioned(&entry, path, adm_access, FALSE, pool));
> > -
> > -      SVN_ERR(resolve_found_entry_callback(path, entry, baton, pool));
> > -    }
> > -  else
> > -    {
> > -      SVN_ERR(svn_wc_walk_entries3(path, adm_access,
> > -                                   &resolve_walk_callbacks, baton, depth,
> > -                                   FALSE, cancel_func, cancel_baton, pool));
> > -
> > -    }
> > +  SVN_ERR(svn_wc_walk_entries3(path, adm_access,
> > +                               &resolve_walk_callbacks, baton, depth,
> > +                               FALSE, cancel_func, cancel_baton, pool));
> >  
> >    return SVN_NO_ERROR;
> >  }
> >
> > ---------------------------------------------------------------------
> > To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
> > For additional commands, e-mail: dev-help@subversion.tigris.org
> 
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
> For additional commands, e-mail: dev-help@subversion.tigris.org
> 

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org

Re: [PATCH] Simplify walk-entries code by not special-casing non-recursive case

Posted by Julian Foad <ju...@btopenworld.com>.
On Thu, 2008-06-12 at 10:43 +0200, Erik Huelsmann wrote:
> On 6/12/08, Karl Fogel <kf...@red-bean.com> wrote:
> > Julian Foad <ju...@btopenworld.com> writes:
> > > The attached patch simplifies code that walks a WC, by always using the
> > > "walk_entries" function, and no longer special-casing the single-file
> > > (or non-recursive directory) cases. The "walk_entries" function can
> > > handle those cases itself.
> > >
> > > This patch is NOT FINISHED: two merge tests fail, presumably due to some
> > > intentional difference between the single-item case and the recursive
> > > cases. I'll investigate and fix if there are no objections.
> > >
> > > Does anyone see a flaw in this approach?
> >
> > Not only do I not see a flaw in this approach, I see a big improvement
> > to our code...
> 
> I had the same reaction, but didn't understand the diff (reading it
> from work). So, I didn't send my reaction. But a very big +1 to the
> concept.

Thanks.

Looking into the test failures, this reveals inconsistencies in the way
svn, especially property commands, treats items scheduled for deletion.
The patch changes some of this behaviour towards more consistency, but
this needs discussion.

I'll have to come back to that issue in a separate discussion and leave
that part of the patch aside for now. I might be able to apply the other
parts of it.

- Julian



---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org

Re: [PATCH] Simplify walk-entries code by not special-casing non-recursive case

Posted by Erik Huelsmann <eh...@gmail.com>.
On 6/12/08, Karl Fogel <kf...@red-bean.com> wrote:
> Julian Foad <ju...@btopenworld.com> writes:
> > The attached patch simplifies code that walks a WC, by always using the
> > "walk_entries" function, and no longer special-casing the single-file
> > (or non-recursive directory) cases. The "walk_entries" function can
> > handle those cases itself.
> >
> > This patch is NOT FINISHED: two merge tests fail, presumably due to some
> > intentional difference between the single-item case and the recursive
> > cases. I'll investigate and fix if there are no objections.
> >
> > Does anyone see a flaw in this approach?
>
> Not only do I not see a flaw in this approach, I see a big improvement
> to our code...

I had the same reaction, but didn't understand the diff (reading it
from work). So, I didn't send my reaction. But a very big +1 to the
concept.

Bye,

Erik.

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org

Re: [PATCH] Simplify walk-entries code by not special-casing non-recursive case

Posted by Karl Fogel <kf...@red-bean.com>.
Julian Foad <ju...@btopenworld.com> writes:
> The attached patch simplifies code that walks a WC, by always using the
> "walk_entries" function, and no longer special-casing the single-file
> (or non-recursive directory) cases. The "walk_entries" function can
> handle those cases itself.
>
> This patch is NOT FINISHED: two merge tests fail, presumably due to some
> intentional difference between the single-item case and the recursive
> cases. I'll investigate and fix if there are no objections.
>
> Does anyone see a flaw in this approach?

Not only do I not see a flaw in this approach, I see a big improvement
to our code...

-K

> Simplify code that walks a WC by always using the "walk_entries" function,
> removing the special case processing of the single-file or non-recursive
> cases. The "walk_entries" function can handle those cases itself.
>
> There is no functional change.
>
> In one case, the behaviour was intentionally slightly different when
> processing a single target, and in this case the special behaviour is
> now implemented as an option in the generic code path.
>
> ### FAIL:  merge_tests.py 3 20
>
> * subversion/libsvn_client/info.c
>   (crawl_entries): Don't special-case a single file.
>
> * subversion/libsvn_client/merge.c
>   (get_mergeinfo_paths): Don't special-case a single file.
>
> * subversion/libsvn_client/prop_commands.c
>   (propset_walk_baton): Add an "ignore_illegal_targets" flag.
>   (propset_walk_cb): Obey the "ignore_illegal_targets" flag.
>   (svn_client_propset3): Don't special case a single file; instead, just
>     set the "ignore_illegal_targets" flag appropriately.
>   (svn_client__get_prop_from_wc): Don't special-case depth=empty.
>   (svn_client_proplist3): Don't special-case depth=empty.
>
> * subversion/libsvn_wc/adm_ops.c
>   (svn_wc_resolved_conflict3): Don't special-case depth=empty.
>
> Index: subversion/libsvn_client/info.c
> ===================================================================
> --- subversion/libsvn_client/info.c	(revision 31702)
> +++ subversion/libsvn_client/info.c	(working copy)
> @@ -257,35 +257,20 @@ crawl_entries(const char *wcpath,
>                apr_pool_t *pool)
>  {
>    svn_wc_adm_access_t *adm_access;
> -  const svn_wc_entry_t *entry;
>    int adm_lock_level = SVN_WC__LEVELS_TO_LOCK_FROM_DEPTH(depth);
> +  struct found_entry_baton fe_baton;
>  
>    SVN_ERR(svn_wc_adm_probe_open3(&adm_access, NULL, wcpath, FALSE, 
>                                   adm_lock_level, ctx->cancel_func, 
>                                   ctx->cancel_baton, pool));
> -  SVN_ERR(svn_wc__entry_versioned(&entry, wcpath, adm_access, FALSE, pool));
>  
> -
> -  if (entry->kind == svn_node_file)
> -    {
> -      if (SVN_WC__CL_MATCH(changelist_hash, entry))
> -        {
> -          svn_info_t *info;
> -          SVN_ERR(build_info_from_entry(&info, entry, pool));
> -          return receiver(receiver_baton, wcpath, info, pool);
> -        }
> -    }
> -  else if (entry->kind == svn_node_dir)
> -    {
> -      struct found_entry_baton fe_baton;
> -      fe_baton.changelist_hash = changelist_hash;
> -      fe_baton.receiver = receiver;
> -      fe_baton.receiver_baton = receiver_baton;
> -      SVN_ERR(svn_wc_walk_entries3(wcpath, adm_access,
> -                                   &entry_walk_callbacks, &fe_baton,
> -                                   depth, FALSE, ctx->cancel_func,
> -                                   ctx->cancel_baton, pool));
> -    }
> +  fe_baton.changelist_hash = changelist_hash;
> +  fe_baton.receiver = receiver;
> +  fe_baton.receiver_baton = receiver_baton;
> +  SVN_ERR(svn_wc_walk_entries3(wcpath, adm_access,
> +                               &entry_walk_callbacks, &fe_baton,
> +                               depth, FALSE, ctx->cancel_func,
> +                               ctx->cancel_baton, pool));
>  
>    return SVN_NO_ERROR;
>  }
> Index: subversion/libsvn_client/merge.c
> ===================================================================
> --- subversion/libsvn_client/merge.c	(revision 31702)
> +++ subversion/libsvn_client/merge.c	(working copy)
> @@ -3392,15 +3392,11 @@ get_mergeinfo_paths(apr_array_header_t *
>    /* Cover cases 1), 2), 6), and 7) by walking the WC to get all paths which
>       have mergeinfo and/or are switched or are absent from disk or is the
>       target of the merge. */
> -  if (entry->kind == svn_node_file)
> -    SVN_ERR(walk_callbacks.found_entry(merge_cmd_baton->target, entry, &wb,
> -                                       pool));
> -  else
> -    SVN_ERR(svn_wc_walk_entries3(merge_cmd_baton->target, adm_access,
> -                                 &walk_callbacks, &wb, depth, TRUE,
> -                                 merge_cmd_baton->ctx->cancel_func,
> -                                 merge_cmd_baton->ctx->cancel_baton,
> -                                 pool));
> +  SVN_ERR(svn_wc_walk_entries3(merge_cmd_baton->target, adm_access,
> +                               &walk_callbacks, &wb, depth, TRUE,
> +                               merge_cmd_baton->ctx->cancel_func,
> +                               merge_cmd_baton->ctx->cancel_baton,
> +                               pool));
>  
>    /* CHILDREN_WITH_MERGEINFO must be in depth first order, but
>       svn_wc_walk_entries3() relies on svn_wc_entries_read() which means the
> Index: subversion/libsvn_client/prop_commands.c
> ===================================================================
> --- subversion/libsvn_client/prop_commands.c	(revision 31702)
> +++ subversion/libsvn_client/prop_commands.c	(working copy)
> @@ -86,6 +86,8 @@ struct propset_walk_baton
>    const svn_string_t *propval;  /* The value to set. */
>    svn_wc_adm_access_t *base_access;  /* Access for the tree being walked. */
>    svn_boolean_t force;  /* True iff force was passed. */
> +  svn_boolean_t ignore_illegal_targets;  /* Don't report failures to set
> +                                            svn:executable on a dir, etc. */
>    apr_hash_t *changelist_hash;  /* Keys are changelists to filter on. */
>    svn_wc_notify_func2_t notify_func;
>    void *notify_baton;
> @@ -130,9 +132,11 @@ propset_walk_cb(const char *path,
>                           path, adm_access, wb->force, pool);
>    if (err)
>      {
> -      if (err->apr_err != SVN_ERR_ILLEGAL_TARGET)
> +      if (err->apr_err == SVN_ERR_ILLEGAL_TARGET
> +          && wb->ignore_illegal_targets)
> +        svn_error_clear(err);
> +      else
>          return err;
> -      svn_error_clear(err);
>      }
>  
>    /* Notify we updated a property value */
> @@ -388,6 +392,9 @@ svn_client_propset3(svn_commit_info_t **
>        int adm_lock_level = SVN_WC__LEVELS_TO_LOCK_FROM_DEPTH(depth);
>        const svn_wc_entry_t *entry;
>        apr_hash_t *changelist_hash = NULL;
> +      static const svn_wc_entry_callbacks2_t walk_callbacks
> +        = { propset_walk_cb, svn_client__default_walker_error_handler };
> +      struct propset_walk_baton wb;
>  
>        if (changelists && changelists->nelts)
>          SVN_ERR(svn_hash_from_cstring_keys(&changelist_hash, 
> @@ -399,28 +406,19 @@ svn_client_propset3(svn_commit_info_t **
>        SVN_ERR(svn_wc__entry_versioned(&entry, target, adm_access, 
>                                        FALSE, pool));
>  
> -      if (depth >= svn_depth_files && entry->kind == svn_node_dir)
> -        {
> -          static const svn_wc_entry_callbacks2_t walk_callbacks
> -            = { propset_walk_cb, svn_client__default_walker_error_handler };
> -          struct propset_walk_baton wb;
> -
> -          wb.base_access = adm_access;
> -          wb.propname = propname;
> -          wb.propval = propval;
> -          wb.force = skip_checks;
> -          wb.changelist_hash = changelist_hash;
> -          wb.notify_func = ctx->notify_func2;
> -          wb.notify_baton = ctx->notify_baton2;
> -          SVN_ERR(svn_wc_walk_entries3(target, adm_access, &walk_callbacks, 
> -                                       &wb, depth, FALSE, ctx->cancel_func, 
> -                                       ctx->cancel_baton, pool));
> -        }
> -      else if (SVN_WC__CL_MATCH(changelist_hash, entry))
> -        {
> -          SVN_ERR(svn_wc_prop_set2(propname, propval, target,
> -                                   adm_access, skip_checks, pool));
> -        }
> +      wb.base_access = adm_access;
> +      wb.propname = propname;
> +      wb.propval = propval;
> +      wb.force = skip_checks;
> +      wb.ignore_illegal_targets =
> +        (depth >= svn_depth_files && entry->kind == svn_node_dir);
> +      wb.changelist_hash = changelist_hash;
> +      wb.notify_func = ctx->notify_func2;
> +      wb.notify_baton = ctx->notify_baton2;
> +      SVN_ERR(svn_wc_walk_entries3(target, adm_access, &walk_callbacks,
> +                                   &wb, depth, FALSE, ctx->cancel_func,
> +                                   ctx->cancel_baton, pool));
> +
>        SVN_ERR(svn_wc_adm_close(adm_access));
>      }
>  
> @@ -842,17 +840,10 @@ svn_client__get_prop_from_wc(apr_hash_t 
>    wb.props = props;
>  
>    /* Fetch the property, recursively or for a single resource. */
> -  if (depth >= svn_depth_files && entry->kind == svn_node_dir)
> -    {
> -      SVN_ERR(svn_wc_walk_entries3(target, adm_access,
> -                                   &walk_callbacks, &wb, depth, FALSE,
> -                                   ctx->cancel_func, ctx->cancel_baton,
> -                                   pool));
> -    }
> -  else if (SVN_WC__CL_MATCH(changelist_hash, entry))
> -    {
> -      SVN_ERR(propget_walk_cb(target, entry, &wb, pool));
> -    }
> +  SVN_ERR(svn_wc_walk_entries3(target, adm_access,
> +                               &walk_callbacks, &wb, depth, FALSE,
> +                               ctx->cancel_func, ctx->cancel_baton,
> +                               pool));
>  
>    return SVN_NO_ERROR;
>  }
> @@ -1236,15 +1227,15 @@ svn_client_proplist3(const char *path_or
>      {
>        svn_boolean_t pristine;
>        int levels_to_lock = SVN_WC__LEVELS_TO_LOCK_FROM_DEPTH(depth);
> -      const svn_wc_entry_t *entry;
>        apr_hash_t *changelist_hash = NULL;
> +      static const svn_wc_entry_callbacks2_t walk_callbacks
> +        = { proplist_walk_cb, svn_client__default_walker_error_handler };
> +      struct proplist_walk_baton wb;
>  
>        SVN_ERR(svn_wc_adm_probe_open3(&adm_access, NULL, path_or_url,
>                                       FALSE, levels_to_lock,
>                                       ctx->cancel_func, ctx->cancel_baton,
>                                       pool));
> -      SVN_ERR(svn_wc__entry_versioned(&entry, path_or_url, adm_access, 
> -                                      FALSE, pool));
>  
>        if ((revision->kind == svn_opt_revision_committed)
>            || (revision->kind == svn_opt_revision_base))
> @@ -1260,34 +1251,16 @@ svn_client_proplist3(const char *path_or
>          SVN_ERR(svn_hash_from_cstring_keys(&changelist_hash, 
>                                             changelists, pool));
>  
> -      /* Fetch, recursively or not. */
> -      if (depth >= svn_depth_files && (entry->kind == svn_node_dir))
> -        {
> -          static const svn_wc_entry_callbacks2_t walk_callbacks
> -            = { proplist_walk_cb, svn_client__default_walker_error_handler };
> -          struct proplist_walk_baton wb;
> -
> -          wb.base_access = adm_access;
> -          wb.pristine = pristine;
> -          wb.changelist_hash = changelist_hash;
> -          wb.receiver = receiver;
> -          wb.receiver_baton = receiver_baton;
> -
> -          SVN_ERR(svn_wc_walk_entries3(path_or_url, adm_access,
> -                                       &walk_callbacks, &wb, depth, FALSE,
> -                                       ctx->cancel_func, ctx->cancel_baton,
> -                                       pool));
> -        }
> -      else if (SVN_WC__CL_MATCH(changelist_hash, entry))
> -        {
> -          apr_hash_t *hash;
> -
> -          SVN_ERR(pristine_or_working_props(&hash, path_or_url, adm_access,
> -                                            pristine, pool));
> -          SVN_ERR(call_receiver(path_or_url, hash, 
> -                                receiver, receiver_baton, pool));
> -
> -        }
> +      /* Fetch the properties. */
> +      wb.base_access = adm_access;
> +      wb.pristine = pristine;
> +      wb.changelist_hash = changelist_hash;
> +      wb.receiver = receiver;
> +      wb.receiver_baton = receiver_baton;
> +      SVN_ERR(svn_wc_walk_entries3(path_or_url, adm_access,
> +                                   &walk_callbacks, &wb, depth, FALSE,
> +                                   ctx->cancel_func, ctx->cancel_baton,
> +                                   pool));
>  
>        SVN_ERR(svn_wc_adm_close(adm_access));
>      }
> Index: subversion/libsvn_wc/adm_ops.c
> ===================================================================
> --- subversion/libsvn_wc/adm_ops.c	(revision 31702)
> +++ subversion/libsvn_wc/adm_ops.c	(working copy)
> @@ -2909,20 +2909,9 @@ svn_wc_resolved_conflict3(const char *pa
>    baton->notify_baton = notify_baton;
>    baton->conflict_choice = conflict_choice;
>  
> -  if (depth == svn_depth_empty)
> -    {
> -      const svn_wc_entry_t *entry;
> -      SVN_ERR(svn_wc__entry_versioned(&entry, path, adm_access, FALSE, pool));
> -
> -      SVN_ERR(resolve_found_entry_callback(path, entry, baton, pool));
> -    }
> -  else
> -    {
> -      SVN_ERR(svn_wc_walk_entries3(path, adm_access,
> -                                   &resolve_walk_callbacks, baton, depth,
> -                                   FALSE, cancel_func, cancel_baton, pool));
> -
> -    }
> +  SVN_ERR(svn_wc_walk_entries3(path, adm_access,
> +                               &resolve_walk_callbacks, baton, depth,
> +                               FALSE, cancel_func, cancel_baton, pool));
>  
>    return SVN_NO_ERROR;
>  }
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
> For additional commands, e-mail: dev-help@subversion.tigris.org

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org