Anyone up for discussing the two '###'-comments in this patch's log message? I think this patch is necessary, but feel free to discuss it to smithereens. I'd say +1 for both '###'-comments, for consistency with dir externals.
Omit pegged file externals (that have a specific rev) from recursive commit. Explicit targets are not skipped.
Raison: file externals with a specific revision number are supposed to stay at
that revision. When they are locally modified, they will usually just abort
the entire commit with out-of-date-ness. Even if the peg revision is currently
up-to-date, a commit to it causes the file external to be bumped to HEAD in
the WC, and subsequent updates complain about the file external's state:
[[[
svn: warning: W155035: The specified path has an unexpected status
[...]
svn: E205011: Failure occurred processing one or more externals definitions
]]]
### It is debatable if even explicit targets should be skipped.
### It is also debatable if we should also skip *un*pegged file externals from
recursion, just like we do with dir externals.
* subversion/libsvn_client/commit_util.c
(harvest_committables):
New parameter IS_EXPLICIT_TARGET. Omit pegged file externals from commit.
Recursions pass IS_EXPLICIT_TARGET as FALSE.
(svn_client__harvest_committables, harvest_copy_committables):
Pass IS_EXPLICIT_TARGET as TRUE.
Index: subversion/libsvn_client/commit_util.c
===================================================================
--- subversion/libsvn_client/commit_util.c (revision 1162797)
+++ subversion/libsvn_client/commit_util.c (working copy)
@@ -410,6 +410,9 @@ bail_on_tree_conflicted_ancestor(svn_wc_
when harvesting committables; that is, don't add a path to
COMMITTABLES unless it's a member of one of those changelists.
+ IS_EXPLICIT_TARGET should always be passed as TRUE (except when
+ harvest_committables() calls itself in recursion).
+
If CANCEL_FUNC is non-null, call it with CANCEL_BATON to see
if the user has cancelled the operation.
@@ -428,6 +431,7 @@ harvest_committables(svn_wc_context_t *w
apr_hash_t *changelists,
svn_boolean_t skip_files,
svn_boolean_t skip_dirs,
+ svn_boolean_t is_explicit_target,
svn_client__check_url_kind_t check_url_func,
void *check_url_baton,
svn_cancel_func_t cancel_func,
@@ -543,12 +547,34 @@ harvest_committables(svn_wc_context_t *w
svn_dirent_local_style(local_abspath, scratch_pool));
}
- /* ### in need of comment */
- if (copy_mode
- && is_update_root
+ /* Handle file externals.
+ * (IS_UPDATE_ROOT is more generally defined, but at the moment this
+ * condition matches only file externals.) */
+ if (is_update_root
&& db_kind == svn_node_file)
{
- return SVN_NO_ERROR;
+
+ /* Don't copy files that svn:externals brought into the WC. */
+ if (copy_mode)
+ return SVN_NO_ERROR;
+
+ /* Check if it's a *pegged* file external, and skip it if so.
+ * They usually cause an out-of-date error and abort the entire
+ * commit. But, never skip an explicit target. */
+ if (! is_explicit_target)
+ {
+ svn_node_kind_t xkind;
+ svn_revnum_t xrev;
+
+ SVN_ERR(svn_wc__read_external_info(&xkind, NULL, NULL, NULL,
+ &xrev,
+ wc_ctx, NULL,
+ local_abspath,
+ TRUE,
+ scratch_pool, scratch_pool));
+ if (xkind == svn_node_file && xrev != SVN_INVALID_REVNUM)
+ return SVN_NO_ERROR;
+ }
}
/* If NODE is in our changelist, then examine it for conflicts. We
@@ -841,6 +867,7 @@ harvest_committables(svn_wc_context_t *w
changelists,
(depth < svn_depth_files),
(depth < svn_depth_immediates),
+ FALSE, /* is_explicit_target */
check_url_func, check_url_baton,
cancel_func, cancel_baton,
notify_func, notify_baton,
@@ -1125,6 +1152,7 @@ svn_client__harvest_committables(svn_cli
FALSE /* COPY_MODE_ROOT */,
depth, just_locked, changelist_hash,
FALSE, FALSE,
+ TRUE /* IS_EXPLICIT_TARGET */,
check_url_func, check_url_baton,
ctx->cancel_func, ctx->cancel_baton,
ctx->notify_func2, ctx->notify_baton2,
@@ -1218,6 +1246,7 @@ harvest_copy_committables(void *baton, v
FALSE, /* JUST_LOCKED */
NULL,
FALSE, FALSE, /* skip files, dirs */
+ TRUE, /* IS_EXPLICIT_TARGET (don't care) */
btn->check_url_func,
btn->check_url_baton,
btn->ctx->cancel_func,
signature.asc
Description: OpenPGP digital signature
