firemeteor_at_tigris.org writes:
> Check those calls to svn_wc_entry(), svn_wc_entries_read() and
> svn_wc_walk_entries3() with show_hidden set to TRUE. The caller should be
> prepared to handle excluded items. This only covers part of the potential
> modificatoin positions.
>
> * subversion/libsvn_wc/relocate.c
> (svn_wc_reloacate3):
> * subversion/libsvn_wc/crop.c
> (svn_wc_crop_tree):
> * subversion/libsvn_wc/props.c
> (svn_wc_prop_list, svn_wc_prop_get, modified_props):
> * subversion/libsvn_wc/entries.c
> (svn_wc_walk_entries3):
> * subversion/libsvn_wc/copy.c
> (copy_added_dir_administratively):
> * subversion/libsvn_wc/adm_ops.c
> (process_committed_internal, svn_wc_delete3, revert_entry):
> * subversion/libsvn_client/merge.c
> (get_mergeinfo_walk_cb):
> * subversion/libsvn_client/commit_util.c
> (harvest_committables): Handles svn_depth_exclude or Add TODO for later
> checking.
>
> --- branches/issue-2843-dev/subversion/libsvn_client/commit_util.c (r31985)
> +++ branches/issue-2843-dev/subversion/libsvn_client/commit_util.c (r31986)
> @@ -566,6 +566,11 @@ harvest_committables(apr_hash_t *committ
> continue;
>
> this_entry = val;
> +
> + /* Skip the excluded item. */
> + if (this_entry->depth == svn_depth_exclude)
> + continue;
> +
> name_uri = svn_path_uri_encode(name, loop_pool);
>
> full_path = svn_path_join(path, name, loop_pool);
Looks good to me; clearly, we should be skipping excluded items when
we're harvesting committables.
> --- branches/issue-2843-dev/subversion/libsvn_client/merge.c (r31985)
> +++ branches/issue-2843-dev/subversion/libsvn_client/merge.c (r31986)
> @@ -3260,6 +3260,8 @@ get_mergeinfo_walk_cb(const char *path,
> !svn_path_compare_paths(path, wb->merge_target_path);
> const char *parent_path = svn_path_dirname(path, pool);
>
> + /* TODO(#2843) How to deal with a excluded item on merge? */
> +
> /* We're going to receive dirents twice; we want to ignore the
> first one (where it's a child of a parent dir), and only use
> the second one (where we're looking at THIS_DIR). The exception
I think we should handle that the same way we would if the item were
deleted... which we can see a bit later in the code:
/* Ignore the entry if it does not exist at the time of interest. */
if (entry->deleted)
return SVN_NO_ERROR;
> --- branches/issue-2843-dev/subversion/libsvn_wc/adm_ops.c (r31985)
> +++ branches/issue-2843-dev/subversion/libsvn_wc/adm_ops.c (r31986)
> @@ -538,6 +538,14 @@ process_committed_internal(int *log_numb
> if (! strcmp(name, SVN_WC_ENTRY_THIS_DIR))
> continue;
>
> + /* TODO(#2843)
> + We come to this branch since we are commiting an added tree.
> + Check this again after the behavior of croping an newly added
> + tree is definined. Anyway, it will be safer to check for excluded
> + items here. */
> + if (current_entry->depth == svn_depth_exclude)
> + continue;
> +
> /* Create child path by telescoping the main path. */
> this_path = svn_path_join(path, name, subpool);
"cropping" and "defined", and be careful of line length in the comment.
What exactly do you mean by "safer" above? (I don't think the code is
wrong, I'm just trying to understand the comment.)
> @@ -1212,6 +1220,8 @@ svn_wc_delete3(const char *path,
> /* The deleted state is only available in the entry in parent's
> entries file */
> SVN_ERR(svn_wc_adm_retrieve(&parent_access, adm_access, parent, pool));
> + /* We don't need to check for excluded item, since we won't fall into
> + this path in that case. */
> SVN_ERR(svn_wc_entries_read(&entries, parent_access, TRUE, pool));
> entry_in_parent = apr_hash_get(entries, base_name, APR_HASH_KEY_STRING);
> was_deleted = entry_in_parent ? entry_in_parent->deleted : FALSE;
When you say "path", you're talking about a code path, not a filesystem
path, right? Might want to clarify that.
> @@ -2034,6 +2044,8 @@ revert_entry(svn_depth_t *depth,
> "directory; please try again from the "
> "parent directory"));
>
> + /* We don't need to check for excluded item, since we won't fall
> + into this path in that case. */
> SVN_ERR(svn_wc_entries_read(&entries, parent_access, TRUE, pool));
> parents_entry = apr_hash_get(entries, basey, APR_HASH_KEY_STRING);
> if (parents_entry)
Same here.
> --- branches/issue-2843-dev/subversion/libsvn_wc/copy.c (r31985)
> +++ branches/issue-2843-dev/subversion/libsvn_wc/copy.c (r31986)
> @@ -223,6 +223,7 @@ copy_added_dir_administratively(const ch
> SVN_ERR(svn_wc_entry(&entry, src_fullpath, src_child_dir_access,
> TRUE, subpool));
>
> + /* TODO(#2843) Should we skip the excluded src? */
> /* Recurse on directories; add files; ignore the rest. */
> if (this_entry.filetype == APR_DIR)
> {
Hmmm. I'm not sure. If someone copies a tree T that contains excluded
subdir S, then when whey commit, will the new tree T_NEW (in the
repository) also contain S_NEW? Once we answer this question, I think
it will be clear how other things should behave.
> @@ -558,6 +559,7 @@ post_copy_cleanup(svn_wc_adm_access_t *a
>
> svn_pool_clear(subpool);
>
> + /* TODO(#2843) Check if we need to handle exclude here. Possibly not. */
> apr_hash_this(hi, &key, NULL, &val);
> entry = val;
> kind = entry->kind;
I commented on this one later, in my response to r31987.
> --- branches/issue-2843-dev/subversion/libsvn_wc/crop.c (r31985)
> +++ branches/issue-2843-dev/subversion/libsvn_wc/crop.c (r31986)
> @@ -209,6 +209,10 @@ svn_wc_crop_tree(svn_wc_adm_access_t *an
> if (!entry || entry->kind != svn_node_dir)
> return SVN_NO_ERROR;
>
> + /* TODO(#2843): Re-consider the behavior of cropping items with scheduled
> + add/delete. Maybe we don't need to setup the exclude flag when the taget
> + is just added without history. */
> +
> /* Crop the target itself if we are requested to. */
> if (depth == svn_depth_exclude)
> {
"target", not "taget"
I think I understand. We don't need to set up the exclude flag, we just
need to unversion the target tree, right?
> --- branches/issue-2843-dev/subversion/libsvn_wc/entries.c (r31985)
> +++ branches/issue-2843-dev/subversion/libsvn_wc/entries.c (r31986)
> @@ -3029,7 +3029,7 @@ svn_wc_walk_entries3(const char *path,
> svn_path_local_style(path, pool)),
> walk_baton, pool);
>
> - if (entry->kind == svn_node_file)
> + if (entry->kind == svn_node_file || entry->depth == svn_depth_exclude)
> return walk_callbacks->handle_error
> (path, walk_callbacks->found_entry(path, entry, walk_baton, pool),
> walk_baton, pool);
Should we ce balling walk_callbacks->found_entry() on the path at all,
in the svn_depth_exclude case? What would happen if we just did
nothing?
> --- branches/issue-2843-dev/subversion/libsvn_wc/props.c (r31985)
> +++ branches/issue-2843-dev/subversion/libsvn_wc/props.c (r31986)
> @@ -2212,8 +2212,9 @@ svn_wc_prop_list(apr_hash_t **props,
> SVN_ERR(svn_wc_entry(&entry, path, adm_access, TRUE, pool));
>
> /* if there is no entry, 'path' is not under version control and
> - therefore has no props */
> - if (! entry)
> + therefore has no props. And if the path is excluded, the props
> + are also gone. */
> + if (! entry || entry->depth == svn_depth_exclude)
> {
> *props = apr_hash_make(pool);
> return SVN_NO_ERROR;
*nod*
> @@ -2262,7 +2263,7 @@ svn_wc_prop_get(const svn_string_t **val
>
> SVN_ERR(svn_wc_entry(&entry, path, adm_access, TRUE, pool));
>
> - if (entry == NULL)
> + if (entry == NULL || entry->depth == svn_depth_exclude)
> {
> *value = NULL;
> return SVN_NO_ERROR;
Follows from previous, yes.
> @@ -2824,8 +2825,8 @@ modified_props(svn_boolean_t *modified_p
>
> SVN_ERR(svn_wc_entry(&entry, path, adm_access, TRUE, subpool));
>
> - /* If we have no entry, we can't have any prop mods. */
> - if (! entry)
> + /* If we have no entry or excluded entry, we can't have any prop mods. */
> + if (! entry || entry->depth == svn_depth_exclude)
> {
> *modified_p = FALSE;
> goto cleanup;
Likewise.
> --- branches/issue-2843-dev/subversion/libsvn_wc/relocate.c (r31985)
> +++ branches/issue-2843-dev/subversion/libsvn_wc/relocate.c (r31986)
> @@ -136,6 +136,7 @@ svn_wc_relocate3(const char *path,
> return SVN_NO_ERROR;
> }
>
> + /* TODO(#2843) Check the case of excluded path. */
> /* Relocate THIS_DIR first, in order to pre-validate the relocated URL
> of all of the other entries. This is technically cheating because
> it relies on knowledge of the libsvn_client implementation, but it
> @@ -163,7 +164,8 @@ svn_wc_relocate3(const char *path,
>
> if (recurse && (entry->kind == svn_node_dir)
> && (! entry->deleted || (entry->schedule == svn_wc_schedule_add))
> - && ! entry->absent)
> + && ! entry->absent
> + && (entry->depth != svn_depth_exclude))
> {
> svn_wc_adm_access_t *subdir_access;
> const char *subdir = svn_path_join(path, key, subpool);
Looks good.
-Karl
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe_at_subversion.tigris.org
For additional commands, e-mail: dev-help_at_subversion.tigris.org
Received on 2008-07-07 21:33:13 CEST