On Sat, Jun 14, 2008 at 08:57:33PM -0400, Karl Fogel wrote:
> firemeteor_at_tigris.org writes:
> > Log:
> > On issue-2843-dev branch
> >
> > According to Karl: 1) report 'D' on those locally modified items is Okay. And
> > 2) crop a subtree to svn_depth_empty should not take the root away. This
> > purpose should be covered by a dedicated enum, such as svn_depth_excluded.
> >
> > * subversion/tests/cmdline/depth_tests.py
> > (fold_tree_with_unversioned_modified_items): Tune comments.
> > (test_list): The above test is no longer XFAIL.
> >
> > * subversion/libsvn_wc/crop.c
> > (svn_wc_crop_tree): don't crop the root on svn_depth_empty.
>
> The log message is accurate, but perhaps a bit incomplete? In
> svn_wc_crop_tree(), you've done more than just make it not crop root in
> the svn_depth_empty case: you've also added preliminary code to handle
> svn_depth_exclude.
I've updated the log message.
> > - if (parent_entry && parent_entry->depth <= svn_depth_files)
> > + if (parent_entry && parent_entry->depth > svn_depth_files)
> > {
> > - /* Crop the target with the subtree altogether if the parent
> > - does not want sub-directories. */
> > - SVN_ERR(svn_wc_adm_retrieve(&dir_access, anchor, full_path, pool));
> > - SVN_ERR_IGNORE_LOCAL_MOD
> > - (svn_wc_remove_from_revision_control(dir_access,
> > - SVN_WC_ENTRY_THIS_DIR,
> > - TRUE, /* destroy */
> > - FALSE, /* instant error */
> > - cancel_func,
> > - cancel_baton,
> > - pool));
> > + /* TODO: mark the target as excluded, since the parent require it by
> > + default. */
> > + }
>
> So, the comment above means "mark the target as excluded in the parent's
> entries list", right? And I wasn't sure what the "since the parent
> requires it by default" part meant -- but I see you changed "since" to
> "if" in a later commit, which clarifies things a bit: it means, if the
> parent's depth is deep enough to still keep the empty subdir, right?
> And that's what the conditional change
>
> "if (parent_entry && parent_entry->depth > svn_depth_files)"
>
> above was about too, I guess.
Yes! I just reused the old comment without double check on it. Sorry for the
confusion.
>
> By the way, sometimes you say "XXX", and sometimes you say "TODO" :-).
>
> May I suggest what we did on the sparse-dirs branch? Use a special TODO
> form, like "/* TODO(#2843): ...", so that we can always find all of the
> TODO spots, even after merging the branch.
Hehe. Vim just recognize the both. I'll switch to the special form in a new
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-15 07:20:10 CEST