On Wed, Jun 18, 2008 at 12:17:35AM -0400, Karl Fogel wrote:
> 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?
Yes, just an abstraction to the code and comments.
> > --- 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.)
Well, I think it will be helpful to present some examples. Maybe you can help
coming up with a clearer explanation.
Lets' just take the greek tree as an example.
Prepare wc_dir first:
svn co url_to_repos wc_dir
svn up --set-depth exclude wc_dir/A/B/E
Now we have an excluded path wc_dir/A/B/E
Consider the two different commands:
svn up wc_dir/A/B
and
svn up wc_dir/A/B/E
The first simply update an existing path to HEAD revision and is constrained
by the ambient depth. However, in the second command, the 'explicitly pull in'
rule applies since the path E is not exist.
In both case, the 'honor_depth_exclude' flag is always setup since depth is
not sticky. However, the result is quite different. In general, we never
report the update target as excluded.
Is it clear enough?
> 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.
Done, waiting for commit.
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-06-18 16:42:28 CEST