Den mån 13 nov. 2023 kl 13:41 skrev Daniel Sahlberg < daniel.l.sahlb...@gmail.com>:
> Den mån 13 nov. 2023 kl 13:26 skrev Ontonator <ontona...@me.com>: > >> Hi, >> >> I've experienced an issue with merging two working copy paths in which >> the behaviour of `svn` doesn't seem to match up with the documentation. >> >> My system Subversion version is: >> >> > svn, version 1.14.2 (r1899510) >> >> > compiled Oct 23 2023, 15:43:14 on x86_64-pc-linux-gnu >> >> but I have also reproduced it on revision 1913725 built from source. >> >> The only reference I could find to this issue was this users@ mailing >> list post from almost a decade ago with no responses: >> >> https://lists.apache.org/thread/16vf8rr4w7nfkbjlsgfrnrfh0ly4mk41 >> >> The relevant sections of the documentation are (from `svn help merge`): >> >> > usage: 1. merge SOURCE[@REV] [TARGET_WCPATH] >> >> > (the 'complete' merge) >> >> > 2. merge [-c M[,N...] | -r N:M ...] SOURCE[@REV] [TARGET_WCPATH] >> >> > (the 'cherry-pick' merge) >> >> > 3. merge SOURCE1[@REV1] SOURCE2[@REV2] [TARGET_WCPATH] >> >> > (the '2-URL' merge) >> >> and (regarding form 1): >> >> > SOURCE is usually a URL. The optional '@REV' specifies both the peg >> >> > revision of the URL and the latest revision that will be considered >> >> > for merging; if REV is not specified, the HEAD revision is >> assumed. If >> >> > SOURCE is a working copy path, the corresponding URL of the path is >> >> > used, and the default value of 'REV' is the base revision (usually >> the >> >> > revision last updated to). >> >> However, running a command like `svn merge a b` results in: >> >> > svn: E195002: Invalid merge source 'a'; a working copy path can only be >> used with a repository revision (a number, a date, or head) >> >> I have a reproduction `sh` script: >> >> #! /usr/bin/env sh >> >> set -e >> >> mkdir scratch >> >> cd scratch >> >> svnadmin create repos >> >> svn checkout "file://$(pwd)/repos" working >> >> cd working >> >> svn mkdir ^/a --message='Created a' >> >> svn copy ^/a ^/b --message='Created b' >> >> svn update >> >> set +e >> >> printf '\n=== Merging with two working copy paths (explicit target) ===\n' >> >> svn merge a b >> >> printf '\n=== Merging with two working copy paths (inside directory >> target) ===\n' >> >> cd b >> >> svn merge ../a . >> >> cd .. >> >> printf '\n=== Merging with a single working copy path (inferred target) >> ===\n' >> >> cd b >> >> svn merge ../a >> >> cd .. >> >> (End of reproduction) >> >> The output of this script is: >> >> > Checked out revision 0. >> >> > Committing transaction... >> >> > Committed revision 1. >> >> > Committing transaction... >> >> > Committed revision 2. >> >> > Updating '.': >> >> > A a >> >> > A b >> >> > Updated to revision 2. >> >> > >> >> > === Merging with two working copy paths (explicit target) === >> >> > svn: E195002: Invalid merge source 'a'; a working copy path can only be >> used with a repository revision (a number, a date, or head) >> >> > >> >> > === Merging with two working copy paths (inside directory target) === >> >> > svn: E195002: Invalid merge source '../a'; a working copy path can only >> be used with a repository revision (a number, a date, or head) >> >> > >> >> > === Merging with a single working copy path (inferred target) === >> >> > --- Recording mergeinfo for merge of r2 into '.': >> >> > U . >> >> As can be seen in the reproduction, with an explicitly specified >> `TARGET_WCPATH` there is an error, but when it is omitted, it is assumed to >> be `.` and succeeds. >> >> From a brief search of the code, I believe the error lies at >> `subversion/svn/merge-cmd.c:334` (I have revision 1913725 checked out). For >> the lazy, it looks like: >> >> > if (targets->nelts <= 1) >> >> > { >> >> > two_sources_specified = FALSE; >> >> > } >> >> > else if (targets->nelts == 2) >> >> > { >> >> > if (svn_path_is_url(sourcepath1) && >> !svn_path_is_url(sourcepath2)) // <-- This one >> >> > two_sources_specified = FALSE; >> >> > } >> >> In particular, it checks that `sourcepath1` is a URL, which doesn't seem >> right to me. It seems like this code was introduced in revision 868224, >> whose log message makes no mention of intending to change this behaviour >> (although I can't verify that the behaviour was introduced here as it uses >> Python 2 as part of the build process). >> >> While I can't be sure that this behaviour is not intended, it does not >> match the documentation, so at least one of the implementation and the >> documentation must be wrong. I would assume that there is an error in the >> implementation given the inconsistency between passing the second argument >> explicitly vs implicitly. >> >> I haven't reported any issues to Subversion before, so please tell me if >> there is anything I have neglected to do or should have done differently. >> > > Thank you for your report, it is excellent, especially the reproduction > script is super useful. > > I've only skimmed the details but I agree with you that it looks like the > behaviour doesn't match the documentation. I'll try to dig down into the > history of this code, in particular how the sourcepath[12] and > two_sources_specified is used but it might take a few days before I find > the time to do this. > The actual error message is raised in ensure_wc_path_has_repo_revision, with a comment explaining the reasoning: /* Catch 'svn merge wc_path1 wc_path2 [target]' without explicit revisions--since it ignores local modifications it may not do what the user expects. That is, it doesn't read from the WC itself, it reads from the WC's URL. Forcing the user to specify a repository revision should avoid any confusion. */ SVN_ERR(ensure_wc_path_has_repo_revision(sourcepath1, &first_range_start, pool)); SVN_ERR(ensure_wc_path_has_repo_revision(sourcepath2, &first_range_end, pool)); I havn't yet looked at the test case but I assume if a revision is specified (svn merge a@2 b) it would work. It still looks inconsistent that it works with an implicit target (the third test case) so something probably has to change further down.. Out of time for now but I'll try to continue looking. Kind regards, Daniel
