firemeteor_at_tigris.org writes:
> Log:
> On the issue-2843-dev branch.
>
> This commit solves on problem exposed by the new test case in r31764.
> We cannot report excluded branch to server when depth_is_sticky, otherwise the
> server will not send contents of that path and we will be in trouble.
> Moreover, we have to report delete_path() on it(when !report_everything),
> to ensure that the server will send us full content.
I think this explanation maybe belongs as a comment in the code?
> * subversion/include/svn_wc.h,
> subversion/libsvn_wc/adm_crawler.c
> (svn_wc_crawl_revision4): New API, with new parameter honor_depth_exclude.
> (svn_wc_crawl_revision3): Deprecated.
>
> * subversion/libsvn_client/switch.c
> (svn_client__switch_internal):
> * subversion/libsvn_client/status.c
> (svn_client_status3):
> * subversion/libsvn_client/diff.c
> (diff_repos_wc):
> * subversion/libsvn_client/update.c
> (svn_client__update_internal): Adjust all callers of the deprecated API.
>
> --- branches/issue-2843-dev/subversion/include/svn_wc.h (r31764)
> +++ branches/issue-2843-dev/subversion/include/svn_wc.h (r31765)
> @@ -3256,6 +3256,11 @@ svn_wc_process_committed(const char *pat
> * course. If @a depth is @c svn_depth_unknown, then just use
> * @c svn_depth_infinity, which in practice means depth of @a path.
> *
> + * Iff @a honor_depth_exclude is TRUE, the crawler will report the excluded
> + * path and thus prevent the server from pushing update on it. Don't set this
> + * flag if you wish to pull in excluded path. @c svn_depth_exclude flag on
> + * the @a path will never be honored. This enable explicitly pull in target.
> + *
I didn't understand the last two sentences above; could you try writing
them with more detail? (Your doc string changes are very good in
general, by the way, I'm just not bothering to comment about all the
ones I like.)
> --- branches/issue-2843-dev/subversion/libsvn_wc/adm_crawler.c (r31764)
> +++ branches/issue-2843-dev/subversion/libsvn_wc/adm_crawler.c (r31765)
> @@ -278,19 +279,32 @@ report_revisions_and_depths(svn_wc_adm_a
> continue;
> }
>
> - /* Report the excluded path, no matter whether report_everything flag is
> - set. Because the report_everything flag indicates that the server
> - will treate the wc as empty and thus push full content of the
> - files/subdirs. We just want to say no to this. */
> if (current_entry->depth == svn_depth_exclude)
> {
> - SVN_ERR(reporter->set_path(report_baton,
> - this_path,
> - SVN_INVALID_REVNUM,
> - svn_depth_exclude,
> - FALSE,
> - NULL,
> - iterpool));
> + if (honor_depth_exclude)
> + {
> + /* Report the excluded path, no matter whether report_everything
> + flag is set. Because the report_everything flag indicates
> + that the server will treate the wc as empty and thus push
> + full content of the files/subdirs. We just want to say no to
> + this. */
> + SVN_ERR(reporter->set_path(report_baton,
> + this_path,
> + SVN_INVALID_REVNUM,
> + svn_depth_exclude,
> + FALSE,
> + NULL,
> + iterpool));
> + }
Maybe the last sentence of that comment would be clearer if it said "But
we want to prevent the server from pushing the full content of this_path
at us."
> + else
> + {
> + /* We want to pull in the excluded target, report it as deleted
> + when needed, and server will response properly. */
> + if (! report_everything)
> + SVN_ERR(reporter->delete_path(report_baton,
> + this_path, iterpool));
> + }
> +
s/response/respond/
But, let's fix the grammar a bit:
/* We want to pull in the excluded target. So, report it as deleted,
and server will respond properly. */
The code itself makes sense to me, so I'm concentrating on the comments
that will explain the code to people who are less familiar with it.
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe_at_subversion.tigris.org
For additional commands, e-mail: dev-help_at_subversion.tigris.org
Received on 2008-06-18 06:17:57 CEST