> -----Original Message-----
> From: Stefan Sperling [mailto:stsp_at_elego.de]
> Sent: woensdag 11 maart 2009 11:36
> To: Ryan Schmidt
> Cc: Mark Eichin; users_at_subversion.tigris.org; dev_at_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
Received on 2009-03-11 16:20:44 CET