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/07/07 19:32:56 UTC

Re: svn commit: r31986 - in branches/issue-2843-dev/subversion: libsvn_client libsvn_wc

firemeteor@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@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org

Re: svn commit: r31986 - in branches/issue-2843-dev/subversion: libsvn_client libsvn_wc

Posted by "Rui, Guo" <ti...@mail.ustc.edu.cn>.
On Mon, Jul 07, 2008 at 03:32:56PM -0400, Karl Fogel wrote:
> > --- 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;
> 
I'm still not sure of this. If you go further, there are some code dealing
with sparse directories and absent files. I really have no idea how the
mergeinfo is maintainted. Maybe I should go through the document first.

Rui

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org

Re: svn commit: r31986 - in branches/issue-2843-dev/subversion: libsvn_client libsvn_wc

Posted by Karl Fogel <kf...@red-bean.com>.
"Rui, Guo" <ti...@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@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org

Re: svn commit: r31986 - in branches/issue-2843-dev/subversion: libsvn_client libsvn_wc

Posted by "Rui, Guo" <ti...@mail.ustc.edu.cn>.
I was not very confident to make these modifications, since I'm not familar
with them. It is good to hear your positive reply.

On Mon, Jul 07, 2008 at 03:32:56PM -0400, Karl Fogel wrote:
> firemeteor@tigris.org writes:
> > --- 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;
Thanks for pointing it out. I do not understand the code very well.

> 
> > --- 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.

> 
> > @@ -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.
Okay.

> > --- 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?

> > --- 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?
Yep. 

> 
> > --- 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.

Rui

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org

Re: svn commit: r31986 - in branches/issue-2843-dev/subversion: libsvn_client libsvn_wc

Posted by Karl Fogel <kf...@red-bean.com>.
"Rui, Guo" <ti...@mail.ustc.edu.cn> writes:
>> > 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?
>
> After a second check on this, I find that this function is only used to copy
> items added without history (In other words, those not yet in the repos). So,
> this code path is irrelevant to the exclude flag.

Oh, heh, then ignore my mail that said: "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." :-).

-K

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org

Re: svn commit: r31986 - in branches/issue-2843-dev/subversion: libsvn_client libsvn_wc

Posted by "Rui, Guo" <ti...@mail.ustc.edu.cn>.
On Tue, Jul 08, 2008 at 11:32:41AM +0800, Rui, Guo wrote:
> > > --- 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?
> 
After a second check on this, I find that this function is only used to copy
items added without history (In other words, those not yet in the repos). So,
this code path is irrelevant to the exclude flag.

Rui

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org