You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@subversion.apache.org by Brian Denny <br...@briandenny.net> on 2003/03/08 20:56:22 UTC
[PATCH] Re: issue #785 (was Re: wanted: nibble-sized coding task)
Okay, here's my first stab at this issue. It passes 'make check',
new test included. (And i tried the new test without the merge-cmd
change to make sure it failed).
It probably should have more tests (i only tested one subcase, so far --
svn merge -rX:Y URL). But AFAICT the existing python tests aren't
super-agressive about testing every possible syntactic variation,
either... let me know what you think.
-brian
Log:
Resolves issue #785:
* merge-cmd.c
(svn_cl__merge): If no target path was specified, try to infer it
from the source path(s).
* merge_tests.py
(merge_without_target): New test function. Merges a file, specifying
a revision range and source URL but no target path.
Index: subversion/clients/cmdline/merge-cmd.c
===================================================================
--- subversion/clients/cmdline/merge-cmd.c (revision 5216)
+++ subversion/clients/cmdline/merge-cmd.c (working copy)
@@ -126,6 +126,25 @@
targetpath = "";
}
+ /* If no targetpath was specified, see if we can infer it from the
+ sourcepaths. */
+ if (! strcmp (targetpath, ""))
+ {
+ char *sp1_basename, *sp2_basename;
+ sp1_basename = svn_path_basename (sourcepath1, pool);
+ sp2_basename = svn_path_basename (sourcepath2, pool);
+
+ if (! strcmp (sp1_basename, sp2_basename))
+ {
+ svn_node_kind_t kind;
+ SVN_ERR (svn_io_check_path (sp1_basename, &kind, pool));
+ if (kind == svn_node_file)
+ {
+ targetpath = sp1_basename;
+ }
+ }
+ }
+
if (opt_state->start_revision.kind == svn_opt_revision_unspecified)
opt_state->start_revision.kind = svn_opt_revision_head;
if (opt_state->end_revision.kind == svn_opt_revision_unspecified)
Index: subversion/tests/clients/cmdline/merge_tests.py
===================================================================
--- subversion/tests/clients/cmdline/merge_tests.py (revision 5216)
+++ subversion/tests/clients/cmdline/merge_tests.py (working copy)
@@ -938,6 +938,97 @@
#----------------------------------------------------------------------
+def merge_without_target (sbox):
+ "merging a file, with no explicitly specified target path"
+
+ if sbox.build():
+ return 1
+
+ wc_dir = sbox.wc_dir
+ # url = os.path.join(svntest.main.test_area_url, sbox.repo_dir)
+
+ # Change mu for revision 2
+ mu_path = os.path.join(wc_dir, 'A', 'mu')
+ mu_text = ""
+ for x in range(2,11):
+ mu_text = mu_text + '\nThis is line ' + `x` + ' in mu'
+ mu_text += "\n"
+ svntest.main.file_append(mu_path, mu_text)
+
+ # Create expected output tree for initial commit
+ expected_output = wc.State(wc_dir, {
+ 'A/mu' : Item(verb='Sending'),
+ })
+
+ # Create expected status tree; all local revisions should be at 1,
+ # but mu should be at revision 2.
+ expected_status = svntest.actions.get_virginal_state(wc_dir, 2)
+ expected_status.tweak(wc_rev=1)
+ expected_status.tweak('A/mu', wc_rev=2)
+
+ # Initial commit.
+ if svntest.actions.run_and_verify_commit (wc_dir,
+ expected_output,
+ expected_status,
+ None,
+ None, None, None, None,
+ wc_dir):
+ return 1
+
+ # Make the "other" working copy
+ other_wc = wc_dir + '.other'
+ svntest.actions.duplicate_dir(wc_dir, other_wc)
+
+ # Now commit some more mods from the original working copy, to
+ # produce revision 3.
+ additional_mu_text = "" # saving mu text changes from previous commit
+ for x in range(2,11):
+ additional_mu_text = additional_mu_text \
+ + '\nThis is additional line ' + `x` + ' in mu'
+ additional_mu_text += "\n"
+ svntest.main.file_append(mu_path, additional_mu_text)
+
+ # Created expected output tree for 'svn ci'
+ expected_output = wc.State(wc_dir, {
+ 'A/mu' : Item(verb='Sending'),
+ })
+
+ # Create expected status tree.
+ expected_status = svntest.actions.get_virginal_state(wc_dir, 3)
+ expected_status.tweak(wc_rev=1)
+ expected_status.tweak('A/mu', wc_rev=3)
+
+ # Commit revision 3.
+ if svntest.actions.run_and_verify_commit(wc_dir,
+ expected_output,
+ expected_status,
+ None,
+ None, None, None, None,
+ wc_dir):
+ return 1
+
+ # Make local mods in wc.other
+ other_mu_path = os.path.join(other_wc, 'A', 'mu')
+ svntest.main.file_append(other_mu_path, additional_mu_text)
+
+ # Try the merge without an explicit target; it should succeed.
+ # Can't use run_and_verify_merge cuz it expects a directory argument.
+ # We don't care about the output anyway; that's tested elsewhere.
+ # If we don't get an error, all is well.
+ mu_url = os.path.join(svntest.main.current_repo_url, 'A', 'mu')
+ was_cwd = os.getcwd()
+ try:
+ os.chdir(os.path.join(other_wc, 'A'))
+ out, err = svntest.main.run_svn(0, 'merge', '-r', '1:3',
+ mu_url)
+ if err:
+ print err
+ return 1
+ finally:
+ os.chdir(was_cwd)
+
+#----------------------------------------------------------------------
+
########################################################################
# Run the tests
@@ -948,6 +1039,7 @@
add_with_history,
delete_file_and_dir,
simple_property_merges,
+ merge_without_target,
# merge_one_file, # See issue #1150.
# property_merges_galore, # Would be nice to have this.
# tree_merges_galore, # Would be nice to have this.
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: [PATCH] Re: issue #785 (was Re: wanted: nibble-sized coding task)
Posted by Ben Collins-Sussman <su...@collab.net>.
Brian Denny <br...@briandenny.net> writes:
> It probably should have more tests (i only tested one subcase, so far --
> svn merge -rX:Y URL)
I think people are bitten far more often by 'svn merge -rX:Y foo.c'.
People hate typing URLs, but almost always are trying to patch a file
sitting in front of them.
Can you add that use-case to your test?
I'll take a look at your patch a little later today. :-)
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: [PATCH] Re: issue #785 (was Re: wanted: nibble-sized coding task)
Posted by Ben Collins-Sussman <su...@collab.net>.
Brian Denny <br...@briandenny.net> writes:
> On Fri, Mar 14, 2003 at 08:33:05AM -0800, Ben Collins-Sussman wrote:
> >
> > Cool, I applied this patch in r5330. Thanks!
> >
>
> cool. :) one thing, in ch08.xml:
>
>
> + <programlisting>merge -r N:M SOURCE [PATH]</programlisting>
> ^^^^
>
> did you mean to change this PATH to WCPATH as well?
Doh, yeah.
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: [PATCH] Re: issue #785 (was Re: wanted: nibble-sized coding task)
Posted by Brian Denny <br...@briandenny.net>.
On Fri, Mar 14, 2003 at 08:33:05AM -0800, Ben Collins-Sussman wrote:
>
> Cool, I applied this patch in r5330. Thanks!
>
cool. :) one thing, in ch08.xml:
+ <programlisting>merge -r N:M SOURCE [PATH]</programlisting>
^^^^
did you mean to change this PATH to WCPATH as well?
-brian
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: [PATCH] Re: issue #785 (was Re: wanted: nibble-sized coding task)
Posted by Ben Collins-Sussman <su...@collab.net>.
Brian Denny <br...@briandenny.net> writes:
> here it is. note that i made a couple changes to the test case in
> addition to your request - i renamed it so that it would be less
> confusing next to the new 'merge_catches_nonexistent_target'; i removed
> an unneeded change-commit cycle; and i verified the contents of the
> merged file.
Cool, I applied this patch in r5330. Thanks!
>
> documentation-wise -- it's kind of a hard 'special case' to document
> completely and correctly without being extremely verbose. i won't be
> offended if you want to edit my explanations.
I did. :-)
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
[PATCH] Re: issue #785 (was Re: wanted: nibble-sized coding task)
Posted by Brian Denny <br...@briandenny.net>.
On Mon, Mar 10, 2003 at 12:16:31PM -0800, Ben Collins-Sussman wrote:
>
> Here's what we need to commit this patch:
>
> * expand the test to try 'svn merge -r X:Y wc-file'
>
> * add new documentation to 'svn help merge' (in main.c)
>
> * add new documentation to chapters 3 and 8 in the Book.
>
> Can you add that stuff to your patch?
here it is. note that i made a couple changes to the test case in
addition to your request - i renamed it so that it would be less
confusing next to the new 'merge_catches_nonexistent_target'; i removed
an unneeded change-commit cycle; and i verified the contents of the
merged file.
documentation-wise -- it's kind of a hard 'special case' to document
completely and correctly without being extremely verbose. i won't be
offended if you want to edit my explanations.
-brian
Log:
Resolves issue #785:
* merge-cmd.c
(svn_cl__merge): If no target path was specified, try to infer it
from the source path(s).
* main.c
(svn_cl__cmd_table): Document the new use case in the output of
'svn help merge'.
* merge_tests.py
(merge_with_implicit_target): New test function. Merges a file,
specifying a revision range and source file/URL but no target path.
* ch04.xml
Explain the conditions under which 'svn merge' can infer a target
path.
* ch08.xml
Document the new use case in the reference section for 'svn merge'.
Add an example.
Index: subversion/clients/cmdline/merge-cmd.c
===================================================================
--- subversion/clients/cmdline/merge-cmd.c (revision 5272)
+++ subversion/clients/cmdline/merge-cmd.c (working copy)
@@ -126,6 +126,25 @@
targetpath = "";
}
+ /* If no targetpath was specified, see if we can infer it from the
+ sourcepaths. */
+ if (! strcmp (targetpath, ""))
+ {
+ char *sp1_basename, *sp2_basename;
+ sp1_basename = svn_path_basename (sourcepath1, pool);
+ sp2_basename = svn_path_basename (sourcepath2, pool);
+
+ if (! strcmp (sp1_basename, sp2_basename))
+ {
+ svn_node_kind_t kind;
+ SVN_ERR (svn_io_check_path (sp1_basename, &kind, pool));
+ if (kind == svn_node_file)
+ {
+ targetpath = sp1_basename;
+ }
+ }
+ }
+
if (opt_state->start_revision.kind == svn_opt_revision_unspecified)
opt_state->start_revision.kind = svn_opt_revision_head;
if (opt_state->end_revision.kind == svn_opt_revision_unspecified)
Index: subversion/clients/cmdline/main.c
===================================================================
--- subversion/clients/cmdline/main.c (revision 5272)
+++ subversion/clients/cmdline/main.c (working copy)
@@ -283,8 +283,13 @@
" 2. In the second form TARGET can be an URL, or it can be a working copy\n"
" path in which case the corresponding URL is used. This URL, at\n"
" revisions N and M, defines the two sources to be compared.\n\n"
- " PATH is the working copy path that will receive the changes.\n"
- " If omitted, a default value of '.' is assumed.\n\n",
+ " PATH is the working copy path that will receive the changes. PATH may\n"
+ " be omitted if:\n\n"
+ " (a) the TARGET (or URLs) refer to a directory; then PATH is taken\n"
+ " to be the current working directory.\n\n"
+ " (b) there is a file in the current working directory whose name\n"
+ " is the same as the basename of TARGET (or both URLs); then\n"
+ " PATH is taken to be this file.\n",
{'r', 'N', 'q', svn_cl__force_opt, svn_cl__dry_run_opt,
svn_cl__merge_cmd_opt, SVN_CL__AUTH_OPTIONS} },
Index: subversion/tests/clients/cmdline/merge_tests.py
===================================================================
--- subversion/tests/clients/cmdline/merge_tests.py (revision 5272)
+++ subversion/tests/clients/cmdline/merge_tests.py (working copy)
@@ -994,6 +994,81 @@
#----------------------------------------------------------------------
+def merge_with_implicit_target (sbox):
+ "merging a file, with no explicitly-specified target path (issue #785)"
+
+ if sbox.build():
+ return 1
+
+ wc_dir = sbox.wc_dir
+
+ # Change mu for revision 2
+ mu_path = os.path.join(wc_dir, 'A', 'mu')
+ orig_mu_text = svntest.tree.get_text(mu_path);
+ added_mu_text = ""
+ for x in range(2,11):
+ added_mu_text = added_mu_text + '\nThis is line ' + `x` + ' in mu'
+ added_mu_text += "\n"
+ svntest.main.file_append(mu_path, added_mu_text)
+
+ # Create expected output tree for initial commit
+ expected_output = wc.State(wc_dir, {
+ 'A/mu' : Item(verb='Sending'),
+ })
+
+ # Create expected status tree; all local revisions should be at 1,
+ # but mu should be at revision 2.
+ expected_status = svntest.actions.get_virginal_state(wc_dir, 2)
+ expected_status.tweak(wc_rev=1)
+ expected_status.tweak('A/mu', wc_rev=2)
+
+ # Initial commit.
+ if svntest.actions.run_and_verify_commit (wc_dir,
+ expected_output,
+ expected_status,
+ None,
+ None, None, None, None,
+ wc_dir):
+ return 1
+
+ # Make the "other" working copy
+ other_wc = wc_dir + '.other'
+ svntest.actions.duplicate_dir(wc_dir, other_wc)
+
+ # Try the merge without an explicit target; it should succeed.
+ # Can't use run_and_verify_merge cuz it expects a directory argument.
+ mu_url = os.path.join(svntest.main.current_repo_url, 'A', 'mu')
+ was_cwd = os.getcwd()
+ try:
+ os.chdir(os.path.join(other_wc, 'A'))
+
+ # merge using URL for sourcepath
+ out, err = svntest.main.run_svn(0, 'merge', '-r', '2:1',
+ mu_url)
+ if err:
+ print err
+ return 1
+
+ # sanity-check resulting file
+ if (svntest.tree.get_text('mu') != orig_mu_text):
+ return 1
+
+ # merge using filename for sourcepath
+ out, err = svntest.main.run_svn(0, 'merge', '-r', '1:2',
+ 'mu')
+ if err:
+ print err
+ return 1
+
+ # sanity-check resulting file
+ if (svntest.tree.get_text('mu') != orig_mu_text + added_mu_text):
+ return 1
+
+ finally:
+ os.chdir(was_cwd)
+
+#----------------------------------------------------------------------
+
########################################################################
# Run the tests
@@ -1004,6 +1079,7 @@
add_with_history,
delete_file_and_dir,
simple_property_merges,
+ merge_with_implicit_target,
merge_catches_nonexistent_target,
# merge_one_file, # See issue #1150.
# property_merges_galore, # Would be nice to have this.
Index: doc/book/book/ch04.xml
===================================================================
--- doc/book/book/ch04.xml (revision 5272)
+++ doc/book/book/ch04.xml (working copy)
@@ -573,8 +573,30 @@
help</command>. For example, <command>svn merge</command>
requires a working-copy path as a target, i.e. a place where it
should apply the tree-changes. If the target isn't specified,
- it assumes you want the changes applied to the current working
- directory. If you want changes applied somewhere else, you'll
+ it will try to assume that you are trying to perform one of the
+ following common operations:</para>
+
+ <orderedlist>
+ <listitem>
+ <para>You want to merge changes all the changes in a
+ directory into your current working directory.</para>
+ </listitem>
+ <listitem>
+ <para>You want to merge the changes in a specific file into
+ a file by the same name which exists in your current working
+ directory.</para>
+ </listitem>
+ </orderedlist>
+
+ <para>If you are merging a directory and haven't specified a
+ target path, <command>svn merge</command> assumes the first
+ case above and tries to apply the changes into your current
+ directory. If you are merging a file, and that file (or a file
+ by the same name) exists in your current working directory,
+ <command>svn merge</command> assumes the second case and tries
+ to apply the changes to the local file.</para>
+
+ <para>If you want changes applied somewhere else, you'll
need to say so:</para>
<screen>
Index: doc/book/book/ch08.xml
===================================================================
--- doc/book/book/ch08.xml (revision 5272)
+++ doc/book/book/ch08.xml (working copy)
@@ -1821,8 +1821,22 @@
two sources to be compared.</para>
<para>PATH is the working copy path that will receive the
- changes. If omitted, a default value of '.' is
- assumed.</para>
+ changes. PATH may be omitted if:</para>
+
+ <orderedlist>
+ <listitem>
+ <para>The TARGET (or URLs) refer to a directory;
+ then PATH is taken to be the current working
+ directory.</para>
+ </listitem>
+
+ <listitem>
+ <para>There is a file in the current working directory
+ whose name is the same as the basename of TARGET
+ (or both URLs); then PATH is taken to be this file.</para>
+ </listitem>
+ </orderedlist>
+
</refsect1>
<refsect1>
<title>Alternate names</title>
@@ -1879,6 +1893,14 @@
...
</screen>
+ <para>To merge changes to a single file:</para>
+
+ <screen>
+$ cd myproj
+$ svn merge -r 30:31 thhgttg.txt
+U thhgttg.txt
+ </screen>
+
</refsect1>
</refentry>
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: [PATCH] Re: issue #785 (was Re: wanted: nibble-sized coding task)
Posted by Brian Denny <br...@briandenny.net>.
On Mon, Mar 10, 2003 at 12:23:22PM -0600, cmpilato@collab.net wrote:
> Ben Collins-Sussman <su...@collab.net> writes:
>
> > Here's what we need to commit this patch:
> >
> > * expand the test to try 'svn merge -r X:Y wc-file'
> >
> > * add new documentation to 'svn help merge' (in main.c)
will do.
>
> > * add new documentation to chapters 3 and 8 in the Book.
>
> I think this is optional. Ben seems to agree.
i'll probably go ahead and do this, too, unless i run into some
unanticipated difficulty with it.
-brian
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: [PATCH] Re: issue #785 (was Re: wanted: nibble-sized coding task)
Posted by cm...@collab.net.
Ben Collins-Sussman <su...@collab.net> writes:
> Here's what we need to commit this patch:
>
> * expand the test to try 'svn merge -r X:Y wc-file'
>
> * add new documentation to 'svn help merge' (in main.c)
+1
> * add new documentation to chapters 3 and 8 in the Book.
I think this is optional. Ben seems to agree.
Developers can certainly feel free to make book changes, but we're not
ready to call that a responsibility of all patch providers (though, it
would be helpful if patch providers would at least point out places in
the book that might need to be adjusted as a result of the patch).
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: [PATCH] Re: issue #785 (was Re: wanted: nibble-sized coding task)
Posted by Ben Collins-Sussman <su...@collab.net>.
Brian Denny <br...@briandenny.net> writes:
> Resolves issue #785:
>
> * merge-cmd.c
> (svn_cl__merge): If no target path was specified, try to infer it
> from the source path(s).
Very simple solution, I like it. :-)
Works for me, too.
>
> * merge_tests.py
> (merge_without_target): New test function. Merges a file, specifying
> a revision range and source URL but no target path.
Here's what we need to commit this patch:
* expand the test to try 'svn merge -r X:Y wc-file'
* add new documentation to 'svn help merge' (in main.c)
* add new documentation to chapters 3 and 8 in the Book.
Can you add that stuff to your patch?
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org