firemeteor_at_tigris.org writes:
> --- branches/issue-2843-dev/subversion/libsvn_client/copy.c
> +++ branches/issue-2843-dev/subversion/libsvn_client/copy.c
> @@ -1638,13 +1638,28 @@ repos_to_wc_copy(const apr_array_header_
>
> svn_pool_clear(iterpool);
>
> - SVN_ERR(svn_wc_entry(&ent, pair->dst, adm_access, FALSE, iterpool));
> - if (ent && (ent->kind != svn_node_dir) &&
> - (ent->schedule != svn_wc_schedule_delete))
> - return svn_error_createf
> - (SVN_ERR_WC_OBSTRUCTED_UPDATE, NULL,
> - _("Entry for '%s' exists (though the working file is missing)"),
> - svn_path_local_style(pair->dst, pool));
> + SVN_ERR(svn_wc_entry(&ent, pair->dst, adm_access, TRUE, iterpool));
> + if (ent)
> + {
> + /* TODO(#2843): Rework the error report. Maybe we can simplify the
> + condition. Currently, the first is about hidden items and the
> + second is for missing items. */
> + if (ent->depth == svn_depth_exclude
> + || ent->absent)
> + {
> + return svn_error_createf
> + (SVN_ERR_ENTRY_EXISTS,
> + NULL, _("'%s' is already under version control"),
> + svn_path_local_style(pair->dst, pool));
> + }
> + else if ((ent->kind != svn_node_dir) &&
> + (ent->schedule != svn_wc_schedule_delete)
> + && ! ent->deleted)
> + return svn_error_createf
> + (SVN_ERR_WC_OBSTRUCTED_UPDATE, NULL,
> + _("Entry for '%s' exists (though the working file is missing)"),
> + svn_path_local_style(pair->dst, pool));
> + }
> }
I think the detailed error reporting here is good, and I'm not sure
there's any way to simplify the conditional (nor is there any need to).
> --- branches/issue-2843-dev/subversion/libsvn_wc/copy.c
> +++ branches/issue-2843-dev/subversion/libsvn_wc/copy.c
> @@ -810,6 +813,21 @@ svn_wc_copy2(const char *src_path,
> _("Cannot copy to '%s' as it is scheduled for deletion"),
> svn_path_local_style(svn_wc_adm_access_path(dst_parent), pool));
>
> + /* TODO(#2843): Rework the erroer report. */
> + /* Check if the copy target is missing or hidden and thus not exist on the
> + disk, before actually doing the file copy. */
> + target_path = svn_path_join(dst_path, dst_basename, pool);
> + SVN_ERR(svn_wc_entry(&target_entry, target_path, dst_parent, TRUE, pool));
> + if (target_entry
> + && ((target_entry->depth == svn_depth_exclude)
> + || target_entry->absent))
> + {
> + return svn_error_createf
> + (SVN_ERR_ENTRY_EXISTS,
> + NULL, _("'%s' is already under version control"),
> + svn_path_local_style(target_path, pool));
> + }
> +
> SVN_ERR(svn_io_check_path(src_path, &src_kind, pool));
>
> if (src_kind == svn_node_file)
Hmmm. I expected the opposite condition:
if (target_entry
&& (! ((target_entry->depth == svn_depth_exclude)
|| target_entry->absent)))
That is: "if there's an entry already here, and it's *not* excluded or
absent, then let's error".
But your conditional seems to say: "if there's entry already here *and*
it is excluded or absent, then let's error (otherwise, let's not error)"
Isn't that the opposite of what we want?
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe_at_subversion.tigris.org
For additional commands, e-mail: dev-help_at_subversion.tigris.org
Received on 2008-08-18 03:44:31 CEST