You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@subversion.apache.org by Branko Čibej <br...@xbc.nu> on 2002/07/25 03:23:08 UTC
diff and merge behaviour
I'm sure many of you were glad to notice my huge commit to the
issue-531-dev branch. :-)
While doing this, I noticed several things:
1. During the merge, several new files that had appeared on /trunk
were added to my WC. Well and good; _but_ they were added without
ancestry, creating a new node. I submit that "svn merge" should
actually perform a copy when adding or replacing files; this would
let diff avoid the weirdness I describe below.
2. Now, when I diff /trunk and /branches-531-dev, I get _two_ diffs
for those files; one that removes it, and another that adds the
very same, identical contents. That's bad.
3. The other diff weirdness is that it insists on printing the diff
header, even when the actual diff turns out to be empty.
I suspect fixing (1) will fix (2). (3) should be trivial once we have
the integrated diff library.
Do we have any issues filed for these problems? I seem to remember
something about (3), but not (1) and (2).
--
Brane Čibej <br...@xbc.nu> http://www.xbc.nu/brane/
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: diff and merge behaviour [PATCH]
Posted by Branko Čibej <br...@xbc.nu>.
Ben Collins-Sussman wrote:
>Philip Martin <ph...@codematters.co.uk> writes:
>
>
>
>>How about this? It might not be very efficient, when using ra_dav
>>calling svn_client_copy will open another session, and but it works.
>>
>>
>
>Ah, I see that instead of the merge process calling svn_client_add(),
>you're now having it call svn_client_copy(URL, wc-path).
>
>I have a simpler proposal though: you already *have* the file, and
>already know the copyfrom-path and copyfrom-rev. So go ahead and
>svn_client_add() the new file... and then tweak the entry, inserting
>the copyfrom* args. Voila, no network access, same result. :-)
>
>
+1, this is exactly what should be happening.
--
Brane Čibej <br...@xbc.nu> http://www.xbc.nu/brane/
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: diff and merge behaviour [PATCH]
Posted by Ben Collins-Sussman <su...@collab.net>.
Philip Martin <ph...@codematters.co.uk> writes:
> How about this? It might not be very efficient, when using ra_dav
> calling svn_client_copy will open another session, and but it works.
Ah, I see that instead of the merge process calling svn_client_add(),
you're now having it call svn_client_copy(URL, wc-path).
I have a simpler proposal though: you already *have* the file, and
already know the copyfrom-path and copyfrom-rev. So go ahead and
svn_client_add() the new file... and then tweak the entry, inserting
the copyfrom* args. Voila, no network access, same result. :-)
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: diff and merge behaviour [PATCH]
Posted by Philip Martin <ph...@codematters.co.uk>.
Ben Collins-Sussman <su...@collab.net> writes:
> Branko Äibej <br...@xbc.nu> writes:
>
> > 1. During the merge, several new files that had appeared on /trunk
> > were added to my WC. Well and good; _but_ they were added without
> > ancestry, creating a new node. I submit that "svn merge" should
> > actually perform a copy when adding or replacing files; this would
> > let diff avoid the weirdness I describe below.
>
> Wow, that's a really great idea. I can't believe we never thought of
> this before. Can you file an enhancement request for this?
How about this? It might not be very efficient, when using ra_dav
calling svn_client_copy will open another session, and but it works.
Is it worth commiting in its present form, in order to get the new
behaviour? As usual I had problems writing the Python test :-(
Copy-with-history when adding new files and directories during a merge.
* subversion/libsvn_client/diff.c
(struct merge_cmd_baton): Add target, path, revision and auth_baton
members.
(merge_file_added, merge_dir_added): Use svn_client_copy instead of
svn_client_add.
(svn_client_merge): Set new members in merge_cmd_baton.
* subversion/tests/clients/cmdline/merge_tests.py
(add_with_history): New test.
Index: ./subversion/libsvn_client/diff.c
===================================================================
--- ./subversion/libsvn_client/diff.c
+++ ./subversion/libsvn_client/diff.c Fri Jul 26 00:10:58 2002
@@ -279,6 +279,10 @@
struct merge_cmd_baton {
svn_boolean_t force;
+ const char *target;
+ const char *path;
+ const svn_client_revision_t *revision;
+ svn_client_auth_baton_t *auth_baton;
apr_pool_t *pool;
};
@@ -347,13 +351,20 @@
struct merge_cmd_baton *merge_b = baton;
apr_pool_t *subpool = svn_pool_create (merge_b->pool);
enum svn_node_kind kind;
+ const char *copyfrom_url;
+ const char *child;
SVN_ERR (svn_io_check_path (mine, &kind, subpool));
switch (kind)
{
case svn_node_none:
- SVN_ERR (svn_io_copy_file (yours, mine, TRUE, subpool));
- SVN_ERR (svn_client_add (mine, FALSE, NULL, NULL, subpool));
+ child = svn_path_is_child(merge_b->target, mine, merge_b->pool);
+ assert (child != NULL);
+ copyfrom_url = svn_path_join (merge_b->path, child, merge_b->pool);
+ /* ### This will get the file again */
+ SVN_ERR (svn_client_copy (NULL, copyfrom_url, merge_b->revision, mine,
+ merge_b->auth_baton, NULL, NULL, NULL, NULL,
+ merge_b->pool));
break;
case svn_node_dir:
/* ### create a .drej conflict or something someday? */
@@ -443,13 +454,19 @@
apr_pool_t *subpool = svn_pool_create (merge_b->pool);
enum svn_node_kind kind;
svn_wc_entry_t *entry;
+ const char *copyfrom_url, *child;
SVN_ERR (svn_io_check_path (path, &kind, subpool));
switch (kind)
{
case svn_node_none:
- SVN_ERR (svn_client_mkdir (NULL, path, NULL, NULL, NULL,
- NULL, NULL, subpool));
+ child = svn_path_is_child (merge_b->target, path, merge_b->pool);
+ assert (child != NULL);
+ copyfrom_url = svn_path_join (merge_b->path, child, merge_b->pool);
+ /* ### This will get the directory tree again */
+ SVN_ERR (svn_client_copy (NULL, copyfrom_url, merge_b->revision, path,
+ merge_b->auth_baton, NULL, NULL, NULL, NULL,
+ merge_b->pool));
break;
case svn_node_dir:
/* ### should unversioned directories generate 'obstructed update'
@@ -1254,6 +1271,10 @@
{
struct merge_cmd_baton merge_cmd_baton;
merge_cmd_baton.force = force;
+ merge_cmd_baton.target = target_wcpath;
+ merge_cmd_baton.path = path2;
+ merge_cmd_baton.revision = revision2;
+ merge_cmd_baton.auth_baton = auth_baton;
merge_cmd_baton.pool = pool;
SVN_ERR (do_merge (notify_func,
Index: ./subversion/tests/clients/cmdline/merge_tests.py
===================================================================
--- ./subversion/tests/clients/cmdline/merge_tests.py
+++ ./subversion/tests/clients/cmdline/merge_tests.py Thu Jul 25 23:48:52 2002
@@ -366,6 +366,115 @@
#----------------------------------------------------------------------
+def add_with_history(sbox):
+ "merge and add new files/dirs with history"
+
+ if sbox.build():
+ return 1
+
+ wc_dir = sbox.wc_dir
+
+ C_path = os.path.join(wc_dir, 'A', 'C')
+ F_path = os.path.join(wc_dir, 'A', 'B', 'F')
+ F_url = os.path.join(svntest.main.current_repo_url, 'A', 'B', 'F')
+ Q_path = os.path.join(F_path, 'Q')
+ svntest.main.run_svn(None, 'mkdir', Q_path)
+ foo_path = os.path.join(F_path, 'foo')
+ svntest.main.file_append(foo_path, "foo")
+ svntest.main.run_svn(None, 'add', foo_path)
+ bar_path = os.path.join(Q_path, 'bar')
+ svntest.main.file_append(bar_path, "bar")
+ svntest.main.run_svn(None, 'add', bar_path)
+
+ expected_output = svntest.wc.State(wc_dir, {
+ 'A/B/F/Q' : Item(verb='Adding'),
+ 'A/B/F/Q/bar' : Item(verb='Adding'),
+ 'A/B/F/foo' : Item(verb='Adding'),
+ })
+ expected_status = svntest.actions.get_virginal_state(wc_dir, 2)
+ expected_status.tweak(wc_rev=1)
+ expected_status.add({
+ 'A/B/F/Q' : Item(status='_ ', wc_rev=2, repos_rev=2),
+ 'A/B/F/Q/bar' : Item(status='_ ', wc_rev=2, repos_rev=2),
+ 'A/B/F/foo' : Item(status='_ ', wc_rev=2, repos_rev=2),
+ })
+ if svntest.actions.run_and_verify_commit(wc_dir,
+ expected_output,
+ expected_status,
+ None,
+ None, None,
+ None, None,
+ wc_dir):
+ print "commit failed"
+ return 1
+
+### I give up, what is the magic incantation to get this to work?
+#
+# expected_output = wc.State(C_path, {'Q' : Item(status='A '),
+# 'Q/bar' : Item(status='A '),
+# 'foo' : Item(status='A '),
+# })
+#
+# expected_disk = wc.State(C_path, {'Q' : wc.StateItem(),
+# 'Q/bar' : wc.StateItem("bar"),
+# 'foo' : wc.StateItem("foo"),
+# })
+#
+# expected_status= wc.State(C_path, {
+# 'Q' : Item(status='A ', wc_rev='-', copied='+', repos_rev=2),
+# 'foo' : Item(status='A ', wc_rev='-', copied='+', repos_rev=2),
+# })
+#
+# if svntest.actions.run_and_verify_merge(C_path, '1', '2', F_url,
+# expected_output,
+# expected_disk,
+# expected_status):
+# print "merge failed"
+# return 1
+
+ out,err = svntest.main.run_svn(None, 'merge', '-r1:2', F_url, C_path)
+ if (not re.match("^A.*/A/C/Q$", out[0])
+ or not re.match("^A.*/A/C/Q/bar$", out[1])
+ or not re.match("^A.*/A/C/foo$", out[2])):
+ print "merge failed"
+ return 1
+
+### Can't get this to work either.
+#
+# expected_status.add({
+# 'A/C/Q' : Item(status='A ', wc_rev='-', copied='+', repos_rev='2'),
+# 'A/C/Q/bar' : Item(status='A ', wc_rev='-', copied='+', repos_rev='2'),
+# 'A/C/foo' : Item(status='A ', wc_rev='-', copied='+', repos_rev='2'),
+# })
+# if svntest.actions.run_and_verify_status(wc_dir, expected_status):
+# print "status failed"
+# return 1
+
+ expected_output = svntest.wc.State(wc_dir, {
+ 'A/C/Q' : Item(verb='Adding'),
+ 'A/C/foo' : Item(verb='Adding'),
+ })
+ expected_status = svntest.actions.get_virginal_state(wc_dir, 3)
+ expected_status.tweak(wc_rev=1)
+ expected_status.add({
+ 'A/B/F/Q' : Item(status='_ ', wc_rev=2, repos_rev=3),
+ 'A/B/F/Q/bar' : Item(status='_ ', wc_rev=2, repos_rev=3),
+ 'A/B/F/foo' : Item(status='_ ', wc_rev=2, repos_rev=3),
+ 'A/C/Q' : Item(status='_ ', wc_rev=3, repos_rev=3),
+ 'A/C/Q/bar' : Item(status='_ ', wc_rev=3, repos_rev=3),
+ 'A/C/foo' : Item(status='_ ', wc_rev=3, repos_rev=3),
+ })
+ if svntest.actions.run_and_verify_commit(wc_dir,
+ expected_output,
+ expected_status,
+ None,
+ None, None,
+ None, None,
+ wc_dir):
+ print "commit of merge failed"
+ return 1
+
+#----------------------------------------------------------------------
########################################################################
# Run the tests
@@ -374,6 +483,7 @@
# list all tests here, starting with None:
test_list = [ None,
textual_merges_galore,
+ add_with_history,
# property_merges_galore, # Would be nice to have this.
# tree_merges_galore, # Would be nice to have this.
# various_merges_galore, # Would be nice to have this.
--
Philip Martin
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: diff and merge behaviour
Posted by Ben Collins-Sussman <su...@collab.net>.
Branko Čibej <br...@xbc.nu> writes:
> 1. During the merge, several new files that had appeared on /trunk
> were added to my WC. Well and good; _but_ they were added without
> ancestry, creating a new node. I submit that "svn merge" should
> actually perform a copy when adding or replacing files; this would
> let diff avoid the weirdness I describe below.
Wow, that's a really great idea. I can't believe we never thought of
this before. Can you file an enhancement request for this?
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org