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

Re: svn commit: r31713 - in branches/issue-2843-dev/subversion: include libsvn_wc svn

From: Karl Fogel <kfogel_at_red-bean.com>
Date: Sat, 14 Jun 2008 20:57:56 -0400

firemeteor_at_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_at_subversion.tigris.org
For additional commands, e-mail: dev-help_at_subversion.tigris.org
Received on 2008-06-15 02:58:39 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.