You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@subversion.apache.org by st...@apache.org on 2011/05/24 18:35:15 UTC
svn commit: r1127134 - in /subversion/trunk/subversion: libsvn_client/add.c
libsvn_wc/adm_ops.c tests/cmdline/tree_conflict_tests.py
Author: stsp
Date: Tue May 24 16:35:14 2011
New Revision: 1127134
URL: http://svn.apache.org/viewvc?rev=1127134&view=rev
Log:
As part of issue #3779, "actual-only nodes need regression tests",
make 'svn add' detect tree conflict victims that do not exist on disk
and prevent adding new nodes at that path with a meaningful error message.
This implies that if users need to add a new node to resolve the conflict
they need to mark the conflict as resolved first. I think this is safer
than allowing accidental additions to take place. Since the node is not
visible on disk the addition might be a mistake.
* subversion/libsvn_wc/adm_ops.c
(check_can_add_node): Don't allow adding new items on top of nonexistent
conflicted nodes.
* subversion/libsvn_client/add.c
(add): As previous.
* subversion/tests/cmdline/tree_conflict_tests.py
(actual_only_node_behaviour): Adjust test cases for 'add' and 'mkdir'.
Modified:
subversion/trunk/subversion/libsvn_client/add.c
subversion/trunk/subversion/libsvn_wc/adm_ops.c
subversion/trunk/subversion/tests/cmdline/tree_conflict_tests.py
Modified: subversion/trunk/subversion/libsvn_client/add.c
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_client/add.c?rev=1127134&r1=1127133&r2=1127134&view=diff
==============================================================================
--- subversion/trunk/subversion/libsvn_client/add.c (original)
+++ subversion/trunk/subversion/libsvn_client/add.c Tue May 24 16:35:14 2011
@@ -537,10 +537,29 @@ add(void *baton, apr_pool_t *result_pool
else if (kind == svn_node_file)
err = add_file(b->local_abspath, b->ctx, scratch_pool);
else if (kind == svn_node_none)
- return svn_error_createf(SVN_ERR_WC_PATH_NOT_FOUND, NULL,
- _("'%s' not found"),
- svn_dirent_local_style(b->local_abspath,
- scratch_pool));
+ {
+ svn_boolean_t tree_conflicted;
+
+ /* Provide a meaningful error message if the node does not exist
+ * on disk but is a tree conflict victim. */
+ err = svn_wc_conflicted_p3(NULL, NULL, &tree_conflicted,
+ b->ctx->wc_ctx, b->local_abspath,
+ scratch_pool);
+ if (err)
+ svn_error_clear(err);
+ else if (tree_conflicted)
+ return svn_error_createf(SVN_ERR_WC_FOUND_CONFLICT, NULL,
+ _("'%s' is an existing item in conflict; "
+ "please mark the conflict as resolved "
+ "before adding a new item here"),
+ svn_dirent_local_style(b->local_abspath,
+ scratch_pool));
+
+ return svn_error_createf(SVN_ERR_WC_PATH_NOT_FOUND, NULL,
+ _("'%s' not found"),
+ svn_dirent_local_style(b->local_abspath,
+ scratch_pool));
+ }
else
return svn_error_createf(SVN_ERR_UNSUPPORTED_FEATURE, NULL,
_("Unsupported node kind for path '%s'"),
Modified: subversion/trunk/subversion/libsvn_wc/adm_ops.c
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_wc/adm_ops.c?rev=1127134&r1=1127133&r2=1127134&view=diff
==============================================================================
--- subversion/trunk/subversion/libsvn_wc/adm_ops.c (original)
+++ subversion/trunk/subversion/libsvn_wc/adm_ops.c Tue May 24 16:35:14 2011
@@ -849,10 +849,12 @@ check_can_add_node(svn_node_kind_t *kind
adding the new node; if not, return an error. */
{
svn_wc__db_status_t status;
+ svn_boolean_t conflicted;
svn_error_t *err
= svn_wc__db_read_info(&status, NULL, NULL, NULL, NULL, NULL, NULL,
NULL, NULL, NULL, NULL, NULL, NULL, NULL,
- NULL, NULL, NULL, NULL, NULL, NULL, NULL,
+ NULL, NULL, NULL, NULL, NULL, NULL,
+ &conflicted,
NULL, NULL, NULL, NULL, NULL, NULL,
db, local_abspath,
scratch_pool, scratch_pool);
@@ -870,6 +872,16 @@ check_can_add_node(svn_node_kind_t *kind
{
is_wc_root = FALSE;
exists = TRUE;
+
+ /* Note that the node may be in conflict even if it does not
+ * exist on disk (certain tree conflict scenarios). */
+ if (conflicted)
+ return svn_error_createf(SVN_ERR_WC_FOUND_CONFLICT, NULL,
+ _("'%s' is an existing item in conflict; "
+ "please mark the conflict as resolved "
+ "before adding a new item here"),
+ svn_dirent_local_style(local_abspath,
+ scratch_pool));
switch (status)
{
case svn_wc__db_status_not_present:
Modified: subversion/trunk/subversion/tests/cmdline/tree_conflict_tests.py
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/tests/cmdline/tree_conflict_tests.py?rev=1127134&r1=1127133&r2=1127134&view=diff
==============================================================================
--- subversion/trunk/subversion/tests/cmdline/tree_conflict_tests.py (original)
+++ subversion/trunk/subversion/tests/cmdline/tree_conflict_tests.py Tue May 24 16:35:14 2011
@@ -1134,23 +1134,17 @@ def actual_only_node_behaviour(sbox):
# add
expected_stdout = None
- expected_stderr = ".*foo.*not found.*"
+ expected_stderr = ".*foo.*is an existing item in conflict.*"
run_and_verify_svn(None, expected_stdout, expected_stderr,
"add", foo_path)
# add (with an existing obstruction of foo)
- ### this does not error out -- needs review
svntest.main.file_write(foo_path, "This is an obstruction of foo.\n")
- expected_stdout = "A.*foo"
- expected_stderr = []
+ expected_stdout = None
+ expected_stderr = ".*foo.*is an existing item in conflict.*"
run_and_verify_svn(None, expected_stdout, expected_stderr,
"add", foo_path)
- ### for now, ignore the fact that add succeeds, revert the entire
- ### working copy, and repeat the merge so we can test more commands
- svntest.main.run_svn(None, "revert", "-R", wc_dir)
os.remove(foo_path) # remove obstruction
- svntest.main.run_svn(None, "merge", '-c', '4', A_copy_url,
- os.path.join(wc_dir, 'A'))
# blame (praise, annotate, ann)
expected_stdout = None
@@ -1267,17 +1261,10 @@ def actual_only_node_behaviour(sbox):
run_and_verify_svn(None, expected_stdout, expected_stderr,
"mergeinfo", A_copy_url + '/foo', foo_path)
# mkdir
- ### this does not error out -- needs review
expected_stdout = None
- expected_stderr = []
+ expected_stderr = ".*foo.*is an existing item in conflict.*"
run_and_verify_svn(None, expected_stdout, expected_stderr,
"mkdir", foo_path)
- ### for now, ignore the fact that mkdir succeeds, revert the entire
- ### working copy, and repeat the merge so we can test more commands
- svntest.main.run_svn(None, "revert", "-R", wc_dir)
- os.rmdir(foo_path) # remove obstruction
- svntest.main.run_svn(None, "merge", '-c', '4', A_copy_url,
- os.path.join(wc_dir, 'A'))
# move (mv, rename, ren)
expected_stdout = None
Re: svn commit: r1127134 - in /subversion/trunk/subversion:
libsvn_client/add.c libsvn_wc/adm_ops.c tests/cmdline/tree_conflict_tests.py
Posted by Paul Burba <pt...@gmail.com>.
On Wed, Sep 7, 2011 at 3:01 AM, Stefan Sperling <st...@apache.org> wrote:
> On Tue, Sep 06, 2011 at 06:04:32PM -0400, Paul Burba wrote:
>> On Tue, May 24, 2011 at 12:35 PM, <st...@apache.org> wrote:
>> > Author: stsp
>> > Date: Tue May 24 16:35:14 2011
>> > New Revision: 1127134
>> >
>> > URL: http://svn.apache.org/viewvc?rev=1127134&view=rev
>> > Log:
>> > As part of issue #3779, "actual-only nodes need regression tests",
>> > make 'svn add' detect tree conflict victims that do not exist on disk
>> > and prevent adding new nodes at that path with a meaningful error message.
>> >
>
>> Hi Stefan,
>>
>> While reviewing some outstanding merge-related issues I noticed that
>> this change breaks the use-case of incoming replacements on local
>> deletes. The delete portion of the replacement is handled and a
>> tree-conflict set by the time the add is done and the above error is
>> raised.
>
> Hmmm, this a problem.
> I remember fixing all sorts of merge replacement issues in 1.6.x.
> So this might even be a regression from 1.6.x :(
Unfortunately it is a regression from 1.6:
svn1.6.18-dev>svn merge ^^/A branch
--- Merging r2 through r4 into 'branch':
C branch\C
Summary of conflicts:
Tree conflicts: 1
svn1.6.18-dev>svn st
M branch
A + C branch\C
> local delete, incoming delete upon merge
>> Not exactly sure how to fix this...I can look at it further tomorrow,
>> just wanted to get your thoughts if you have time.
>
> Does reverting the changes made to libsvn_wc/adm_ops.c fix the problem?
It does (I added issue #4011 for this and a corresponding test in
r1166229). However with that reversion, tree_conflict_tests.py 23:
test behaviour with actual-only nodes now fails:
S:\SVN\src-trunk>win-tests.py -d -c --log-to-stdout --test=tree_conflict#23
Testing Debug configuration on local repository.
START: tree_conflict_tests.py
CMD: svnadmin.exe create svn-test-work\local_tmp\repos
--bdb-txn-nosync --fs-type=fsfs
<TIME = 0.029000>
CMD: svn.exe import -m "Log message for revision 1."
svn-test-work\local_tmp\greekfiles
file:///S:/SVN/src-trunk/Debug/subversion/tests/cmdline/svn-test-work/lo
cal_tmp/repos --config-dir
S:\SVN\src-trunk\Debug\subversion\tests\cmdline\svn-test-work\local_tmp\config
--password rayjandom --no-auth-cache --username jrando
m
<TIME = 0.069000>
Adding svn-test-work\local_tmp\greekfiles\A
Adding svn-test-work\local_tmp\greekfiles\A\B
Adding svn-test-work\local_tmp\greekfiles\A\B\lambda
Adding svn-test-work\local_tmp\greekfiles\A\B\E
Adding svn-test-work\local_tmp\greekfiles\A\B\E\alpha
Adding svn-test-work\local_tmp\greekfiles\A\B\E\beta
Adding svn-test-work\local_tmp\greekfiles\A\B\F
Adding svn-test-work\local_tmp\greekfiles\A\mu
Adding svn-test-work\local_tmp\greekfiles\A\C
Adding svn-test-work\local_tmp\greekfiles\A\D
Adding svn-test-work\local_tmp\greekfiles\A\D\gamma
Adding svn-test-work\local_tmp\greekfiles\A\D\G
Adding svn-test-work\local_tmp\greekfiles\A\D\G\pi
Adding svn-test-work\local_tmp\greekfiles\A\D\G\rho
Adding svn-test-work\local_tmp\greekfiles\A\D\G\tau
Adding svn-test-work\local_tmp\greekfiles\A\D\H
Adding svn-test-work\local_tmp\greekfiles\A\D\H\chi
Adding svn-test-work\local_tmp\greekfiles\A\D\H\omega
Adding svn-test-work\local_tmp\greekfiles\A\D\H\psi
Adding svn-test-work\local_tmp\greekfiles\iota
Committed revision 1.
CMD: svnadmin.exe create
svn-test-work\repositories\tree_conflict_tests-23 --bdb-txn-nosync
--fs-type=fsfs
<TIME = 0.030000>
CMD: svnadmin.exe dump svn-test-work\local_tmp\repos | svnadmin.exe
load svn-test-work\repositories\tree_conflict_tests-23 --ignore-uuid
<TIME = 0.002000>
CMD: svn.exe co
file:///S:/SVN/src-trunk/Debug/subversion/tests/cmdline/svn-test-work/repositories/tree_conflict_tests-23
svn-test-work\working_copies\tree_conf
lict_tests-23 --config-dir
S:\SVN\src-trunk\Debug\subversion\tests\cmdline\svn-test-work\local_tmp\config
--password rayjandom --no-auth-cache --username jrando
m
<TIME = 0.098000>
A svn-test-work\working_copies\tree_conflict_tests-23\A
A svn-test-work\working_copies\tree_conflict_tests-23\A\B
A svn-test-work\working_copies\tree_conflict_tests-23\A\B\lambda
A svn-test-work\working_copies\tree_conflict_tests-23\A\B\E
A svn-test-work\working_copies\tree_conflict_tests-23\A\B\E\alpha
A svn-test-work\working_copies\tree_conflict_tests-23\A\B\E\beta
A svn-test-work\working_copies\tree_conflict_tests-23\A\B\F
A svn-test-work\working_copies\tree_conflict_tests-23\A\mu
A svn-test-work\working_copies\tree_conflict_tests-23\A\C
A svn-test-work\working_copies\tree_conflict_tests-23\A\D
A svn-test-work\working_copies\tree_conflict_tests-23\A\D\gamma
A svn-test-work\working_copies\tree_conflict_tests-23\A\D\G
A svn-test-work\working_copies\tree_conflict_tests-23\A\D\G\pi
A svn-test-work\working_copies\tree_conflict_tests-23\A\D\G\rho
A svn-test-work\working_copies\tree_conflict_tests-23\A\D\G\tau
A svn-test-work\working_copies\tree_conflict_tests-23\A\D\H
A svn-test-work\working_copies\tree_conflict_tests-23\A\D\H\chi
A svn-test-work\working_copies\tree_conflict_tests-23\A\D\H\omega
A svn-test-work\working_copies\tree_conflict_tests-23\A\D\H\psi
A svn-test-work\working_copies\tree_conflict_tests-23\iota
Checked out revision 1.
CMD: svn.exe copy -m "File
'S:\SVN\src-trunk\subversion\tests\cmdline\tree_conflict_tests.py',
line 1119, in actual_only_node_behaviour" file:///S:/SVN/src-trun
k/Debug/subversion/tests/cmdline/svn-test-work/repositories/tree_conflict_tests-23/A
file:///S:/SVN/src-trunk/Debug/subversion/tests/cmdline/svn-test-work/repos
itories/tree_conflict_tests-23/A_copy --config-dir
S:\SVN\src-trunk\Debug\subversion\tests\cmdline\svn-test-work\local_tmp\config
--password rayjandom --no-auth
-cache --username jrandom
<TIME = 0.037000>
Committed revision 2.
CMD: svn.exe checkout
file:///S:/SVN/src-trunk/Debug/subversion/tests/cmdline/svn-test-work/repositories/tree_conflict_tests-23/A_copy
svn-test-work\working_cop
ies\tree_conflict_tests-23.wc2 --config-dir
S:\SVN\src-trunk\Debug\subversion\tests\cmdline\svn-test-work\local_tmp\config
--password rayjandom --no-auth-cache
--username jrandom
<TIME = 0.094000>
A svn-test-work\working_copies\tree_conflict_tests-23.wc2\B
A svn-test-work\working_copies\tree_conflict_tests-23.wc2\B\lambda
A svn-test-work\working_copies\tree_conflict_tests-23.wc2\B\E
A svn-test-work\working_copies\tree_conflict_tests-23.wc2\B\E\alpha
A svn-test-work\working_copies\tree_conflict_tests-23.wc2\B\E\beta
A svn-test-work\working_copies\tree_conflict_tests-23.wc2\B\F
A svn-test-work\working_copies\tree_conflict_tests-23.wc2\mu
A svn-test-work\working_copies\tree_conflict_tests-23.wc2\C
A svn-test-work\working_copies\tree_conflict_tests-23.wc2\D
A svn-test-work\working_copies\tree_conflict_tests-23.wc2\D\gamma
A svn-test-work\working_copies\tree_conflict_tests-23.wc2\D\G
A svn-test-work\working_copies\tree_conflict_tests-23.wc2\D\G\pi
A svn-test-work\working_copies\tree_conflict_tests-23.wc2\D\G\rho
A svn-test-work\working_copies\tree_conflict_tests-23.wc2\D\G\tau
A svn-test-work\working_copies\tree_conflict_tests-23.wc2\D\H
A svn-test-work\working_copies\tree_conflict_tests-23.wc2\D\H\chi
A svn-test-work\working_copies\tree_conflict_tests-23.wc2\D\H\omega
A svn-test-work\working_copies\tree_conflict_tests-23.wc2\D\H\psi
Checked out revision 2.
CMD: svn.exe add
svn-test-work\working_copies\tree_conflict_tests-23.wc2\foo
--config-dir S:\SVN\src-trunk\Debug\subversion\tests\cmdline\svn-test-work\local_tm
p\config --password rayjandom --no-auth-cache --username jrandom
<TIME = 0.025000>
A svn-test-work\working_copies\tree_conflict_tests-23.wc2\foo
CMD: svn.exe commit -m "File
'S:\SVN\src-trunk\subversion\tests\cmdline\tree_conflict_tests.py',
line 1127, in actual_only_node_behaviour" svn-test-work\working
_copies\tree_conflict_tests-23.wc2\foo --config-dir
S:\SVN\src-trunk\Debug\subversion\tests\cmdline\svn-test-work\local_tmp\config
--password rayjandom --no-aut
h-cache --username jrandom
<TIME = 0.048000>
Adding svn-test-work\working_copies\tree_conflict_tests-23.wc2\foo
Transmitting file data .
Committed revision 3.
CMD: svn.exe commit -m "File
'S:\SVN\src-trunk\subversion\tests\cmdline\tree_conflict_tests.py',
line 1132, in actual_only_node_behaviour" svn-test-work\working
_copies\tree_conflict_tests-23.wc2 --config-dir
S:\SVN\src-trunk\Debug\subversion\tests\cmdline\svn-test-work\local_tmp\config
--password rayjandom --no-auth-ca
che --username jrandom
<TIME = 0.052000>
Sending svn-test-work\working_copies\tree_conflict_tests-23.wc2\foo
Transmitting file data .
Committed revision 4.
CMD: svn.exe update
svn-test-work\working_copies\tree_conflict_tests-23 --config-dir
S:\SVN\src-trunk\Debug\subversion\tests\cmdline\svn-test-work\local_tmp\con
fig --password rayjandom --no-auth-cache --username jrandom
<TIME = 0.082000>
Updating 'svn-test-work\working_copies\tree_conflict_tests-23':
A svn-test-work\working_copies\tree_conflict_tests-23\A_copy
A svn-test-work\working_copies\tree_conflict_tests-23\A_copy\B
A svn-test-work\working_copies\tree_conflict_tests-23\A_copy\B\lambda
A svn-test-work\working_copies\tree_conflict_tests-23\A_copy\B\E
A svn-test-work\working_copies\tree_conflict_tests-23\A_copy\B\E\alpha
A svn-test-work\working_copies\tree_conflict_tests-23\A_copy\B\E\beta
A svn-test-work\working_copies\tree_conflict_tests-23\A_copy\B\F
A svn-test-work\working_copies\tree_conflict_tests-23\A_copy\mu
A svn-test-work\working_copies\tree_conflict_tests-23\A_copy\C
A svn-test-work\working_copies\tree_conflict_tests-23\A_copy\D
A svn-test-work\working_copies\tree_conflict_tests-23\A_copy\D\gamma
A svn-test-work\working_copies\tree_conflict_tests-23\A_copy\D\G
A svn-test-work\working_copies\tree_conflict_tests-23\A_copy\D\G\pi
A svn-test-work\working_copies\tree_conflict_tests-23\A_copy\D\G\rho
A svn-test-work\working_copies\tree_conflict_tests-23\A_copy\D\G\tau
A svn-test-work\working_copies\tree_conflict_tests-23\A_copy\D\H
A svn-test-work\working_copies\tree_conflict_tests-23\A_copy\D\H\chi
A svn-test-work\working_copies\tree_conflict_tests-23\A_copy\D\H\omega
A svn-test-work\working_copies\tree_conflict_tests-23\A_copy\D\H\psi
A svn-test-work\working_copies\tree_conflict_tests-23\A_copy\foo
Updated to revision 4.
CMD: svn.exe merge -c 4
file:///S:/SVN/src-trunk/Debug/subversion/tests/cmdline/svn-test-work/repositories/tree_conflict_tests-23/A_copy
svn-test-work\working_c
opies\tree_conflict_tests-23\A --config-dir
S:\SVN\src-trunk\Debug\subversion\tests\cmdline\svn-test-work\local_tmp\config
--password rayjandom --no-auth-cache
--username jrandom
<TIME = 0.047000>
--- Merging r4 into 'svn-test-work\working_copies\tree_conflict_tests-23\A':
C svn-test-work\working_copies\tree_conflict_tests-23\A\foo
--- Recording mergeinfo for merge of r4 into
'svn-test-work\working_copies\tree_conflict_tests-23\A':
U svn-test-work\working_copies\tree_conflict_tests-23\A
Summary of conflicts:
Tree conflicts: 1
CMD: svn.exe add
svn-test-work\working_copies\tree_conflict_tests-23\A\foo --config-dir
S:\SVN\src-trunk\Debug\subversion\tests\cmdline\svn-test-work\local_tmp\
config --password rayjandom --no-auth-cache --username jrandom
CMD: S:\SVN\src-trunk\Debug\subversion\svn\svn.exe add
svn-test-work\working_copies\tree_conflict_tests-23\A\foo --config-dir
S:\SVN\src-trunk\Debug\subversion\
tests\cmdline\svn-test-work\local_tmp\config --password rayjandom
--no-auth-cache --username jrandom exited with 1
<TIME = 0.026000>
..\..\..\subversion\svn\add-cmd.c:86: (apr_err=155015)
..\..\..\subversion\svn\util.c:970: (apr_err=155015)
..\..\..\subversion\libsvn_client\add.c:713: (apr_err=155015)
..\..\..\subversion\libsvn_client\add.c:586: (apr_err=155015)
..\..\..\subversion\libsvn_client\add.c:586: (apr_err=155015)
svn: E155015: 'S:\SVN\src-trunk\Debug\subversion\tests\cmdline\svn-test-work\working_copies\tree_conflict_tests-23\A\foo'
is an existing item in conflict; pleas
e mark the conflict as resolved before adding a new item here
CMD: svn.exe add
svn-test-work\working_copies\tree_conflict_tests-23\A\foo --config-dir
S:\SVN\src-trunk\Debug\subversion\tests\cmdline\svn-test-work\local_tmp\
config --password rayjandom --no-auth-cache --username jrandom
<TIME = 0.025000>
A svn-test-work\working_copies\tree_conflict_tests-23\A\foo
EXPECTED STDERR (regexp):
.*foo.*is an existing item in conflict.*
ACTUAL STDERR:
CWD: S:\SVN\src-trunk\Debug\subversion\tests\cmdline
EXCEPTION: SVNUnmatchedError
Traceback (most recent call last):
File "S:\SVN\src-trunk\subversion\tests\cmdline\svntest\main.py",
line 1312, in run
rc = self.pred.run(sandbox)
File "S:\SVN\src-trunk\subversion\tests\cmdline\svntest\testcase.py",
line 176, in run
return self.func(sandbox)
File "S:\SVN\src-trunk\subversion\tests\cmdline\tree_conflict_tests.py",
line 1154, in actual_only_node_behaviour
"add", foo_path)
File "S:\SVN\src-trunk\subversion\tests\cmdline\svntest\actions.py",
line 268, in run_and_verify_svn
expected_exit, *varargs)
File "S:\SVN\src-trunk\subversion\tests\cmdline\svntest\actions.py",
line 307, in run_and_verify_svn2
verify.verify_outputs(message, out, err, expected_stdout, expected_stderr)
File "S:\SVN\src-trunk\subversion\tests\cmdline\svntest\verify.py",
line 388, in verify_outputs
compare_and_display_lines(message, label, expected, actual, raisable)
File "S:\SVN\src-trunk\subversion\tests\cmdline\svntest\verify.py",
line 361, in compare_and_display_lines
raise raisable
SVNUnmatchedError
FAIL: tree_conflict_tests.py 23: test behaviour with actual-only nodes
END: tree_conflict_tests.py
ELAPSED: tree_conflict_tests.py 0:00:00.968000
> I was aiming at changing the behaviour of 'add' and 'mkdir', not 'merge'.
> Both 'add' and 'mkdir' should be covered by the change to libsvn_client/add.c.
>
> I probably just added the check to adm_ops.c as well in case some API callers
> bypass svn_client_add(). But I did not consider the implications for 'merge'.
>
Re: svn commit: r1127134 - in /subversion/trunk/subversion:
libsvn_client/add.c libsvn_wc/adm_ops.c tests/cmdline/tree_conflict_tests.py
Posted by Stefan Sperling <st...@apache.org>.
On Tue, Sep 06, 2011 at 06:04:32PM -0400, Paul Burba wrote:
> On Tue, May 24, 2011 at 12:35 PM, <st...@apache.org> wrote:
> > Author: stsp
> > Date: Tue May 24 16:35:14 2011
> > New Revision: 1127134
> >
> > URL: http://svn.apache.org/viewvc?rev=1127134&view=rev
> > Log:
> > As part of issue #3779, "actual-only nodes need regression tests",
> > make 'svn add' detect tree conflict victims that do not exist on disk
> > and prevent adding new nodes at that path with a meaningful error message.
> >
> Hi Stefan,
>
> While reviewing some outstanding merge-related issues I noticed that
> this change breaks the use-case of incoming replacements on local
> deletes. The delete portion of the replacement is handled and a
> tree-conflict set by the time the add is done and the above error is
> raised.
Hmmm, this a problem.
I remember fixing all sorts of merge replacement issues in 1.6.x.
So this might even be a regression from 1.6.x :(
> Not exactly sure how to fix this...I can look at it further tomorrow,
> just wanted to get your thoughts if you have time.
Does reverting the changes made to libsvn_wc/adm_ops.c fix the problem?
I was aiming at changing the behaviour of 'add' and 'mkdir', not 'merge'.
Both 'add' and 'mkdir' should be covered by the change to libsvn_client/add.c.
I probably just added the check to adm_ops.c as well in case some API callers
bypass svn_client_add(). But I did not consider the implications for 'merge'.
Re: svn commit: r1127134 - in /subversion/trunk/subversion:
libsvn_client/add.c libsvn_wc/adm_ops.c tests/cmdline/tree_conflict_tests.py
Posted by Paul Burba <pt...@gmail.com>.
On Tue, May 24, 2011 at 12:35 PM, <st...@apache.org> wrote:
> Author: stsp
> Date: Tue May 24 16:35:14 2011
> New Revision: 1127134
>
> URL: http://svn.apache.org/viewvc?rev=1127134&view=rev
> Log:
> As part of issue #3779, "actual-only nodes need regression tests",
> make 'svn add' detect tree conflict victims that do not exist on disk
> and prevent adding new nodes at that path with a meaningful error message.
>
> This implies that if users need to add a new node to resolve the conflict
> they need to mark the conflict as resolved first. I think this is safer
> than allowing accidental additions to take place. Since the node is not
> visible on disk the addition might be a mistake.
>
> * subversion/libsvn_wc/adm_ops.c
> (check_can_add_node): Don't allow adding new items on top of nonexistent
> conflicted nodes.
>
> * subversion/libsvn_client/add.c
> (add): As previous.
>
> * subversion/tests/cmdline/tree_conflict_tests.py
> (actual_only_node_behaviour): Adjust test cases for 'add' and 'mkdir'.
>
> Modified:
> subversion/trunk/subversion/libsvn_client/add.c
> subversion/trunk/subversion/libsvn_wc/adm_ops.c
> subversion/trunk/subversion/tests/cmdline/tree_conflict_tests.py
>
> Modified: subversion/trunk/subversion/libsvn_client/add.c
> URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_client/add.c?rev=1127134&r1=1127133&r2=1127134&view=diff
> ==============================================================================
> --- subversion/trunk/subversion/libsvn_client/add.c (original)
> +++ subversion/trunk/subversion/libsvn_client/add.c Tue May 24 16:35:14 2011
> @@ -537,10 +537,29 @@ add(void *baton, apr_pool_t *result_pool
> else if (kind == svn_node_file)
> err = add_file(b->local_abspath, b->ctx, scratch_pool);
> else if (kind == svn_node_none)
> - return svn_error_createf(SVN_ERR_WC_PATH_NOT_FOUND, NULL,
> - _("'%s' not found"),
> - svn_dirent_local_style(b->local_abspath,
> - scratch_pool));
> + {
> + svn_boolean_t tree_conflicted;
> +
> + /* Provide a meaningful error message if the node does not exist
> + * on disk but is a tree conflict victim. */
> + err = svn_wc_conflicted_p3(NULL, NULL, &tree_conflicted,
> + b->ctx->wc_ctx, b->local_abspath,
> + scratch_pool);
> + if (err)
> + svn_error_clear(err);
> + else if (tree_conflicted)
> + return svn_error_createf(SVN_ERR_WC_FOUND_CONFLICT, NULL,
> + _("'%s' is an existing item in conflict; "
> + "please mark the conflict as resolved "
> + "before adding a new item here"),
> + svn_dirent_local_style(b->local_abspath,
> + scratch_pool));
> + return svn_error_createf(SVN_ERR_WC_PATH_NOT_FOUND, NULL,
> + _("'%s' not found"),
> + svn_dirent_local_style(b->local_abspath,
> + scratch_pool));
> + }
> else
> return svn_error_createf(SVN_ERR_UNSUPPORTED_FEATURE, NULL,
> _("Unsupported node kind for path '%s'"),
>
> Modified: subversion/trunk/subversion/libsvn_wc/adm_ops.c
> URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_wc/adm_ops.c?rev=1127134&r1=1127133&r2=1127134&view=diff
> ==============================================================================
> --- subversion/trunk/subversion/libsvn_wc/adm_ops.c (original)
> +++ subversion/trunk/subversion/libsvn_wc/adm_ops.c Tue May 24 16:35:14 2011
> @@ -849,10 +849,12 @@ check_can_add_node(svn_node_kind_t *kind
> adding the new node; if not, return an error. */
> {
> svn_wc__db_status_t status;
> + svn_boolean_t conflicted;
> svn_error_t *err
> = svn_wc__db_read_info(&status, NULL, NULL, NULL, NULL, NULL, NULL,
> NULL, NULL, NULL, NULL, NULL, NULL, NULL,
> - NULL, NULL, NULL, NULL, NULL, NULL, NULL,
> + NULL, NULL, NULL, NULL, NULL, NULL,
> + &conflicted,
> NULL, NULL, NULL, NULL, NULL, NULL,
> db, local_abspath,
> scratch_pool, scratch_pool);
> @@ -870,6 +872,16 @@ check_can_add_node(svn_node_kind_t *kind
> {
> is_wc_root = FALSE;
> exists = TRUE;
> +
> + /* Note that the node may be in conflict even if it does not
> + * exist on disk (certain tree conflict scenarios). */
> + if (conflicted)
> + return svn_error_createf(SVN_ERR_WC_FOUND_CONFLICT, NULL,
> + _("'%s' is an existing item in conflict; "
> + "please mark the conflict as resolved "
> + "before adding a new item here"),
> + svn_dirent_local_style(local_abspath,
> + scratch_pool));
Hi Stefan,
While reviewing some outstanding merge-related issues I noticed that
this change breaks the use-case of incoming replacements on local
deletes. The delete portion of the replacement is handled and a
tree-conflict set by the time the add is done and the above error is
raised.
Here's a quick demonstration starting with a vanilla Greek tree:
### Make a branch:
>svn copy ^^/A ^^/branch -m "Create a branch"
Committed revision 2.
### Replace a directory on the "trunk":
>svn del A\C
D A\C
>svn mkdir A\C
A A\C
>svn ci -m "Replace A/C"
Replacing A\C
Committed revision 3.
### Delete the corresponding directory on the "branch":
>svn del ^^/branch/C -m "Delete branch/C"
Committed revision 4.
>svn up -q
### Now try to merge the replacement onto the deletion; it fails:
>svn merge ^^/A branch
..\..\..\subversion\svn\util.c:913: (apr_err=155015)
..\..\..\subversion\libsvn_client\merge.c:11349: (apr_err=155015)
..\..\..\subversion\libsvn_client\merge.c:11303: (apr_err=155015)
..\..\..\subversion\libsvn_client\merge.c:11303: (apr_err=155015)
..\..\..\subversion\libsvn_client\merge.c:11273: (apr_err=155015)
..\..\..\subversion\libsvn_client\merge.c:9287: (apr_err=155015)
..\..\..\subversion\libsvn_client\merge.c:8870: (apr_err=155015)
..\..\..\subversion\libsvn_client\merge.c:5349: (apr_err=155015)
..\..\..\subversion\libsvn_repos\reporter.c:1430: (apr_err=155015)
..\..\..\subversion\libsvn_client\ra.c:247: (apr_err=155015)
..\..\..\subversion\libsvn_repos\reporter.c:1269: (apr_err=155015)
..\..\..\subversion\libsvn_repos\reporter.c:1205: (apr_err=155015)
..\..\..\subversion\libsvn_repos\reporter.c:920: (apr_err=155015)
..\..\..\subversion\libsvn_delta\cancel.c:120: (apr_err=155015)
..\..\..\subversion\libsvn_delta\cancel.c:120: (apr_err=155015)
..\..\..\subversion\libsvn_client\repos_diff.c:710: (apr_err=155015)
..\..\..\subversion\libsvn_client\merge.c:2234: (apr_err=155015)
..\..\..\subversion\libsvn_wc\adm_ops.c:1069: (apr_err=155015)
..\..\..\subversion\libsvn_wc\adm_ops.c:937: (apr_err=155015)
svn: E155015:
'C:\SVN\src-trunk\Debug\subversion\tests\cmdline\svn-test-work\working_copies\merge_tree_conflict_tests-24\branch\C'
is an existing item in conflict; please mark the conflict as resolved
before adding a new item here
>svn st
? C branch\C
> local delete, incoming delete upon merge
Summary of conflicts:
Tree conflicts: 1
### Prior to r1127134 the merge raised a tree conflict:
trunk@1127133>svn merge ^^/A branch
--- Merging r2 through r4 into 'branch':
C branch\C
--- Recording mergeinfo for merge of r2 through r4 into 'branch':
U branch
Summary of conflicts:
Tree conflicts: 1
trunk@1127133>svn st
M branch
! C branch\C
> local delete, incoming delete upon merge
Summary of conflicts:
Tree conflicts: 1
Not exactly sure how to fix this...I can look at it further tomorrow,
just wanted to get your thoughts if you have time.
Paul
> switch (status)
> {
> case svn_wc__db_status_not_present:
>
> Modified: subversion/trunk/subversion/tests/cmdline/tree_conflict_tests.py
> URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/tests/cmdline/tree_conflict_tests.py?rev=1127134&r1=1127133&r2=1127134&view=diff
> ==============================================================================
> --- subversion/trunk/subversion/tests/cmdline/tree_conflict_tests.py (original)
> +++ subversion/trunk/subversion/tests/cmdline/tree_conflict_tests.py Tue May 24 16:35:14 2011
> @@ -1134,23 +1134,17 @@ def actual_only_node_behaviour(sbox):
>
> # add
> expected_stdout = None
> - expected_stderr = ".*foo.*not found.*"
> + expected_stderr = ".*foo.*is an existing item in conflict.*"
> run_and_verify_svn(None, expected_stdout, expected_stderr,
> "add", foo_path)
>
> # add (with an existing obstruction of foo)
> - ### this does not error out -- needs review
> svntest.main.file_write(foo_path, "This is an obstruction of foo.\n")
> - expected_stdout = "A.*foo"
> - expected_stderr = []
> + expected_stdout = None
> + expected_stderr = ".*foo.*is an existing item in conflict.*"
> run_and_verify_svn(None, expected_stdout, expected_stderr,
> "add", foo_path)
> - ### for now, ignore the fact that add succeeds, revert the entire
> - ### working copy, and repeat the merge so we can test more commands
> - svntest.main.run_svn(None, "revert", "-R", wc_dir)
> os.remove(foo_path) # remove obstruction
> - svntest.main.run_svn(None, "merge", '-c', '4', A_copy_url,
> - os.path.join(wc_dir, 'A'))
>
> # blame (praise, annotate, ann)
> expected_stdout = None
> @@ -1267,17 +1261,10 @@ def actual_only_node_behaviour(sbox):
> run_and_verify_svn(None, expected_stdout, expected_stderr,
> "mergeinfo", A_copy_url + '/foo', foo_path)
> # mkdir
> - ### this does not error out -- needs review
> expected_stdout = None
> - expected_stderr = []
> + expected_stderr = ".*foo.*is an existing item in conflict.*"
> run_and_verify_svn(None, expected_stdout, expected_stderr,
> "mkdir", foo_path)
> - ### for now, ignore the fact that mkdir succeeds, revert the entire
> - ### working copy, and repeat the merge so we can test more commands
> - svntest.main.run_svn(None, "revert", "-R", wc_dir)
> - os.rmdir(foo_path) # remove obstruction
> - svntest.main.run_svn(None, "merge", '-c', '4', A_copy_url,
> - os.path.join(wc_dir, 'A'))
>
> # move (mv, rename, ren)
> expected_stdout = None
>
>
>
Re: svn commit: r1127134 - in /subversion/trunk/subversion:
libsvn_client/add.c libsvn_wc/adm_ops.c tests/cmdline/tree_conflict_tests.py
Posted by Paul Burba <pt...@gmail.com>.
On Tue, May 24, 2011 at 12:35 PM, <st...@apache.org> wrote:
> Author: stsp
> Date: Tue May 24 16:35:14 2011
> New Revision: 1127134
>
> URL: http://svn.apache.org/viewvc?rev=1127134&view=rev
> Log:
> As part of issue #3779, "actual-only nodes need regression tests",
> make 'svn add' detect tree conflict victims that do not exist on disk
> and prevent adding new nodes at that path with a meaningful error message.
>
> This implies that if users need to add a new node to resolve the conflict
> they need to mark the conflict as resolved first. I think this is safer
> than allowing accidental additions to take place. Since the node is not
> visible on disk the addition might be a mistake.
>
> * subversion/libsvn_wc/adm_ops.c
> (check_can_add_node): Don't allow adding new items on top of nonexistent
> conflicted nodes.
>
> * subversion/libsvn_client/add.c
> (add): As previous.
>
> * subversion/tests/cmdline/tree_conflict_tests.py
> (actual_only_node_behaviour): Adjust test cases for 'add' and 'mkdir'.
>
> Modified:
> subversion/trunk/subversion/libsvn_client/add.c
> subversion/trunk/subversion/libsvn_wc/adm_ops.c
> subversion/trunk/subversion/tests/cmdline/tree_conflict_tests.py
>
> Modified: subversion/trunk/subversion/libsvn_client/add.c
> URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_client/add.c?rev=1127134&r1=1127133&r2=1127134&view=diff
> ==============================================================================
> --- subversion/trunk/subversion/libsvn_client/add.c (original)
> +++ subversion/trunk/subversion/libsvn_client/add.c Tue May 24 16:35:14 2011
> @@ -537,10 +537,29 @@ add(void *baton, apr_pool_t *result_pool
> else if (kind == svn_node_file)
> err = add_file(b->local_abspath, b->ctx, scratch_pool);
> else if (kind == svn_node_none)
> - return svn_error_createf(SVN_ERR_WC_PATH_NOT_FOUND, NULL,
> - _("'%s' not found"),
> - svn_dirent_local_style(b->local_abspath,
> - scratch_pool));
> + {
> + svn_boolean_t tree_conflicted;
> +
> + /* Provide a meaningful error message if the node does not exist
> + * on disk but is a tree conflict victim. */
> + err = svn_wc_conflicted_p3(NULL, NULL, &tree_conflicted,
> + b->ctx->wc_ctx, b->local_abspath,
> + scratch_pool);
> + if (err)
> + svn_error_clear(err);
> + else if (tree_conflicted)
> + return svn_error_createf(SVN_ERR_WC_FOUND_CONFLICT, NULL,
> + _("'%s' is an existing item in conflict; "
> + "please mark the conflict as resolved "
> + "before adding a new item here"),
> + svn_dirent_local_style(b->local_abspath,
> + scratch_pool));
> + return svn_error_createf(SVN_ERR_WC_PATH_NOT_FOUND, NULL,
> + _("'%s' not found"),
> + svn_dirent_local_style(b->local_abspath,
> + scratch_pool));
> + }
> else
> return svn_error_createf(SVN_ERR_UNSUPPORTED_FEATURE, NULL,
> _("Unsupported node kind for path '%s'"),
>
> Modified: subversion/trunk/subversion/libsvn_wc/adm_ops.c
> URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_wc/adm_ops.c?rev=1127134&r1=1127133&r2=1127134&view=diff
> ==============================================================================
> --- subversion/trunk/subversion/libsvn_wc/adm_ops.c (original)
> +++ subversion/trunk/subversion/libsvn_wc/adm_ops.c Tue May 24 16:35:14 2011
> @@ -849,10 +849,12 @@ check_can_add_node(svn_node_kind_t *kind
> adding the new node; if not, return an error. */
> {
> svn_wc__db_status_t status;
> + svn_boolean_t conflicted;
> svn_error_t *err
> = svn_wc__db_read_info(&status, NULL, NULL, NULL, NULL, NULL, NULL,
> NULL, NULL, NULL, NULL, NULL, NULL, NULL,
> - NULL, NULL, NULL, NULL, NULL, NULL, NULL,
> + NULL, NULL, NULL, NULL, NULL, NULL,
> + &conflicted,
> NULL, NULL, NULL, NULL, NULL, NULL,
> db, local_abspath,
> scratch_pool, scratch_pool);
> @@ -870,6 +872,16 @@ check_can_add_node(svn_node_kind_t *kind
> {
> is_wc_root = FALSE;
> exists = TRUE;
> +
> + /* Note that the node may be in conflict even if it does not
> + * exist on disk (certain tree conflict scenarios). */
> + if (conflicted)
> + return svn_error_createf(SVN_ERR_WC_FOUND_CONFLICT, NULL,
> + _("'%s' is an existing item in conflict; "
> + "please mark the conflict as resolved "
> + "before adding a new item here"),
> + svn_dirent_local_style(local_abspath,
> + scratch_pool));
Hi Stefan,
While reviewing some outstanding merge-related issues I noticed that
this change breaks the use-case of incoming replacements on local
deletes. The delete portion of the replacement is handled and a
tree-conflict set by the time the add is done and the above error is
raised.
Here's a quick demonstration starting with a vanilla Greek tree:
### Make a branch:
>svn copy ^^/A ^^/branch -m "Create a branch"
Committed revision 2.
### Replace a directory on the "trunk":
>svn del A\C
D A\C
>svn mkdir A\C
A A\C
>svn ci -m "Replace A/C"
Replacing A\C
Committed revision 3.
### Delete the corresponding directory on the "branch":
>svn del ^^/branch/C -m "Delete branch/C"
Committed revision 4.
>svn up -q
### Now try to merge the replacement onto the deletion; it fails:
>svn merge ^^/A branch
..\..\..\subversion\svn\util.c:913: (apr_err=155015)
..\..\..\subversion\libsvn_client\merge.c:11349: (apr_err=155015)
..\..\..\subversion\libsvn_client\merge.c:11303: (apr_err=155015)
..\..\..\subversion\libsvn_client\merge.c:11303: (apr_err=155015)
..\..\..\subversion\libsvn_client\merge.c:11273: (apr_err=155015)
..\..\..\subversion\libsvn_client\merge.c:9287: (apr_err=155015)
..\..\..\subversion\libsvn_client\merge.c:8870: (apr_err=155015)
..\..\..\subversion\libsvn_client\merge.c:5349: (apr_err=155015)
..\..\..\subversion\libsvn_repos\reporter.c:1430: (apr_err=155015)
..\..\..\subversion\libsvn_client\ra.c:247: (apr_err=155015)
..\..\..\subversion\libsvn_repos\reporter.c:1269: (apr_err=155015)
..\..\..\subversion\libsvn_repos\reporter.c:1205: (apr_err=155015)
..\..\..\subversion\libsvn_repos\reporter.c:920: (apr_err=155015)
..\..\..\subversion\libsvn_delta\cancel.c:120: (apr_err=155015)
..\..\..\subversion\libsvn_delta\cancel.c:120: (apr_err=155015)
..\..\..\subversion\libsvn_client\repos_diff.c:710: (apr_err=155015)
..\..\..\subversion\libsvn_client\merge.c:2234: (apr_err=155015)
..\..\..\subversion\libsvn_wc\adm_ops.c:1069: (apr_err=155015)
..\..\..\subversion\libsvn_wc\adm_ops.c:937: (apr_err=155015)
svn: E155015:
'C:\SVN\src-trunk\Debug\subversion\tests\cmdline\svn-test-work\working_copies\merge_tree_conflict_tests-24\branch\C'
is an existing item in conflict; please mark the conflict as resolved
before adding a new item here
>svn st
? C branch\C
> local delete, incoming delete upon merge
Summary of conflicts:
Tree conflicts: 1
### Prior to r1127134 the merge raised a tree conflict:
trunk@1127133>svn merge ^^/A branch
--- Merging r2 through r4 into 'branch':
C branch\C
--- Recording mergeinfo for merge of r2 through r4 into 'branch':
U branch
Summary of conflicts:
Tree conflicts: 1
trunk@1127133>svn st
M branch
! C branch\C
> local delete, incoming delete upon merge
Summary of conflicts:
Tree conflicts: 1
Not exactly sure how to fix this...I can look at it further tomorrow,
just wanted to get your thoughts if you have time.
Paul
> switch (status)
> {
> case svn_wc__db_status_not_present:
>
> Modified: subversion/trunk/subversion/tests/cmdline/tree_conflict_tests.py
> URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/tests/cmdline/tree_conflict_tests.py?rev=1127134&r1=1127133&r2=1127134&view=diff
> ==============================================================================
> --- subversion/trunk/subversion/tests/cmdline/tree_conflict_tests.py (original)
> +++ subversion/trunk/subversion/tests/cmdline/tree_conflict_tests.py Tue May 24 16:35:14 2011
> @@ -1134,23 +1134,17 @@ def actual_only_node_behaviour(sbox):
>
> # add
> expected_stdout = None
> - expected_stderr = ".*foo.*not found.*"
> + expected_stderr = ".*foo.*is an existing item in conflict.*"
> run_and_verify_svn(None, expected_stdout, expected_stderr,
> "add", foo_path)
>
> # add (with an existing obstruction of foo)
> - ### this does not error out -- needs review
> svntest.main.file_write(foo_path, "This is an obstruction of foo.\n")
> - expected_stdout = "A.*foo"
> - expected_stderr = []
> + expected_stdout = None
> + expected_stderr = ".*foo.*is an existing item in conflict.*"
> run_and_verify_svn(None, expected_stdout, expected_stderr,
> "add", foo_path)
> - ### for now, ignore the fact that add succeeds, revert the entire
> - ### working copy, and repeat the merge so we can test more commands
> - svntest.main.run_svn(None, "revert", "-R", wc_dir)
> os.remove(foo_path) # remove obstruction
> - svntest.main.run_svn(None, "merge", '-c', '4', A_copy_url,
> - os.path.join(wc_dir, 'A'))
>
> # blame (praise, annotate, ann)
> expected_stdout = None
> @@ -1267,17 +1261,10 @@ def actual_only_node_behaviour(sbox):
> run_and_verify_svn(None, expected_stdout, expected_stderr,
> "mergeinfo", A_copy_url + '/foo', foo_path)
> # mkdir
> - ### this does not error out -- needs review
> expected_stdout = None
> - expected_stderr = []
> + expected_stderr = ".*foo.*is an existing item in conflict.*"
> run_and_verify_svn(None, expected_stdout, expected_stderr,
> "mkdir", foo_path)
> - ### for now, ignore the fact that mkdir succeeds, revert the entire
> - ### working copy, and repeat the merge so we can test more commands
> - svntest.main.run_svn(None, "revert", "-R", wc_dir)
> - os.rmdir(foo_path) # remove obstruction
> - svntest.main.run_svn(None, "merge", '-c', '4', A_copy_url,
> - os.path.join(wc_dir, 'A'))
>
> # move (mv, rename, ren)
> expected_stdout = None
>
>
>