You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@subversion.apache.org by Stefan Sperling <st...@elego.de> on 2009/03/11 10:35:54 UTC
Re: Merge tracking: MyServer equals Myserver
[Cross-posting to dev@ to get comments on patch.
If you reply, you might want to keep only one of the lists
in Cc, whichever you think suits your reply better.]
On Wed, Mar 11, 2009 at 01:26:42AM -0500, Ryan Schmidt wrote:
> On Mar 10, 2009, at 23:44, Mark Eichin wrote:
>
> > On Tue, Mar 10, 2009 at 9:46 AM, Harald Hoyer wrote:
> >
> >> When checking if merge tracking is to be used, a check is
> >> contained if the adressed repos are the same. Where can I setup
> >> that for example "svn://MyServer/myrepo/trunk" and "svn://Myserver/
> >> myrepo/trunk" are considered a to be the same repo, since the
> >> (case-insensitve) server names are the same.
> >
> > A related problem is that svn://svnserver and
> > svn://svnserver.my.domain are treated as distinct. (Given that
> > repositories have uuids, should the code in question even be *looking*
> > at the dns name?)
>
> Or you could be accessing the same repository via multiple protocols
> (svn:// and http://). I agree it seems reasonable to use the
> repository UUID and not care about the URL.
People, you're right:
1) It was comparing URLs case-sensitively, which is wrong for
the protocol and hostname parts.
2) Checking UUIDs should be enough.
In fact, I'd say even looking at the URLs is really, really wrong.
It looks like the URL compare was meant to be an optimisation
to save a round-trip to the repo. But I'd say this is not safe
because who know what the repository of the day at this particular
URL is. People do silly things sometimes so we'd better play it safe!
So the patch below (against trunk) should fix this.
I still have to run the regression tests with this to see
if it breaks anything (it shouldn't).
Stefan
Index: subversion/libsvn_client/merge.c
==================================================================--- subversion/libsvn_client/merge.c (revision 36480)
+++ subversion/libsvn_client/merge.c (working copy)
@@ -192,7 +192,39 @@ check_scheme_match(svn_wc_adm_access_t *adm_access
return SVN_NO_ERROR;
}
+/* Determine whether the repository the working copy is based on has
+ * the same UUID as the repository pointed to by RA_SESSION.
+ *
+ * CTX is the client context.
+ *
+ * If non-NULL, ENTRY is used to retrieve the UUID of the repository
+ * the working copy is based on. Otherwise, fall back to the UUID of
+ * the repository located at URL WC_REPOS_ROOT.
+ *
+ * Do all allocations in pool.
+ */
+static svn_error_t *
+is_same_repos(svn_boolean_t *same_repos,
+ svn_ra_session_t *ra_session,
+ svn_client_ctx_t *ctx,
+ const svn_wc_entry_t *entry,
+ const char *wc_repos_root,
+ apr_pool_t* pool)
+{
+ const char *source_repos_uuid;
+ const char *wc_repos_uuid;
+ SVN_ERR(svn_ra_get_uuid2(ra_session, &source_repos_uuid, pool));
+ if (entry)
+ wc_repos_uuid = entry->uuid;
+ else
+ SVN_ERR(svn_client_uuid_from_url(&wc_repos_uuid, wc_repos_root,
+ ctx, pool));
+ *same_repos = (strcmp(wc_repos_uuid, source_repos_uuid) == 0);
+
+ return SVN_NO_ERROR;
+}
+
/*-----------------------------------------------------------------------*/
/*** Repos-Diff Editor Callbacks ***/
@@ -6720,22 +6752,9 @@ merge_cousins_and_supplement_mergeinfo(const char
const char *old_url;
svn_boolean_t same_repos;
- if (strcmp(wc_repos_root, source_repos_root) != 0)
- {
- const char *source_repos_uuid;
- const char *wc_repos_uuid;
+ SVN_ERR(is_same_repos(&same_repos, ra_session, ctx, entry, wc_repos_root,
+ pool));
- SVN_ERR(svn_ra_get_uuid2(ra_session, &source_repos_uuid, pool));
- if (entry)
- wc_repos_uuid = entry->uuid;
- else
- SVN_ERR(svn_client_uuid_from_url(&wc_repos_uuid, wc_repos_root,
- ctx, pool));
- same_repos = (strcmp(wc_repos_uuid, source_repos_uuid) == 0);
- }
- else
- same_repos = TRUE;
-
peg_revision.kind = svn_opt_revision_number;
SVN_ERR(svn_ra_get_session_url(ra_session, &old_url, pool));
@@ -6913,22 +6932,9 @@ svn_client_merge3(const char *source1,
SVN_ERR(svn_ra_get_repos_root2(ra_session1, &source_repos_root, sesspool));
/* Do our working copy and sources come from the same repository? */
- if (strcmp(wc_repos_root, source_repos_root) != 0)
- {
- const char *source_repos_uuid;
- const char *wc_repos_uuid;
+ SVN_ERR(is_same_repos(&same_repos, ra_session1, ctx, entry, wc_repos_root,
+ pool));
- SVN_ERR(svn_ra_get_uuid2(ra_session1, &source_repos_uuid, pool));
- if (entry)
- wc_repos_uuid = entry->uuid;
- else
- SVN_ERR(svn_client_uuid_from_url(&wc_repos_uuid, wc_repos_root,
- ctx, pool));
- same_repos = (strcmp(wc_repos_uuid, source_repos_uuid) == 0);
- }
- else
- same_repos = TRUE;
-
/* Unless we're ignoring ancestry, see if the two sources are related. */
if (! ignore_ancestry)
SVN_ERR(svn_client__get_youngest_common_ancestor(&yc_path, &yc_rev,
Re: Merge tracking: MyServer equals Myserver
Posted by Stefan Sperling <st...@elego.de>.
On Wed, Mar 11, 2009 at 01:00:28PM +0100, Harald Hoyer wrote:
> > From: Stefan Sperling [mailto:stsp@elego.de]
> > (BTW you didn't mention which version of Subversion you are using.)
> We plan to use TortoiseSVN 1.5.9 and CollabNetSubversionServer 1.5.5-1. So
> all my investigations took place on these versions. But I already decided to
> postpone the rollout until TortoiseSVN 1.6. has been released. So which
> version of Subversion should be used?
Bert Huijben pointed out that URLs are always normalised in 1.6.
In 1.5 they weren't. So my patch is not needed, and upgrading
to 1.6 will fix your problem.
Stefan
RE: Merge tracking: MyServer equals Myserver
Posted by Harald Hoyer <ha...@t-online.de>.
> From: Stefan Sperling [mailto:stsp@elego.de]
> (BTW you didn't mention which version of Subversion you are using.)
We plan to use TortoiseSVN 1.5.9 and CollabNetSubversionServer 1.5.5-1. So
all my investigations took place on these versions. But I already decided to
postpone the rollout until TortoiseSVN 1.6. has been released. So which
version of Subversion should be used?
Harald
------------------------------------------------------
http://subversion.tigris.org/ds/viewMessage.do?dsForumId=1065&dsMessageId=1307661
To unsubscribe from this discussion, e-mail: [users-unsubscribe@subversion.tigris.org].
Re: Merge tracking: MyServer equals Myserver
Posted by Stefan Sperling <st...@elego.de>.
On Wed, Mar 11, 2009 at 10:35:54AM +0000, Stefan Sperling wrote:
> [Cross-posting to dev@ to get comments on patch.
> If you reply, you might want to keep only one of the lists
> in Cc, whichever you think suits your reply better.]
>
> On Wed, Mar 11, 2009 at 01:26:42AM -0500, Ryan Schmidt wrote:
> > On Mar 10, 2009, at 23:44, Mark Eichin wrote:
> >
> > > On Tue, Mar 10, 2009 at 9:46 AM, Harald Hoyer wrote:
> > >
> > >> When checking if merge tracking is to be used, a check is
> > >> contained if the adressed repos are the same. Where can I setup
> > >> that for example "svn://MyServer/myrepo/trunk" and "svn://Myserver/
> > >> myrepo/trunk" are considered a to be the same repo, since the
> > >> (case-insensitve) server names are the same.
Harald, by the way, this should already work, even without
the patch I posted (see below, I don't know if you got that
mail, you won't have got it unless you are subscribed to the users list).
In the example you presented, the client would compare the URLs
case-sensitively, and since they don't match, it would go on to
check the UUID.
All my patch does is making the client skip comparing URLs altogether,
and go straight to the UUID check.
So if you are seeing a problem where merge tracking is not being
used because the URLs are not the same, my patch does not address it.
Your client should already be checking the UUID.
(BTW you didn't mention which version of Subversion you are using.)
Stefan
> > > A related problem is that svn://svnserver and
> > > svn://svnserver.my.domain are treated as distinct. (Given that
> > > repositories have uuids, should the code in question even be *looking*
> > > at the dns name?)
> >
> > Or you could be accessing the same repository via multiple protocols
> > (svn:// and http://). I agree it seems reasonable to use the
> > repository UUID and not care about the URL.
>
> People, you're right:
>
> 1) It was comparing URLs case-sensitively, which is wrong for
> the protocol and hostname parts.
>
> 2) Checking UUIDs should be enough.
> In fact, I'd say even looking at the URLs is really, really wrong.
> It looks like the URL compare was meant to be an optimisation
> to save a round-trip to the repo. But I'd say this is not safe
> because who know what the repository of the day at this particular
> URL is. People do silly things sometimes so we'd better play it safe!
>
> So the patch below (against trunk) should fix this.
> I still have to run the regression tests with this to see
> if it breaks anything (it shouldn't).
>
> Stefan
>
> Index: subversion/libsvn_client/merge.c
> ==================================================================--- subversion/libsvn_client/merge.c (revision 36480)
> +++ subversion/libsvn_client/merge.c (working copy)
> @@ -192,7 +192,39 @@ check_scheme_match(svn_wc_adm_access_t *adm_access
> return SVN_NO_ERROR;
> }
>
> +/* Determine whether the repository the working copy is based on has
> + * the same UUID as the repository pointed to by RA_SESSION.
> + *
> + * CTX is the client context.
> + *
> + * If non-NULL, ENTRY is used to retrieve the UUID of the repository
> + * the working copy is based on. Otherwise, fall back to the UUID of
> + * the repository located at URL WC_REPOS_ROOT.
> + *
> + * Do all allocations in pool.
> + */
> +static svn_error_t *
> +is_same_repos(svn_boolean_t *same_repos,
> + svn_ra_session_t *ra_session,
> + svn_client_ctx_t *ctx,
> + const svn_wc_entry_t *entry,
> + const char *wc_repos_root,
> + apr_pool_t* pool)
> +{
> + const char *source_repos_uuid;
> + const char *wc_repos_uuid;
>
> + SVN_ERR(svn_ra_get_uuid2(ra_session, &source_repos_uuid, pool));
> + if (entry)
> + wc_repos_uuid = entry->uuid;
> + else
> + SVN_ERR(svn_client_uuid_from_url(&wc_repos_uuid, wc_repos_root,
> + ctx, pool));
> + *same_repos = (strcmp(wc_repos_uuid, source_repos_uuid) == 0);
> +
> + return SVN_NO_ERROR;
> +}
> +
> /*-----------------------------------------------------------------------*/
>
> /*** Repos-Diff Editor Callbacks ***/
> @@ -6720,22 +6752,9 @@ merge_cousins_and_supplement_mergeinfo(const char
> const char *old_url;
> svn_boolean_t same_repos;
>
> - if (strcmp(wc_repos_root, source_repos_root) != 0)
> - {
> - const char *source_repos_uuid;
> - const char *wc_repos_uuid;
> + SVN_ERR(is_same_repos(&same_repos, ra_session, ctx, entry, wc_repos_root,
> + pool));
>
> - SVN_ERR(svn_ra_get_uuid2(ra_session, &source_repos_uuid, pool));
> - if (entry)
> - wc_repos_uuid = entry->uuid;
> - else
> - SVN_ERR(svn_client_uuid_from_url(&wc_repos_uuid, wc_repos_root,
> - ctx, pool));
> - same_repos = (strcmp(wc_repos_uuid, source_repos_uuid) == 0);
> - }
> - else
> - same_repos = TRUE;
> -
> peg_revision.kind = svn_opt_revision_number;
> SVN_ERR(svn_ra_get_session_url(ra_session, &old_url, pool));
>
> @@ -6913,22 +6932,9 @@ svn_client_merge3(const char *source1,
> SVN_ERR(svn_ra_get_repos_root2(ra_session1, &source_repos_root, sesspool));
>
> /* Do our working copy and sources come from the same repository? */
> - if (strcmp(wc_repos_root, source_repos_root) != 0)
> - {
> - const char *source_repos_uuid;
> - const char *wc_repos_uuid;
> + SVN_ERR(is_same_repos(&same_repos, ra_session1, ctx, entry, wc_repos_root,
> + pool));
>
> - SVN_ERR(svn_ra_get_uuid2(ra_session1, &source_repos_uuid, pool));
> - if (entry)
> - wc_repos_uuid = entry->uuid;
> - else
> - SVN_ERR(svn_client_uuid_from_url(&wc_repos_uuid, wc_repos_root,
> - ctx, pool));
> - same_repos = (strcmp(wc_repos_uuid, source_repos_uuid) == 0);
> - }
> - else
> - same_repos = TRUE;
> -
> /* Unless we're ignoring ancestry, see if the two sources are related. */
> if (! ignore_ancestry)
> SVN_ERR(svn_client__get_youngest_common_ancestor(&yc_path, &yc_rev,
>
RE: Merge tracking: MyServer equals Myserver
Posted by Bert Huijben <rh...@sharpsvn.net>.
> -----Original Message-----
> From: Mark Phippard [mailto:markphip@gmail.com]
> Sent: woensdag 11 maart 2009 13:31
> To: Ryan Schmidt; Mark Eichin; users@subversion.tigris.org;
> dev@subversion.tigris.org
> Subject: Re: Merge tracking: MyServer equals Myserver
>
> You should look at this issue:
>
> http://subversion.tigris.org/issues/show_bug.cgi?id=3361
>
> Senthil has been working on this. There is a simplistic patch in
> current trunk and 1.6. But still more to do. I think the issue is
> trying to do it without adding more server calls.
>
> Mark
>
>
>
> On Wed, Mar 11, 2009 at 6:35 AM, Stefan Sperling <st...@elego.de> wrote:
> > [Cross-posting to dev@ to get comments on patch.
> > If you reply, you might want to keep only one of the lists
> > in Cc, whichever you think suits your reply better.]
> >
> > On Wed, Mar 11, 2009 at 01:26:42AM -0500, Ryan Schmidt wrote:
> >> On Mar 10, 2009, at 23:44, Mark Eichin wrote:
> >>
> >> > On Tue, Mar 10, 2009 at 9:46 AM, Harald Hoyer wrote:
> >> >
> >> >> When checking if merge tracking is to be used, a check is
> >> >> contained if the adressed repos are the same. Where can I setup
> >> >> that for example "svn://MyServer/myrepo/trunk" and "svn://Myserver/
> >> >> myrepo/trunk" are considered a to be the same repo, since the
> >> >> (case-insensitve) server names are the same.
In Subversion 1.6 server names are always normalized to lowercase. This resolves this exact problem.
(It doesn't resolve a svn://myserver/Repos vs svn://myserver/rEPOS on Windows though).
Bert
------------------------------------------------------
http://subversion.tigris.org/ds/viewMessage.do?dsForumId=462&dsMessageId=1307887
Re: Merge tracking: MyServer equals Myserver
Posted by Mark Phippard <ma...@gmail.com>.
You should look at this issue:
http://subversion.tigris.org/issues/show_bug.cgi?id=3361
Senthil has been working on this. There is a simplistic patch in
current trunk and 1.6. But still more to do. I think the issue is
trying to do it without adding more server calls.
Mark
On Wed, Mar 11, 2009 at 6:35 AM, Stefan Sperling <st...@elego.de> wrote:
> [Cross-posting to dev@ to get comments on patch.
> If you reply, you might want to keep only one of the lists
> in Cc, whichever you think suits your reply better.]
>
> On Wed, Mar 11, 2009 at 01:26:42AM -0500, Ryan Schmidt wrote:
>> On Mar 10, 2009, at 23:44, Mark Eichin wrote:
>>
>> > On Tue, Mar 10, 2009 at 9:46 AM, Harald Hoyer wrote:
>> >
>> >> When checking if merge tracking is to be used, a check is
>> >> contained if the adressed repos are the same. Where can I setup
>> >> that for example "svn://MyServer/myrepo/trunk" and "svn://Myserver/
>> >> myrepo/trunk" are considered a to be the same repo, since the
>> >> (case-insensitve) server names are the same.
>> >
>> > A related problem is that svn://svnserver and
>> > svn://svnserver.my.domain are treated as distinct. (Given that
>> > repositories have uuids, should the code in question even be *looking*
>> > at the dns name?)
>>
>> Or you could be accessing the same repository via multiple protocols
>> (svn:// and http://). I agree it seems reasonable to use the
>> repository UUID and not care about the URL.
>
> People, you're right:
>
> 1) It was comparing URLs case-sensitively, which is wrong for
> the protocol and hostname parts.
>
> 2) Checking UUIDs should be enough.
> In fact, I'd say even looking at the URLs is really, really wrong.
> It looks like the URL compare was meant to be an optimisation
> to save a round-trip to the repo. But I'd say this is not safe
> because who know what the repository of the day at this particular
> URL is. People do silly things sometimes so we'd better play it safe!
>
> So the patch below (against trunk) should fix this.
> I still have to run the regression tests with this to see
> if it breaks anything (it shouldn't).
>
> Stefan
>
> Index: subversion/libsvn_client/merge.c
> ==================================================================--- subversion/libsvn_client/merge.c (revision 36480)
> +++ subversion/libsvn_client/merge.c (working copy)
> @@ -192,7 +192,39 @@ check_scheme_match(svn_wc_adm_access_t *adm_access
> return SVN_NO_ERROR;
> }
>
> +/* Determine whether the repository the working copy is based on has
> + * the same UUID as the repository pointed to by RA_SESSION.
> + *
> + * CTX is the client context.
> + *
> + * If non-NULL, ENTRY is used to retrieve the UUID of the repository
> + * the working copy is based on. Otherwise, fall back to the UUID of
> + * the repository located at URL WC_REPOS_ROOT.
> + *
> + * Do all allocations in pool.
> + */
> +static svn_error_t *
> +is_same_repos(svn_boolean_t *same_repos,
> + svn_ra_session_t *ra_session,
> + svn_client_ctx_t *ctx,
> + const svn_wc_entry_t *entry,
> + const char *wc_repos_root,
> + apr_pool_t* pool)
> +{
> + const char *source_repos_uuid;
> + const char *wc_repos_uuid;
>
> + SVN_ERR(svn_ra_get_uuid2(ra_session, &source_repos_uuid, pool));
> + if (entry)
> + wc_repos_uuid = entry->uuid;
> + else
> + SVN_ERR(svn_client_uuid_from_url(&wc_repos_uuid, wc_repos_root,
> + ctx, pool));
> + *same_repos = (strcmp(wc_repos_uuid, source_repos_uuid) == 0);
> +
> + return SVN_NO_ERROR;
> +}
> +
> /*-----------------------------------------------------------------------*/
>
> /*** Repos-Diff Editor Callbacks ***/
> @@ -6720,22 +6752,9 @@ merge_cousins_and_supplement_mergeinfo(const char
> const char *old_url;
> svn_boolean_t same_repos;
>
> - if (strcmp(wc_repos_root, source_repos_root) != 0)
> - {
> - const char *source_repos_uuid;
> - const char *wc_repos_uuid;
> + SVN_ERR(is_same_repos(&same_repos, ra_session, ctx, entry, wc_repos_root,
> + pool));
>
> - SVN_ERR(svn_ra_get_uuid2(ra_session, &source_repos_uuid, pool));
> - if (entry)
> - wc_repos_uuid = entry->uuid;
> - else
> - SVN_ERR(svn_client_uuid_from_url(&wc_repos_uuid, wc_repos_root,
> - ctx, pool));
> - same_repos = (strcmp(wc_repos_uuid, source_repos_uuid) == 0);
> - }
> - else
> - same_repos = TRUE;
> -
> peg_revision.kind = svn_opt_revision_number;
> SVN_ERR(svn_ra_get_session_url(ra_session, &old_url, pool));
>
> @@ -6913,22 +6932,9 @@ svn_client_merge3(const char *source1,
> SVN_ERR(svn_ra_get_repos_root2(ra_session1, &source_repos_root, sesspool));
>
> /* Do our working copy and sources come from the same repository? */
> - if (strcmp(wc_repos_root, source_repos_root) != 0)
> - {
> - const char *source_repos_uuid;
> - const char *wc_repos_uuid;
> + SVN_ERR(is_same_repos(&same_repos, ra_session1, ctx, entry, wc_repos_root,
> + pool));
>
> - SVN_ERR(svn_ra_get_uuid2(ra_session1, &source_repos_uuid, pool));
> - if (entry)
> - wc_repos_uuid = entry->uuid;
> - else
> - SVN_ERR(svn_client_uuid_from_url(&wc_repos_uuid, wc_repos_root,
> - ctx, pool));
> - same_repos = (strcmp(wc_repos_uuid, source_repos_uuid) == 0);
> - }
> - else
> - same_repos = TRUE;
> -
> /* Unless we're ignoring ancestry, see if the two sources are related. */
> if (! ignore_ancestry)
> SVN_ERR(svn_client__get_youngest_common_ancestor(&yc_path, &yc_rev,
>
>
--
Thanks
Mark Phippard
http://markphip.blogspot.com/
------------------------------------------------------
http://subversion.tigris.org/ds/viewMessage.do?dsForumId=462&dsMessageId=1307779
Re: Merge tracking: MyServer equals Myserver
Posted by Erik Huelsmann <eh...@gmail.com>.
On Wed, Mar 11, 2009 at 5:22 PM, Branko Cibej <br...@xbc.nu> wrote:
> Stefan Sperling wrote:
>> Yes, that is their problem. Tool used wrong. If I hit my thumb
>> with the hammer, I don't run to the manufacturers and ask them
>> to fix the hammer. It's my own fault because I didn't use the
>> tool right.
>>
>
> Even if I didn't know beforehand, the above would prove conclusively
> that you're not from the US. :-D
>
> -- Brane
>
> P.S.: Sorry ... can't forget the story about a guy successfully suing a
> 'fridge manufacturer for damages when he sprained his back trying to
> carry the 'fridge ... which did not display a warning about carrying
> 'fridges being liable to cause sprained backs... rofl and all that ...
I have the same with the woman who used the oven to dry her cat. Until
she bought a microwave and tried the same. Once...
Bye,
Erik.
------------------------------------------------------
http://subversion.tigris.org/ds/viewMessage.do?dsForumId=462&dsMessageId=1308544
Re: Merge tracking: MyServer equals Myserver
Posted by "C. Michael Pilato" <cm...@collab.net>.
Branko Cibej wrote:
> Stefan Sperling wrote:
>> Yes, that is their problem. Tool used wrong. If I hit my thumb
>> with the hammer, I don't run to the manufacturers and ask them
>> to fix the hammer. It's my own fault because I didn't use the
>> tool right.
>>
>
> Even if I didn't know beforehand, the above would prove conclusively
> that you're not from the US. :-D
Sheesh... welcome to my litigious society.
--
C. Michael Pilato <cm...@collab.net>
CollabNet <> www.collab.net <> Distributed Development On Demand
------------------------------------------------------
http://subversion.tigris.org/ds/viewMessage.do?dsForumId=462&dsMessageId=1308510
Re: Merge tracking: MyServer equals Myserver
Posted by Branko Cibej <br...@xbc.nu>.
Stefan Sperling wrote:
> Yes, that is their problem. Tool used wrong. If I hit my thumb
> with the hammer, I don't run to the manufacturers and ask them
> to fix the hammer. It's my own fault because I didn't use the
> tool right.
>
Even if I didn't know beforehand, the above would prove conclusively
that you're not from the US. :-D
-- Brane
P.S.: Sorry ... can't forget the story about a guy successfully suing a
'fridge manufacturer for damages when he sprained his back trying to
carry the 'fridge ... which did not display a warning about carrying
'fridges being liable to cause sprained backs... rofl and all that ...
------------------------------------------------------
http://subversion.tigris.org/ds/viewMessage.do?dsForumId=462&dsMessageId=1308503
Re: Merge tracking: MyServer equals Myserver
Posted by Stefan Sperling <st...@elego.de>.
On Wed, Mar 11, 2009 at 04:19:19PM +0100, Bert Huijben wrote:
> I don't think this patch simply handles the case where a single repository
> is available as svn://server/repos/ and http://server/svn/repos/ (note the
> /svn/ part)... Which will past through this test anyway. (Same case as a
> patch a few months ago)
I'm not sure I understand what you are saying.
Pass through which test in what way?
Do you mean the patch does not handle the case where two URLs
don't match after the hostname part? I think it does handle it,
and it looks like in this particular piece of code, Subversion
does handle this, with or without my patch. Or am I missing something?
Maybe you are talking about a different problem?
Another code path where we make another similar mistake that
causes Subversion to not see the same repository just because
it has two different URLs pointing to the same repository?
> I'm not sure if we should rely only on the UUID being the same to just
> accept it is the same repository...
Why even have a Universal Unique IDentifier if we're not using it as such?
> I remember a case where somebody just
> duplicated empty repositories instead of creating empty ones. (Well, that is
> his problem anyway...)
Yes, that is their problem. Tool used wrong. If I hit my thumb
with the hammer, I don't run to the manufacturers and ask them
to fix the hammer. It's my own fault because I didn't use the
tool right.
> In most code paths we verify whether the repository is the same by checking
> the UUID.. And this continues to replace this usage by using the uuid as the
> primary key on what a repository is, instead of the Url.
Yes, that is the whole point. What is wrong with that?
A given repository may have more than one URL pointing at
it so using URLs as primary keys does not work.
URLs are just a convenient ways for users to refer to a
repository with a unique UUID, aren't they?
Stefan
RE: Merge tracking: MyServer equals Myserver
Posted by Bert Huijben <rh...@sharpsvn.net>.
> -----Original Message-----
> From: Stefan Sperling [mailto:stsp@elego.de]
> Sent: woensdag 11 maart 2009 11:36
> To: Ryan Schmidt
> Cc: Mark Eichin; users@subversion.tigris.org; dev@subversion.tigris.org
> Subject: Re: Merge tracking: MyServer equals Myserver
>
> [Cross-posting to dev@ to get comments on patch.
> If you reply, you might want to keep only one of the lists
> in Cc, whichever you think suits your reply better.]
>
> On Wed, Mar 11, 2009 at 01:26:42AM -0500, Ryan Schmidt wrote:
> > On Mar 10, 2009, at 23:44, Mark Eichin wrote:
> >
> > > On Tue, Mar 10, 2009 at 9:46 AM, Harald Hoyer wrote:
> > >
> > >> When checking if merge tracking is to be used, a check is
> > >> contained if the adressed repos are the same. Where can I setup
> > >> that for example "svn://MyServer/myrepo/trunk" and "svn://Myserver/
> > >> myrepo/trunk" are considered a to be the same repo, since the
> > >> (case-insensitve) server names are the same.
> > >
> > > A related problem is that svn://svnserver and
> > > svn://svnserver.my.domain are treated as distinct. (Given that
> > > repositories have uuids, should the code in question even be *looking*
> > > at the dns name?)
> >
> > Or you could be accessing the same repository via multiple protocols
> > (svn:// and http://). I agree it seems reasonable to use the
> > repository UUID and not care about the URL.
>
> People, you're right:
>
> 1) It was comparing URLs case-sensitively, which is wrong for
> the protocol and hostname parts.
>
> 2) Checking UUIDs should be enough.
> In fact, I'd say even looking at the URLs is really, really wrong.
> It looks like the URL compare was meant to be an optimisation
> to save a round-trip to the repo. But I'd say this is not safe
> because who know what the repository of the day at this particular
> URL is. People do silly things sometimes so we'd better play it safe!
>
> So the patch below (against trunk) should fix this.
> I still have to run the regression tests with this to see
> if it breaks anything (it shouldn't).
>
> Stefan
>
> Index: subversion/libsvn_client/merge.c
> ==================================================================---
> subversion/libsvn_client/merge.c (revision 36480)
> +++ subversion/libsvn_client/merge.c (working copy)
> @@ -192,7 +192,39 @@ check_scheme_match(svn_wc_adm_access_t *adm_access
> return SVN_NO_ERROR;
> }
>
> +/* Determine whether the repository the working copy is based on has
> + * the same UUID as the repository pointed to by RA_SESSION.
> + *
> + * CTX is the client context.
> + *
> + * If non-NULL, ENTRY is used to retrieve the UUID of the repository
> + * the working copy is based on. Otherwise, fall back to the UUID of
> + * the repository located at URL WC_REPOS_ROOT.
> + *
> + * Do all allocations in pool.
> + */
> +static svn_error_t *
> +is_same_repos(svn_boolean_t *same_repos,
> + svn_ra_session_t *ra_session,
> + svn_client_ctx_t *ctx,
> + const svn_wc_entry_t *entry,
> + const char *wc_repos_root,
> + apr_pool_t* pool)
> +{
> + const char *source_repos_uuid;
> + const char *wc_repos_uuid;
>
> + SVN_ERR(svn_ra_get_uuid2(ra_session, &source_repos_uuid, pool));
> + if (entry)
> + wc_repos_uuid = entry->uuid;
> + else
> + SVN_ERR(svn_client_uuid_from_url(&wc_repos_uuid, wc_repos_root,
> + ctx, pool));
> + *same_repos = (strcmp(wc_repos_uuid, source_repos_uuid) == 0);
> +
> + return SVN_NO_ERROR;
> +}
> +
>
/*-----------------------------------------------------------------------*/
>
> /*** Repos-Diff Editor Callbacks ***/
> @@ -6720,22 +6752,9 @@ merge_cousins_and_supplement_mergeinfo(const char
> const char *old_url;
> svn_boolean_t same_repos;
>
> - if (strcmp(wc_repos_root, source_repos_root) != 0)
> - {
> - const char *source_repos_uuid;
> - const char *wc_repos_uuid;
> + SVN_ERR(is_same_repos(&same_repos, ra_session, ctx, entry,
wc_repos_root,
> + pool));
>
> - SVN_ERR(svn_ra_get_uuid2(ra_session, &source_repos_uuid, pool));
> - if (entry)
> - wc_repos_uuid = entry->uuid;
> - else
> - SVN_ERR(svn_client_uuid_from_url(&wc_repos_uuid, wc_repos_root,
> - ctx, pool));
> - same_repos = (strcmp(wc_repos_uuid, source_repos_uuid) == 0);
> - }
> - else
> - same_repos = TRUE;
> -
> peg_revision.kind = svn_opt_revision_number;
> SVN_ERR(svn_ra_get_session_url(ra_session, &old_url, pool));
>
> @@ -6913,22 +6932,9 @@ svn_client_merge3(const char *source1,
> SVN_ERR(svn_ra_get_repos_root2(ra_session1, &source_repos_root,
sesspool));
>
> /* Do our working copy and sources come from the same repository? */
> - if (strcmp(wc_repos_root, source_repos_root) != 0)
> - {
> - const char *source_repos_uuid;
> - const char *wc_repos_uuid;
> + SVN_ERR(is_same_repos(&same_repos, ra_session1, ctx, entry,
wc_repos_root,
> + pool));
>
> - SVN_ERR(svn_ra_get_uuid2(ra_session1, &source_repos_uuid, pool));
> - if (entry)
> - wc_repos_uuid = entry->uuid;
> - else
> - SVN_ERR(svn_client_uuid_from_url(&wc_repos_uuid, wc_repos_root,
> - ctx, pool));
> - same_repos = (strcmp(wc_repos_uuid, source_repos_uuid) == 0);
> - }
> - else
> - same_repos = TRUE;
> -
> /* Unless we're ignoring ancestry, see if the two sources are related.
*/
> if (! ignore_ancestry)
> SVN_ERR(svn_client__get_youngest_common_ancestor(&yc_path, &yc_rev,
Hi,
I don't think this patch simply handles the case where a single repository
is available as svn://server/repos/ and http://server/svn/repos/ (note the
/svn/ part)... Which will past through this test anyway. (Same case as a
patch a few months ago)
I'm not sure if we should rely only on the UUID being the same to just
accept it is the same repository... I remember a case where somebody just
duplicated empty repositories instead of creating empty ones. (Well, that is
his problem anyway...)
In most code paths we verify whether the repository is the same by checking
the UUID.. And this continues to replace this usage by using the uuid as the
primary key on what a repository is, instead of the Url.
Bert
------------------------------------------------------
http://subversion.tigris.org/ds/viewMessage.do?dsForumId=462&dsMessageId=1308314