You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@subversion.apache.org by Karl Fogel <kf...@red-bean.com> on 2008/06/15 00:57:56 UTC
Re: svn commit: r31713 - in branches/issue-2843-dev/subversion: include libsvn_wc svn
firemeteor@tigris.org writes:
> Log:
> On issue-2843-dev branch.
>
> Make 'svn update' honors the svn_depth_exclude flag. It works for clean wc
> now. And all existing test suites remained working. Hoever, the crawler has
> not been updated yet. So, filtering modification in the excluded branch will
> not work now.
I didn't understand the last sentence above. Do you mean that the
svn_depth_exclude conditional branch will not be reached?
> The svn_wc_entry() and svn_wc_entries_read() now filter out excluded
> item when show_hidden is FALSE.
>
> [in subversion/libsvn_wc]
>
> * svn_wc.h, entries.c, lock.c
> (svn_wc_entry, svn_wc_entries_read, svn_wc_walk_entries3): Document the
> new filter behavior.
> (read_entries, prune_deleted): Implement the documented behavior.
Oops, don't mix files together unless they contain exactly the same
symbols (function names, variable names, etc). I know svn_wc.h has all
the symbols, but then they are split across the two .c files.
> * crop.c
> (svn_wc_crop_tree): Remove the defensive check against excluded target.
>
> * ambient_depth_filter_editor.c
> (make_dir_baton): Update the filter logic.
>
> * adm_ops.c
> (tweak_entries, svn_wc__do_update_cleanup): Properly tweak excluded entry.
>
> * update_editor.c
> (complete_directory): Clear the exclude flag properly when needed.
>
> [in subversion/svn]
> * main.c (main): Accept value exclude for --set-depth.
Same thing about using full relative path.
> --- branches/issue-2843-dev/subversion/libsvn_wc/adm_ops.c (r31712)
> +++ branches/issue-2843-dev/subversion/libsvn_wc/adm_ops.c (r31713)
> @@ -129,7 +129,8 @@ tweak_entries(svn_wc_adm_access_t *dirpa
> /* If a file, or deleted or absent dir, then tweak the entry
> but don't recurse. */
> if ((current_entry->kind == svn_node_file)
> - || (current_entry->deleted || current_entry->absent))
> + || (current_entry->deleted || current_entry->absent
> + || current_entry->depth == svn_depth_exclude))
> {
> if (! excluded)
> SVN_ERR(svn_wc__tweak_entry(entries, name,
Want to adjust the comment too?
> --- branches/issue-2843-dev/subversion/libsvn_wc/ambient_depth_filter_editor.c (r31712)
> +++ branches/issue-2843-dev/subversion/libsvn_wc/ambient_depth_filter_editor.c (r31713)
> @@ -133,19 +133,32 @@ make_dir_baton(struct dir_baton **d_p,
> if (path)
> d->path = svn_path_join(d->path, path, pool);
>
> - if (pb
> - && (pb->ambient_depth == svn_depth_empty
> - || pb->ambient_depth == svn_depth_files))
> + if (pb)
> {
> - /* This is not a depth upgrade, and the parent directory is
> - depth==empty or depth==files. So if the parent doesn't
> - already have an entry for the new dir, then the parent
> - doesn't want the new dir at all, thus we should initialize
> - it with ambiently_excluded=TRUE. */
> const svn_wc_entry_t *entry;
> + svn_boolean_t exclude;
>
> - SVN_ERR(svn_wc_entry(&entry, d->path, eb->adm_access, FALSE, pool));
> - if (! entry)
> + SVN_ERR(svn_wc_entry(&entry, d->path, eb->adm_access, TRUE, pool));
> + if (pb->ambient_depth == svn_depth_empty
> + || pb->ambient_depth == svn_depth_files)
> + {
> + /* This is not a depth upgrade, and the parent directory is
> + depth==empty or depth==files. So if the parent doesn't
> + already have an entry for the new dir, then the parent
> + doesn't want the new dir at all, thus we should initialize
> + it with ambiently_excluded=TRUE. */
> + exclude = entry == NULL;
Suggest parens for clarity: exclude = (entry == NULL);
> + }
> + else
> + {
> + /* If the parent expect all children by default, only exclude
> + it whenever it is explicitly marked as exclude. The
> + svn_depth_unknown means that: 1) pb is the anchor; 2) there
> + is an non-null target, for which we are preparing the baton.
> + This enables explicitly pull in the target. */
> + exclude = entry && entry->depth == svn_depth_exclude;
Same.
> --- branches/issue-2843-dev/subversion/libsvn_wc/crop.c (r31712)
> +++ branches/issue-2843-dev/subversion/libsvn_wc/crop.c (r31713)
> @@ -198,11 +198,6 @@ svn_wc_crop_tree(svn_wc_adm_access_t *an
> if (!entry || entry->kind != svn_node_dir)
> return SVN_NO_ERROR;
>
> - /* Defensive test against excluded target. We may need this until we filter
> - them out in svn_wc_entry(). */
> - if (entry->depth == svn_depth_exclude)
> - return SVN_NO_ERROR;
That answers my question from previous review :-).
> --- branches/issue-2843-dev/subversion/libsvn_wc/update_editor.c (r31712)
> +++ branches/issue-2843-dev/subversion/libsvn_wc/update_editor.c (r31713)
> @@ -473,7 +473,23 @@ complete_directory(struct edit_baton *eb
> /* If this is the root directory and there is a target, we can't
> mark this directory complete. */
> if (is_root_dir && *eb->target)
This isn't your code, but I see a lot of "if (*eb->target)" in this
file, which makes me think that when there is no target, eb->target is
"" rather than NULL. Which is fine, but we should document that in
struct edit_baton!
> @@ -541,21 +557,27 @@ complete_directory(struct edit_baton *eb
> {
> const char *child_path = svn_path_join(path, name, subpool);
>
> - if ((svn_wc__adm_missing(adm_access, child_path))
> - && (! current_entry->absent)
> - && (current_entry->schedule != svn_wc_schedule_add))
> + if (current_entry->depth == svn_depth_exclude)
> {
> - svn_wc__entry_remove(entries, name);
> - if (eb->notify_func)
> - {
> - svn_wc_notify_t *notify
> - = svn_wc_create_notify(child_path,
> - svn_wc_notify_update_delete,
> - subpool);
> - notify->kind = current_entry->kind;
> - (* eb->notify_func)(eb->notify_baton, notify, subpool);
> - }
> - }
> + /* Clear the exclude flag if it is pulled in again. */
> + if (eb->depth_is_sticky
> + && eb->requested_depth >= svn_depth_immediates)
> + current_entry->depth = svn_depth_infinity;
The 'current_entry' here is just the parent's entry for the subdir,
right? And therefore the only reason we're using svn_depth_infinity is
so that that subdir entry won't write out an explicit depth, and the
subdir itself will know its own actual depth? (Just checking my own
understanding.)
> --- branches/issue-2843-dev/subversion/svn/main.c (r31712)
> +++ branches/issue-2843-dev/subversion/svn/main.c (r31713)
> @@ -1326,13 +1326,13 @@ main(int argc, const char *argv[])
> _("Error converting depth "
> "from locale to UTF8")), pool, "svn: ");
> opt_state.set_depth = svn_depth_from_word(utf8_opt_arg);
> - if (opt_state.set_depth == svn_depth_unknown
> - || opt_state.set_depth == svn_depth_exclude)
> + /* svn_depth_exclude is okay for --set-depth. */
> + if (opt_state.set_depth == svn_depth_unknown)
> {
> return svn_cmdline_handle_exit_error
> (svn_error_createf(SVN_ERR_CL_ARG_PARSING_ERROR, NULL,
> _("'%s' is not a valid depth; try "
> - "'empty', 'files', 'immediates', "
> + "'exclude', 'empty', 'files', 'immediates', "
> "or 'infinity'"),
> utf8_opt_arg), pool, "svn: ");
> }
Yay! :-)
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: svn commit: r31713 - in branches/issue-2843-dev/subversion: include libsvn_wc svn
Posted by Karl Fogel <kf...@red-bean.com>.
"Rui, Guo" <ti...@mail.ustc.edu.cn> writes:
>> This isn't your code, but I see a lot of "if (*eb->target)" in this
>> file, which makes me think that when there is no target, eb->target is
>> "" rather than NULL. Which is fine, but we should document that in
>> struct edit_baton!
>
> It should have been ensured by svn_wc_adm_open_anchor(), I think. The document
> about anchor in notes/ directory has some words on it, if I recall correctly.
I've documented it now, see r31916.
>> The 'current_entry' here is just the parent's entry for the subdir,
>> right? And therefore the only reason we're using svn_depth_infinity is
>> so that that subdir entry won't write out an explicit depth, and the
>> subdir itself will know its own actual depth? (Just checking my own
>> understanding.)
>
> Yes. The depth field was only stored in this_dir entry. And I extend it
> that the subdir entry may have an 'exclude' depth value indicating that it is
> cropped out. However, the old depth values are still handled in the usual
> way.(in other words, restrictive values other 'exclude' is not allowed in
> subdir entries. May I explicitly check this in read_entry()?)
Yes, in fact I think that would be a good idea! :-)
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: svn commit: r31713 - in branches/issue-2843-dev/subversion: include libsvn_wc svn
Posted by "Rui, Guo" <ti...@mail.ustc.edu.cn>.
On Sat, Jun 14, 2008 at 08:57:56PM -0400, Karl Fogel wrote:
> firemeteor@tigris.org writes:
> > Log:
> > On issue-2843-dev branch.
> >
> > Make 'svn update' honors the svn_depth_exclude flag. It works for clean wc
> > now. And all existing test suites remained working. Hoever, the crawler has
> > not been updated yet. So, filtering modification in the excluded branch will
> > not work now.
>
> I didn't understand the last sentence above. Do you mean that the
> svn_depth_exclude conditional branch will not be reached?
I did not report the excluded path to repository, and thus the server will
send modification in the excluded path. That's all. The modification will be
blocked by ambient_depth_filter_editor though. But that is another story.
>
> > The svn_wc_entry() and svn_wc_entries_read() now filter out excluded
> > item when show_hidden is FALSE.
> >
> > [in subversion/libsvn_wc]
> >
> > * svn_wc.h, entries.c, lock.c
> > (svn_wc_entry, svn_wc_entries_read, svn_wc_walk_entries3): Document the
> > new filter behavior.
> > (read_entries, prune_deleted): Implement the documented behavior.
>
> Oops, don't mix files together unless they contain exactly the same
> symbols (function names, variable names, etc). I know svn_wc.h has all
> the symbols, but then they are split across the two .c files.
Sorry. And thanks for update the log message for me.
PS: propedit on log message seems tend to timeout at the first try. I didn't
notice this on update and commit. Does it takes more time for the server to
response such a request?
>
> > --- branches/issue-2843-dev/subversion/libsvn_wc/adm_ops.c (r31712)
> > +++ branches/issue-2843-dev/subversion/libsvn_wc/adm_ops.c (r31713)
> > @@ -129,7 +129,8 @@ tweak_entries(svn_wc_adm_access_t *dirpa
> > /* If a file, or deleted or absent dir, then tweak the entry
> > but don't recurse. */
> > if ((current_entry->kind == svn_node_file)
> > - || (current_entry->deleted || current_entry->absent))
> > + || (current_entry->deleted || current_entry->absent
> > + || current_entry->depth == svn_depth_exclude))
> > {
> > if (! excluded)
> > SVN_ERR(svn_wc__tweak_entry(entries, name,
>
> Want to adjust the comment too?
Done.
>
> Suggest parens for clarity: exclude = (entry == NULL);
>
> > + }
> > + else
> > + {
> > + /* If the parent expect all children by default, only exclude
> > + it whenever it is explicitly marked as exclude. The
> > + svn_depth_unknown means that: 1) pb is the anchor; 2) there
> > + is an non-null target, for which we are preparing the baton.
> > + This enables explicitly pull in the target. */
> > + exclude = entry && entry->depth == svn_depth_exclude;
>
> Same.
Done.
>
> > --- branches/issue-2843-dev/subversion/libsvn_wc/update_editor.c (r31712)
> > +++ branches/issue-2843-dev/subversion/libsvn_wc/update_editor.c (r31713)
> > @@ -473,7 +473,23 @@ complete_directory(struct edit_baton *eb
> > /* If this is the root directory and there is a target, we can't
> > mark this directory complete. */
> > if (is_root_dir && *eb->target)
>
> This isn't your code, but I see a lot of "if (*eb->target)" in this
> file, which makes me think that when there is no target, eb->target is
> "" rather than NULL. Which is fine, but we should document that in
> struct edit_baton!
It should have been ensured by svn_wc_adm_open_anchor(), I think. The document
about anchor in notes/ directory has some words on it, if I recall correctly.
>
> > @@ -541,21 +557,27 @@ complete_directory(struct edit_baton *eb
> > {
> > const char *child_path = svn_path_join(path, name, subpool);
> >
> > - if ((svn_wc__adm_missing(adm_access, child_path))
> > - && (! current_entry->absent)
> > - && (current_entry->schedule != svn_wc_schedule_add))
> > + if (current_entry->depth == svn_depth_exclude)
> > {
> > - svn_wc__entry_remove(entries, name);
> > - if (eb->notify_func)
> > - {
> > - svn_wc_notify_t *notify
> > - = svn_wc_create_notify(child_path,
> > - svn_wc_notify_update_delete,
> > - subpool);
> > - notify->kind = current_entry->kind;
> > - (* eb->notify_func)(eb->notify_baton, notify, subpool);
> > - }
> > - }
> > + /* Clear the exclude flag if it is pulled in again. */
> > + if (eb->depth_is_sticky
> > + && eb->requested_depth >= svn_depth_immediates)
> > + current_entry->depth = svn_depth_infinity;
>
> The 'current_entry' here is just the parent's entry for the subdir,
> right? And therefore the only reason we're using svn_depth_infinity is
> so that that subdir entry won't write out an explicit depth, and the
> subdir itself will know its own actual depth? (Just checking my own
> understanding.)
Yes. The depth field was only stored in this_dir entry. And I extend it
that the subdir entry may have an 'exclude' depth value indicating that it is
cropped out. However, the old depth values are still handled in the usual
way.(in other words, restrictive values other 'exclude' is not allowed in
subdir entries. May I explicitly check this in read_entry()?)
Rui
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org