[svn.haxx.se] · SVN Dev · SVN Users · SVN Org · TSVN Dev · TSVN Users · Subclipse Dev · Subclipse Users · this month's index

Re: svn commit: r32298 - branches/issue-2843-dev/subversion/libsvn_wc

From: Karl Fogel <kfogel_at_red-bean.com>
Date: Sun, 17 Aug 2008 18:49:52 -0400

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.

> --- branches/issue-2843-dev/subversion/libsvn_wc/crop.c
> +++ branches/issue-2843-dev/subversion/libsvn_wc/crop.c
> @@ -220,43 +219,80 @@ svn_wc_crop_tree(svn_wc_adm_access_t *an
> /* Crop the target itself if we are requested to. */
> if (depth == svn_depth_exclude)
> {
> - svn_boolean_t is_root;
> + svn_boolean_t switched, entry_in_repos;
> + const svn_wc_entry_t *parent_entry = NULL;
> + svn_wc_adm_access_t *p_access;
> +
> + /* Safeguard on bad target. */
> + if (*full_path == 0)
> + return svn_error_createf
> + (SVN_ERR_UNSUPPORTED_FEATURE, NULL,
> + _("Cannot exclude current directory."));
> +
> + if (svn_dirent_is_root(full_path, strlen(full_path)))
> + return svn_error_createf
> + (SVN_ERR_UNSUPPORTED_FEATURE, NULL,
> + _("Cannot exclude root directory."));

Looks good so far.

> + /* 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);

> /* If the target entry is just added without history, it does not exist
> in the repos (in which case we won't exclude it). */
> - svn_boolean_t entry_in_repos
> + entry_in_repos
> = ! ((entry->schedule == svn_wc_schedule_add
> || entry->schedule == svn_wc_schedule_replace)
> && ! entry->copied);

I had to read this conditional twice before I understood :-).

> - /* 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?)

-Karl

---------------------------------------------------------------------
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-18 00:50:11 CEST

This is an archived mail posted to the Subversion Dev mailing list.

This site is subject to the Apache Privacy Policy and the Apache Public Forum Archive Policy.