[PATCH] Fix `svn merge' involving revision range and urls

[Kannan Rengarajan]

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1


Current dev-trunk seems to be broken with `svn merge' involving range of
revisions between a url and working copy, especially cases such as `svn
merge -rBASE:HEAD ^/branches', `svn merge -rPREV:COMMITTED ^/branches'
involving the keywords 'svn_opt_revision_base',
'svn_opt_revision_previous', 'svn_opt_revision_committed' when given in
revision range, but works fine when the range of revisions are specified
as numerals. In general, when there is a url argument to `svn merge',
both working copy path and the source url get appended as local absolute
path(eg. /home/kannan/wc/http:/localhost/repo/branches) in 'client'
layer which when passed to further methods finds it as unversioned, thus
causing the breakage. The following patch fixes the above case.

Log:
When `svn merge' receives source as a url, get it's relative part from
the source root url and form a valid, versioned local absolute path.

* subversion/libsvn_client/merge.c
 (normalize_merge_sources): If the source provided is a url, get its
  relative part from the 'source_root_url' and form the local absolute
  path.

Patch by: Kannan R <kannanr@...>

- --
Regards,
Kannan
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.6 (GNU/Linux)
Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org/

iQEVAwUBSvGgSHlTqcY7ytmIAQL9hQgAla/3xWhocYPyu/Ta0ICXu6Wc3PwGGCub
lSezgnzV22Tiu//KI2K+2swHVuVx/680kPkL9XTuvV9xPmvFj6mLZXNhFVqQynHx
W1hWUKaTRaa+z2Esc2n+eJcfK58CKHFX9n0gXCV1rrMedQTIzjCkWSVQiqtWQdzh
+PBVKv0FQwQ90vLdzkQA+FaSfpOgUR9NfSWLYpurCuWu6l070zHJUWrXo2Th0JE+
AK63OA924b+sOYLPgELKqjgQKkki7btcsjlW4SIyAjAhuRi4Pkz3L3frEjtFNNDY
FZ2FL1ElXTTDn8F16jSJmokdrwylQ73BU1dttqAJqWgduIP7Ak9xdw==
=c4fY
-----END PGP SIGNATURE-----

------------------------------------------------------
http://subversion.tigris.org/ds/viewMessage.do?dsForumId=462&dsMessageId=2414438
Index: subversion/libsvn_client/merge.c
===================================================================
--- subversion/libsvn_client/merge.c (revision 40358)
+++ subversion/libsvn_client/merge.c (working copy)
@@ -5939,9 +5939,15 @@
   apr_pool_t *subpool;
   int i;
   youngest_opt_rev.kind = svn_opt_revision_head;
-
-  SVN_ERR(svn_dirent_get_absolute(&source_abspath, source, pool));
-
+  
+  if(svn_path_is_url(source))
+      SVN_ERR(svn_dirent_get_absolute(&source_abspath,
+                                      svn_uri_skip_ancestor
+                                      (source_root_url, source),
+                                      pool));
+  else
+      SVN_ERR(svn_dirent_get_absolute(&source_abspath, source, pool));
+  
   /* Initialize our return variable. */
   *merge_sources_p = apr_array_make(pool, 1, sizeof(merge_source_t *));
 

[Paul Burba-3]

On Wed, Nov 4, 2009 at 10:39 AM, Kannan <kannanr@...> wrote:

> -----BEGIN PGP SIGNED MESSAGE-----
> Hash: SHA1
>
>
> Current dev-trunk seems to be broken with `svn merge' involving range of
> revisions between a url and working copy, especially cases such as `svn
> merge -rBASE:HEAD ^/branches', `svn merge -rPREV:COMMITTED ^/branches'
> involving the keywords 'svn_opt_revision_base',
> 'svn_opt_revision_previous', 'svn_opt_revision_committed' when given in
> revision range,

Hi Kannan,

If your merge source is a URL, what exactly do you expect BASE, PREV,
and COMMITTED to refer to?  PREV, BASE, and COMMITTED can only refer
to a working copy paths.  If your merge source is a working copy path
these will work, but AFAICT this has never produced anything but an
error with URL merge sources.

Paul

> but works fine when the range of revisions are specified
> as numerals. In general, when there is a url argument to `svn merge',
> both working copy path and the source url get appended as local absolute
> path(eg. /home/kannan/wc/http:/localhost/repo/branches) in 'client'
> layer which when passed to further methods finds it as unversioned, thus
> causing the breakage. The following patch fixes the above case.
>
> Log:
> When `svn merge' receives source as a url, get it's relative part from
> the source root url and form a valid, versioned local absolute path.
>
> * subversion/libsvn_client/merge.c
>  (normalize_merge_sources): If the source provided is a url, get its
>  relative part from the 'source_root_url' and form the local absolute
>  path.
>
> Patch by: Kannan R <kannanr@...>
>
> - --
> Regards,
> Kannan
> -----BEGIN PGP SIGNATURE-----
> Version: GnuPG v1.4.6 (GNU/Linux)
> Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org/
>
> iQEVAwUBSvGgSHlTqcY7ytmIAQL9hQgAla/3xWhocYPyu/Ta0ICXu6Wc3PwGGCub
> lSezgnzV22Tiu//KI2K+2swHVuVx/680kPkL9XTuvV9xPmvFj6mLZXNhFVqQynHx
> W1hWUKaTRaa+z2Esc2n+eJcfK58CKHFX9n0gXCV1rrMedQTIzjCkWSVQiqtWQdzh
> +PBVKv0FQwQ90vLdzkQA+FaSfpOgUR9NfSWLYpurCuWu6l070zHJUWrXo2Th0JE+
> AK63OA924b+sOYLPgELKqjgQKkki7btcsjlW4SIyAjAhuRi4Pkz3L3frEjtFNNDY
> FZ2FL1ElXTTDn8F16jSJmokdrwylQ73BU1dttqAJqWgduIP7Ak9xdw==
> =c4fY
> -----END PGP SIGNATURE-----
>
> ------------------------------------------------------
> http://subversion.tigris.org/ds/viewMessage.do?dsForumId=462&dsMessageId=2414438

------------------------------------------------------
http://subversion.tigris.org/ds/viewMessage.do?dsForumId=462&dsMessageId=2415870

[Kannan Rengarajan]

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

Paul Burba wrote:
>> Hi Kannan,
>
>> If your merge source is a URL, what exactly do you expect BASE, PREV,
>> and COMMITTED to refer to?  PREV, BASE, and COMMITTED can only refer
>> to a working copy paths.  If your merge source is a working copy path
>> these will work, but AFAICT this has never produced anything but an
>> error with URL merge sources.

Thank you Paul. Should've taken a closer look. Sorry for the noise.

- --
Thanks & Regards,
Kannan
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.6 (GNU/Linux)
Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org/

iQEVAwUBSvkLa3lTqcY7ytmIAQKOlgf/bLVfWoVDBXObQYP43BxXT7b7vJ9tAIua
yMUZRthqOGpcRwJOiYJBbGwm+FjL8Z4Y/nNQ8JQek9zf/29nzu+EQRT/6Q7F6Gqe
S4ut7I9io0ixJdPxq9CtvN6+KhMpGBPRvvxL86KMOefs+D+JXDerDsuwmw5usRG8
F7Ll+51zt3AenJdJDnaMEJICTR3pZn8gHhXPHiIj2UYR4b0hL5QWQ+RlsyLj1B5A
jpkWWfNvXaMHQpZN42PANUMmvBruEjVNHCg5UGHkVkPXVGueUDoayUg7dB74gtCm
Sg82NXi5PuhJae/Q3DtoBGIv857W24MXiQ37hieXCCBlIb9ZDj9pKA==
=CQbg
-----END PGP SIGNATURE-----

------------------------------------------------------
http://subversion.tigris.org/ds/viewMessage.do?dsForumId=462&dsMessageId=2416023

[Kannan Rengarajan]

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

Paul Burba wrote:
>> Hi Kannan,
>
>> If your merge source is a URL, what exactly do you expect BASE, PREV,
>> and COMMITTED to refer to?  PREV, BASE, and COMMITTED can only refer
>> to a working copy paths.  If your merge source is a working copy path
>> these will work, but AFAICT this has never produced anything but an
>> error with URL merge sources.

It's an invalid case with BASE, PREV, COMMITTED with a merge source as a
URL, but as stated in
http://subversion.tigris.org/ds/viewMessage.do?dsForumId=462&dsMessageId=2414438
when the above merge case happens, error message obtained is like:

"svn:'/home/kannan/wcs/devwc/http:/localhost/repos/devrepo/branch' is
not under version control"

which needs to be changed. The patch attached herewith provides a
customized error message related to the case above.

[[[
Log:
When `svn merge' with either BASE or PREV or COMMITTED as a revision
range and source as a URL is invoked, provide a customized error message
related to the case.

* subversion/libsvn_client/merge.c
 (normalize_merge_sources): Same as above.

Patch by: Kannan R <kannanr@...>
]]]

- --
Thanks & Regards,
Kannan
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.6 (GNU/Linux)
Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org/

iQEVAwUBSv1TaHlTqcY7ytmIAQK+wQf+PrOJGO3J4Rd+loS+jlH/OpAo2N3j6EFH
9iJqlc12ZHfjvjps8VOkfg8mMq4oBKJejDNlJ1KjDjFj1ipo8LgKF/yVZN1wYhH0
AsdUgRTr8L68LAZNgndZcGwisQ7EM3IJyZrYCdoMCpRub+I6SYoCC6oQzdloLDse
wQWDHB+1jidxGgxhJsdyj0LOTOp3dsYWT0a643ILkWWB3W6mA2pMGUAX6HEGS20K
cmUjSXWJf9x8lyM+lZFO4lIC8CgKVik98LCPuyZAv8O5CtZuqsLBV7ZG3CocXYC6
yI06ICZ9gxkro8HXG2SJ0VKjFSbGCSsTzhSR5K7xqbkRNAzvN8kg8w==
=dRl2
-----END PGP SIGNATURE-----

------------------------------------------------------
http://subversion.tigris.org/ds/viewMessage.do?dsForumId=462&dsMessageId=2417531
Index: subversion/libsvn_client/merge.c
===================================================================
--- subversion/libsvn_client/merge.c (revision 40493)
+++ subversion/libsvn_client/merge.c (working copy)
@@ -5973,6 +5973,17 @@
           || (range_end->kind == svn_opt_revision_unspecified))
         return svn_error_create(SVN_ERR_CLIENT_BAD_REVISION, NULL,
                                 _("Not all required revisions are specified"));
+
+      if (svn_path_is_url(source)
+          && (range_start->kind == svn_opt_revision_base
+          || range_end->kind == svn_opt_revision_base
+          || range_start->kind == svn_opt_revision_committed
+          || range_end->kind == svn_opt_revision_committed
+          || range_start->kind == svn_opt_revision_previous
+          || range_end->kind == svn_opt_revision_previous))
+        return svn_error_create(SVN_ERR_CLIENT_BAD_REVISION, NULL,
+                                _("Invalid revision range for a URL merge source"));
+        
       SVN_ERR(svn_client__get_revision_number(&range_start_rev, &youngest_rev,
                                               ctx->wc_ctx, source_abspath,
                                               ra_session, range_start,

[Paul Burba-3]

On Fri, Nov 13, 2009 at 7:39 AM, Kannan <kannanr@...> wrote:

> -----BEGIN PGP SIGNED MESSAGE-----
> Hash: SHA1
>
> Paul Burba wrote:
>>> Hi Kannan,
>>
>>> If your merge source is a URL, what exactly do you expect BASE, PREV,
>>> and COMMITTED to refer to?  PREV, BASE, and COMMITTED can only refer
>>> to a working copy paths.  If your merge source is a working copy path
>>> these will work, but AFAICT this has never produced anything but an
>>> error with URL merge sources.
>
> It's an invalid case with BASE, PREV, COMMITTED with a merge source as a
> URL, but as stated in
> http://subversion.tigris.org/ds/viewMessage.do?dsForumId=462&dsMessageId=2414438
> when the above merge case happens, error message obtained is like:
>
> "svn:'/home/kannan/wcs/devwc/http:/localhost/repos/devrepo/branch' is
> not under version control"
>
> which needs to be changed. The patch attached herewith provides a
> customized error message related to the case above.

Hi Kannan,

I agree, that error message, while true, is not very helpful.  I
committed your patch in r40504.

I made a few minor changes:

1) I tweaked the indentation on the if statement slightly

2) I changed the wording of the error message to be more specific.

3) I edited your log message.  We try to keep the first line of the
log message under 80 characters so when it flashes by on the #svn-dev
channel and IRC the people there can get an idea what the change is
about -- see http://svn.collab.net/repos/svn/trunk/www/hacking.html#log-messages.
 This is not always possible of course, but we try.

Thanks,

Paul

------------------------------------------------------
http://subversion.tigris.org/ds/viewMessage.do?dsForumId=462&dsMessageId=2417717