You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@subversion.apache.org by "Julian Foad (Jira)" <ji...@apache.org> on 2021/04/07 13:29:00 UTC

[jira] [Comment Edited] (SVN-4874) Avoid foreign repository merge if repository UUIDs match

    [ https://issues.apache.org/jira/browse/SVN-4874?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17316332#comment-17316332 ] 

Julian Foad edited comment on SVN-4874 at 4/7/21, 1:28 PM:
-----------------------------------------------------------

Updated patch: with tests for all cases: [^i4847-merge-foreign-repos-uuid-4.patch]

Committed in revision 1888474, [https://svn.apache.org/r1888474]


was (Author: julianfoad):
Updated patch: with tests for all cases: [^i4847-merge-foreign-repos-uuid-4.patch]

> Avoid foreign repository merge if repository UUIDs match
> --------------------------------------------------------
>
>                 Key: SVN-4874
>                 URL: https://issues.apache.org/jira/browse/SVN-4874
>             Project: Subversion
>          Issue Type: Bug
>          Components: libsvn_client
>    Affects Versions: 1.14.1, 1.10.7
>            Reporter: Julian Foad
>            Assignee: Julian Foad
>            Priority: Major
>         Attachments: i4847-merge-foreign-repos-uuid-3.patch, i4847-merge-foreign-repos-uuid-4.patch
>
>
> SUMMARY
> Subversion determines a "foreign repository merge" case whenever the source and target repository root URLs differ.
> Different repository root URLs legitimately can point to the same repository in some cases, and in other cases can point to mirrored or replicated repository instances that behave as equivalent for the given purposes.  In any of these cases let's call these "equivalent" repositories.
> It has been observed in practice, and seems likely in other cases, that a user will sometimes use a different URL for an equivalent repository unintentionally, or unaware that it makes a difference.  In these cases, if Subversion considers this a foreign repository merge, this silently produces unexpected behaviour of the merge, for example turning copies into plain adds.  Unexpected behaviour can be harmful.
> We propose to resolve this problem by making Subversion raise an error if the repository root URLs differ but their UUIDs match.  This seems the best solution, considering the pros and cons and the history of the current behaviour.
> A fuller description follows.
>  
> FOREIGN REPOSITORY OR EQUIVALENT REPOSITORY
> There are two scenarios where repository root URLs may legitimately differ while pointing to an equivalent repository:
>  * different URLs pointing to the same resource, e.g. "http:" and "https:" variants
>  * replicated/mirrored repositories
> (Throughout this text, in talking about whether URLs differ we are always talking about the repository root URL, ignoring the path-within-repository at the end of the URL.)
> In the first case, different URLs point to the same repository instance and so the repository UUIDs are necessarily identical.  In the second case, the URLs point to different repository instances that are assumed to logically equivalent and interchangeable, at least for the purpose at hand of reading the merge source diff from them.  Their repository UUIDs should be identical.
> (As well as the repository UUID, it should be noted that each repository may also have a per-instance UUID.  Very old repository versions didn't have these.  The per-instance UUIDs should differ between equivalent instances of a repository.  In the context of merging these per-instance UUIDs are not examined, and are not relevant to the discussion.)
> Subversion determines whether the merge source is a "foreign" repository relative to the target WC's repository, according to checks that have in past versions been based on URL and/or UUID comparisons.  (See HISTORY.)
> A merge behaves differently if Subversion considers the source is a "foreign" repository: at least merge tracking is disabled and "copy" operations in the source are turned into plain "add" operations in the target.
>  
> RATIONALE
>  In a scenario where different URLs point to an equivalent repository, a user might check out a working copy using one URL and then give a different repository root URL in the merge source. There is no single "correct" URL to represent the single logical repository in these scenarios, so it is not reasonable to put the responsibility on the user always to be consistent or else face silently unexpected behaviour.
> Therefore we do not want Subversion to choose "foreign repository" merge behaviour when the merge source and target URLs differ but UUIDs match.
> We could resolve this problem by making Subversion consider the merge source and target repositories as being equivalent if their URLs differ and UUIDs match.  This was the case in Subversion 1.6 and 1.7 (see HISTORY section).  However, changing this again now would itself be a silent behaviour change.  Any users intentionally or unintentionally relying on the current behaviour would be affected.
> This scenario of giving a different URL for an equivalent repository is expected to be a rare case.  There is no known good reason for it to be supported, since it should always be feasible for the user to use a source repository root URL that matches that of the target WC.
> Instead, we propose Subversion raise an error if the repository root URLs differ but their UUIDs match.
> Advantages of erroring out in this case:
>  * prevents the bad behaviour
>  * does not introduce a silent behavioural change
>  * still allows usual same-repository merge cases (same URL)
>  * still allows usual foreign-repository merge cases (different URL, different UUID)
> Possible down-sides are addressed in their own section, POSSIBLE DOWN-SIDES.
>  
> TWO-URL MERGE SOURCE URLs
> In the "two-URL" form of merge, two source URLs are given.  Subversion (since 1.6.2) checks the UUIDs of the repositories accessed by these to determine whether they point to the same repository, and errors out if they do not.
> Just as with the source-target check, a high level rationale would say this logic is correct, but in practice the implementation is not prepared to support it. There was already a comment in the source code indicating this check is probably not robust enough. We must require identical repository root URLs for the two source locations to ensure subsequent logic will work correctly. For example, the very next logic after this source1-source2 comparison is the source-target comparison, and that compares the target only with source1 on the assumption that the two source repository root URLs are identical.
> As this source1-source2 check is of the same form as the source-target check, using the same subroutines, and the concerns about it are basically the same, it makes sense to change it at the same time.
> Therefore let us change this too: error out if source1 and source2 repository root URLs differ.
>  
> HISTORY
> The history of checking for "foreign repository" merge (source repository is not the target repository) in any merge.
> Since Subversion 1.0.0: URL comparison.
> Since Subversion 1.6.0: UUID comparison ( [https://svn.apache.org/r875700] , 2009-02-02).
> Since Subversion 1.8.0: URL comparison (UUID comparison accidentally broken by [https://svn.apache.org/r1199840] , 2011-11-09).
> In carefully reviewing the history of the existing checks, I was embarrassed to discover I broke the previously intended behaviour in r1199840, in the lead-up to 1.8.0.  It has remained that way since.  There are no regression tests involving unequal URLs pointing to equivalent repositories, as far as I can see.
> The history of checking that the two source URLs in a two-URL merge point to the same repository (given for context; no change is proposed):
> Since Subversion 1.0.0: no explicit check for the two source URLs in 2-URL merge (and possible wrong behaviour if pointing to different repositories).
> Since Subversion 1.6.2: UUID comparison used for the two source URLs in 2-URL merge ( [https://svn.apache.org/r877593] ); error out if the UUIDs differ.
>  
> POSSIBLE DOWN-SIDES
> Possible down-sides with this proposed change in certain cases include:
>  * Case: equivalent repositories; the given source repository URL is faster to access than the repository associated with the target WC.
>  ** The user will be unable to use the faster source URL for the merge, without a work-around.
>  ** Work-around: first "svn relocate" the WC URL to match the desired URL, then merge with this source URL, then relocate the WC back again if desired.
>  * Case: equivalent repositories; the given source URL is accessible and the URL associated with the target WC is (for the time being) not accessible.
>  ** The user will be unable to perform the merge without a work-around.
>  ** The "relocate" work-around can be used.
>  * Case: user expects a foreign-repository merge, but the other repository erroneously has the same UUID.
>  ** This UUID misconfiguration can reasonably be expected to occur in real life.  For example, if an administrator creates a new repository by copying an empty repository instead of "svnadmin create".
>  ** After the proposed change, the attempted foreign-repository merge would error out, where before it would have proceeded.
>  ** As foreign repository merges are not well supported in Subversion, they are relatively uncommon, and so the combination with UUID misconfiguration is expected to be rare.  If the error message is clear and is seen by the user, this should alert them to the misconfiguration.  An admin would need to fix it.
> For completeness in considering all edge cases, the following case is unaffected:
>  * Case: user attempts a merge using a different source URL of an equivalent repository that erroneously has a different UUID:
>  ** This UUID misconfiguration can reasonably be expected to occur in real life.
>  ** Both before and after the proposed change, the attempted merge would be treated as a foreign repository merge.
>  ** This case is unaffected by the proposal.
>  



--
This message was sent by Atlassian Jira
(v8.3.4#803005)