"Rui, Guo" <timmyguo_at_mail.ustc.edu.cn> writes:
>> > --- 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.)
>
> The check does not make things worse. But maybe we just don't need the check
> here? If we do not set up exclude flag for new added items. Whether the
> code branch handles copy is important.
Ah. A good way to write that is:
We're here because we are commiting an added tree.
Check this code again after we've defined the behavior of
cropping a newly-added tree.
Checking for excluded items here does not make things worse, but
possibly we don't need to check at all, since we don't set up an
exclude flag for newly-added items anyway.
>> > --- 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.
>
> Some day I did an experiment on this. The version I used just copied the
> exclude flag to the new tree. And after commit, the server side will have a
> full source tree in the target position. I think this behavior is just fine.
> Maybe it is a good idea to maintain this behavior and make the code more
> robust?
Agreed. This question is similar to the longer discussion we had about
doing 'svn cp non-depth-infinity-tree new-tree', and I think the
decision here is the same as our decision there.
>> > --- 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?
>
> svn_wc_walk_entries3() is just a general purpose wc walking framework. I don't
> think it is a good idea to preclude anything from the framework, unless that
> is definitely unnecessary. The client can choose whether they want hidden
> entries. If they choose to see hidden entries, they should prepared to handle
> excluded target. As far as I know, there is only one call to this function
> with show_hidden set to TRUE -- in the merge logic.
Okay, that makes sense to me.
-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-21 22:48:54 CEST