On Sun, Aug 17, 2008 at 06:49:52PM -0400, Karl Fogel wrote:
> firemeteor_at_tigris.org writes:
> > Log:
> > On the issue-2843-dev branch.
> >
> > * subversion/libsvn_wc/crop.c
> > (svn_wc_crop_tree): Explicitly prohibit excluding switched path.
> > (crop_children): Move a TODO item to svn_wc_crop_tree().
>
> This log message doesn't mention the change to adm_crawler.c.
>
> > --- branches/issue-2843-dev/subversion/libsvn_wc/adm_crawler.c
> > +++ branches/issue-2843-dev/subversion/libsvn_wc/adm_crawler.c
> > @@ -288,6 +288,10 @@ report_revisions_and_depths(svn_wc_adm_a
> > that the server will treate the wc as empty and thus push
> > full content of the files/subdirs. But we want to prevent the
> > server from pushing the full content of this_path at us. */
> > +
> > + /* The server does not support link_path report on excluded
> > + path. We explicitly prohibit this situation in
> > + svn_wc_crop_tree(). */
> > SVN_ERR(reporter->set_path(report_baton,
> > this_path,
> > SVN_INVALID_REVNUM,
>
> This is the change not mentioned. It's only a comment, so it's no big
> deal, but to avoid confusion the log message should still mention it.
My fault. I must be sleepy when doing the commit. :-(
Fixed.
> > + /* This simulates the logic of svn_wc_is_wc_root(). */
> > + {
> > + const char *bname, *pname;
> > + svn_error_t *err = NULL;
> > + svn_path_split(full_path, &pname, &bname, pool);
> > + SVN_ERR(svn_wc__adm_retrieve_internal(&p_access, anchor, pname,
> > + pool));
> > + if (! p_access)
> > + err = svn_wc_adm_probe_open3(&p_access, NULL, pname, FALSE, 0,
> > + NULL, NULL, pool);
> > +
> > + if (! err)
> > + err = svn_wc_entry(&parent_entry, pname, p_access, FALSE, pool);
> > +
> > + if (err)
> > + svn_error_clear(err);
> > +
> > + switched
> > + = parent_entry && strcmp(entry->url,
> > + svn_path_url_add_component
> > + (parent_entry->url, bname, pool));
> > +
> > + /* The server simply do not accept excluded link_path and thus
> > + switched path can not be excluede. Just completely prohibit this
> > + situation. */
> > + if (switched)
> > + return svn_error_createf
> > + (SVN_ERR_UNSUPPORTED_FEATURE, NULL,
> > + _("Cannot crop '%s': it is a switched path."),
> > + svn_path_local_style(full_path, pool));
> > + }
>
> When I first read the comment at the top...
>
> "/* This simulates the logic of svn_wc_is_wc_root(). */"
>
> ...I thought that you must be about to test whether the path is a wc
> root because a wc root cannot be excluded (right?). But it turns out
> you're simulating the logic of svn_wc_is_wc_root() just to find out
> whether the path is switched (because if it is switched, then we cannot
> exclude it -- a switched directory behaves "like" a wc root in this
> way).
>
> So would it be simpler to just use svn_wc__path_switched()? This is its
> doc string, from include/private/svn_wc_private.h:
>
> /** Given a @a wcpath with its accompanying @a entry, set @a
> * switched to true if @a wcpath is switched, otherwise set @a
> * switched to false. If @a entry is an incomplete entry obtained
> * from @a wcpath's parent return @c SVN_ERR_ENTRY_MISSING_URL.
> * All allocations are done in @a pool.
> *
> * @since New in 1.5.
> */
> svn_error_t *
> svn_wc__path_switched(const char *wcpath,
> svn_boolean_t *switched,
> const svn_wc_entry_t *entry,
> apr_pool_t *pool);
Thanks for pointing this out for me. I just did not aware of this function.
But even if I call svn_wc__path_switched() instead, I have to prepare
parent_entry for later use. So it does not save many code, right?
> > - /* TODO(#2843) Switched path will be treated as root and bypass the
> > - exclude flag setup below. Beause the server simply do not accept
> > - excluded link_path. But without the flag it is not an excluded path
> > - at all, maybe just prohibit this situation? */
> > - svn_wc_is_wc_root(&is_root, full_path, anchor, pool);
> > - if ((! is_root) && entry_in_repos)
> > + /* Mark the target as excluded, if the parent requires it by
> > + default. */
> > + if (parent_entry && entry_in_repos
> > + && (parent_entry->depth > svn_depth_files))
> > {
>
> Should it be
>
> (parent_entry->depth >= svn_depth_files)
>
> ? (Or do we already know for sure that target is a directory?)
We just do not setup an exclude flag if the parent only requires files, since
directories is excluded by default.
Rui
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe_at_subversion.tigris.org
For additional commands, e-mail: dev-help_at_subversion.tigris.org
Received on 2008-08-28 17:07:57 CEST