Daniel Shahaf wrote on Fri, Feb 25, 2022 at 21:41:10 +0000:
> Daniel Shahaf wrote on Fri, Feb 25, 2022 at 15:18:44 +0000:
> > Nathan Hartman wrote on Thu, 24 Feb 2022 23:44 +00:00:
> > > On Thu, Feb 24, 2022 at 2:33 PM Mark Phippard <markp...@gmail.com> wrote:
> > >> Maybe merge should just refuse to run at all if it detects any
> > >> svn:needs-lock properties in the WC?
> > >
> > > Then it would never be able to run; I think you meant: refuse to run
> > > if we don't hold the lock on a file that has the svn:needs-lock
> > > property. However, it would be rather irritating if the refusal to run
> > > were caused by files unaffected by the merge.
> >
> > Or if the user deliberately did the merge without intending to commit it.
> >
> > How about making this situation a conflict? "local file not locked,
> > incoming edit upon merge". Then the file is not silently changed, but
> > the user can use --accept or «svn resolve» to make the changes anyway if
> > they know that's what they want.
>
> Is it just this?
Had a "goto fail;" bug there. Corrected version:
[[[
Index: subversion/include/svn_wc.h
===================================================================
--- subversion/include/svn_wc.h.orig
+++ subversion/include/svn_wc.h
@@ -1657,7 +1657,10 @@ typedef enum svn_wc_conflict_reason_t
/** Object is moved away. @since New in 1.8. */
svn_wc_conflict_reason_moved_away,
/** Object is moved here. @since New in 1.8. */
- svn_wc_conflict_reason_moved_here
+ svn_wc_conflict_reason_moved_here,
+ /** #SVN_PROP_NEEDS_LOCK was set and the file wasn't locked.
+ * @since New in 1.15. */
+ svn_wc_conflict_reason_not_locked
} svn_wc_conflict_reason_t;
Index: subversion/libsvn_client/conflicts.c
===================================================================
--- subversion/libsvn_client/conflicts.c.orig
+++ subversion/libsvn_client/conflicts.c
@@ -244,6 +244,7 @@ static const svn_token_map_t map_conflic
{ "unversioned", svn_wc_conflict_reason_unversioned },
{ "moved-away", svn_wc_conflict_reason_moved_away },
{ "moved-here", svn_wc_conflict_reason_moved_here },
+ { "not-locked", svn_wc_conflict_reason_not_locked },
{ NULL, 0 }
};
@@ -1327,6 +1328,11 @@ describe_local_file_node_change(const ch
}
break;
}
+ case svn_wc_conflict_reason_not_locked:
+ {
+ *description = _("hello world");
+ break;
+ }
}
return SVN_NO_ERROR;
@@ -1477,8 +1483,8 @@ describe_local_dir_node_change(const cha
scratch_pool));
}
}
- }
- break;
+ }
+ break;
case svn_wc_conflict_reason_moved_here:
{
const char *moved_from_abspath;
@@ -1552,6 +1558,10 @@ describe_local_dir_node_change(const cha
}
}
}
+ break;
+ case svn_wc_conflict_reason_not_locked:
+ *description = _("hello world");
+ break;
}
return SVN_NO_ERROR;
@@ -3169,6 +3179,9 @@ describe_local_none_node_change(const ch
*description = _("An item had been moved here in the working copy "
"at the time this conflict was recorded.");
break;
+ case svn_wc_conflict_reason_not_locked:
+ *description = _("hello world");
+ break;
}
return SVN_NO_ERROR;
Index: subversion/libsvn_wc/conflicts.c
===================================================================
--- subversion/libsvn_wc/conflicts.c.orig
+++ subversion/libsvn_wc/conflicts.c
@@ -520,6 +520,7 @@ static const svn_token_map_t reason_map[
{ "replaced", svn_wc_conflict_reason_replaced },
{ "moved-away", svn_wc_conflict_reason_moved_away },
{ "moved-here", svn_wc_conflict_reason_moved_here },
+ { "not-locked", svn_wc_conflict_reason_not_locked },
{ NULL }
};
Index: subversion/libsvn_wc/merge.c
===================================================================
--- subversion/libsvn_wc/merge.c.orig
+++ subversion/libsvn_wc/merge.c
@@ -24,6 +24,7 @@
#include "svn_wc.h"
#include "svn_diff.h"
#include "svn_dirent_uri.h"
+#include "svn_hash.h"
#include "svn_path.h"
#include "svn_pools.h"
#include "svn_props.h"
@@ -1142,6 +1143,32 @@ svn_wc__internal_merge(svn_skel_t **work
cancel_func, cancel_baton,
scratch_pool, scratch_pool));
+ {
+ svn_wc__db_lock_t *lock;
+ apr_hash_t *props;
+
+ /* TODO: short-circuit one of the two calls */
+ SVN_ERR(svn_wc__db_read_props(&props, db, target_abspath,
+ result_pool, scratch_pool));
+ SVN_ERR(svn_wc__db_lock_get(&lock, db, wri_abspath, target_abspath,
+ result_pool, scratch_pool));
+
+ if (svn_hash_gets(props, SVN_PROP_NEEDS_LOCK) && NULL == lock)
+ {
+ if (!*conflict_skel)
+ *conflict_skel = svn_wc__conflict_skel_create(result_pool);
+ SVN_ERR(svn_wc__conflict_skel_add_tree_conflict(*conflict_skel,
+ db,
+ wri_abspath,
+
svn_wc_conflict_reason_not_locked,
+
svn_wc_conflict_action_edit,
+ NULL, NULL,
+ result_pool,
scratch_pool));
+ }
+
+ /* ### TODO notification? */
+ }
+
SVN_ERR(merge_file_trivial(work_items, merge_outcome,
left_abspath, right_abspath,
target_abspath, detranslated_target_abspath,
Index: subversion/libsvn_wc/tree_conflicts.c
===================================================================
--- subversion/libsvn_wc/tree_conflicts.c.orig
+++ subversion/libsvn_wc/tree_conflicts.c
@@ -82,6 +82,7 @@ const svn_token_map_t svn_wc__conflict_r
{ "unversioned", svn_wc_conflict_reason_unversioned },
{ "moved-away", svn_wc_conflict_reason_moved_away },
{ "moved-here", svn_wc_conflict_reason_moved_here },
+ { "not-locked", svn_wc_conflict_reason_not_locked },
{ NULL }
};
Index: subversion/svn/cl-conflicts.c
===================================================================
--- subversion/svn/cl-conflicts.c.orig
+++ subversion/svn/cl-conflicts.c
@@ -55,6 +55,7 @@ static const svn_token_map_t map_conflic
{ "unversioned", svn_wc_conflict_reason_unversioned },
{ "moved-away", svn_wc_conflict_reason_moved_away },
{ "moved-here", svn_wc_conflict_reason_moved_here },
+ { "not-locked", svn_wc_conflict_reason_not_locked },
{ NULL, 0 }
};
@@ -91,6 +92,8 @@ local_reason_str(svn_node_kind_t kind, s
return _("local file moved away");
case svn_wc_conflict_reason_moved_here:
return _("local file moved here");
+ case svn_wc_conflict_reason_not_locked:
+ return _("local file not locked");
}
break;
case svn_node_dir:
@@ -117,6 +120,8 @@ local_reason_str(svn_node_kind_t kind, s
return _("local dir moved away");
case svn_wc_conflict_reason_moved_here:
return _("local dir moved here");
+ case svn_wc_conflict_reason_not_locked:
+ return _("local dir not locked"); /* Can't happen, surely? */
}
break;
case svn_node_none:
@@ -144,6 +149,8 @@ local_reason_str(svn_node_kind_t kind, s
return _("local moved away");
case svn_wc_conflict_reason_moved_here:
return _("local moved here");
+ case svn_wc_conflict_reason_not_locked:
+ return _("local not locked");
}
break;
}
Index: subversion/tests/libsvn_wc/op-depth-test.c
===================================================================
--- subversion/tests/libsvn_wc/op-depth-test.c.orig
+++ subversion/tests/libsvn_wc/op-depth-test.c
@@ -389,6 +389,7 @@ print_conflict(const conflict_info_t *ro
CASE_ENUM_STRVAL(reason, svn_wc_conflict_reason_replaced);
CASE_ENUM_STRVAL(reason, svn_wc_conflict_reason_moved_away);
CASE_ENUM_STRVAL(reason, svn_wc_conflict_reason_moved_here);
+ CASE_ENUM_STRVAL(reason, svn_wc_conflict_reason_not_locked);
default:
SVN_ERR_MALFUNCTION_NO_RETURN();
}
]]]
