On Tue, Mar 20, 2012 at 07:16:44PM +0100, Dmitry Pavlenko wrote:
>
> > I expected you'd get an "/directory/subdirectory_at_1 doesn't exist" error.
>
> I was the older (1.6) SVN behaviour
>
> Now "do {} while()" cycle in "diff_prepare_repos_repos" takes parent URL until URL exists in both
> start and end revisions. So maybe the cycle should be run only once not to let '/'.
> Or, that is maybe better, if URL doesn't exist in one of revision, diff should show complete addition
> or complete removal. So maybe this case should be handled separatedly, using a special report.
>
> I a "doesn't exist" error is expected, the efforts in that "do {} while ()" cycle seem to be useless.
>
> To my opinion It would be reasonable to guarantee that all directories are displayed relative to the
> parent of url1 and url2 arguments that exists in both revisions (that is found in that "do {}
> while()" cycle). Or maybe it would be reasonable to display all paths relative to targets (or
> parents --- for diff on files) as you proposed.
Hi Dmitry,
I'm looking into this.
I think all paths should be relative to user-specified targets.
In the case where we show newly added items we need to use parent paths
internally. These ended up being used for diff output, too, which is
wrong in my opinion.
If you want to help you can test and review the patch below. Thanks!
I am currently running tests over all ra layers with this patch and
should have results in a while.
(sorry, no log message yet -- not enough time right now)
Index: subversion/libsvn_client/diff.c
===================================================================
--- subversion/libsvn_client/diff.c (revision 1333046)
+++ subversion/libsvn_client/diff.c (working copy)
@@ -1540,6 +1540,14 @@ resolve_pegged_diff_target_url(const char **resolv
* *TARGET1 and *TARGET2, based on *URL1 and *URL2.
* Set *BASE_PATH corresponding to the URL opened in the new *RA_SESSION
* which is pointing at *ANCHOR1.
+ *
+ * If one of the diff targets does not exist in the repository, diff targets
+ * specified by the user will be adjusted so that a diff can be generated.
+ * However, the resulting paths in diff output can differ from what users
+ * expect to see. If *RELATIVE_TO_DIR is not NULL and does not already
+ * contain a valid path, set it up such that generated diff output meets user
+ * expectations even if targets have to be adjusted.
+ *
* Use client context CTX. Do all allocations in POOL. */
static svn_error_t *
diff_prepare_repos_repos(const char **url1,
@@ -1551,6 +1559,7 @@ diff_prepare_repos_repos(const char **url1,
const char **anchor2,
const char **target1,
const char **target2,
+ const char **relative_to_dir,
svn_ra_session_t **ra_session,
svn_client_ctx_t *ctx,
const char *path_or_url1,
@@ -1713,6 +1722,31 @@ diff_prepare_repos_repos(const char **url1,
}
while (kind != svn_node_dir);
*anchor1 = *anchor2 = new_anchor;
+
+ /* Diff labels must appear relative to previous anchor. */
+ if (relative_to_dir && *relative_to_dir == NULL)
+ {
+ const char *relative_to_url;
+
+ if (kind1 == svn_node_none)
+ {
+ if (kind2 == svn_node_file)
+ svn_uri_split(&relative_to_url, NULL, *url2, pool);
+ else
+ relative_to_url = *url2;
+ }
+ else
+ {
+ if (kind1 == svn_node_file)
+ svn_uri_split(&relative_to_url, NULL, *url1, pool);
+ else
+ relative_to_url = *url1;
+ }
+
+ *relative_to_dir = svn_uri_skip_ancestor(repos_root,
+ relative_to_url, pool);
+ }
+
/* Diff targets must be relative to the new anchor. */
*target1 = svn_uri_skip_ancestor(new_anchor, *url1, pool);
*target2 = svn_uri_skip_ancestor(new_anchor, *url2, pool);
@@ -2372,6 +2406,7 @@ diff_repos_repos(const svn_wc_diff_callbacks4_t *c
/* Prepare info for the repos repos diff. */
SVN_ERR(diff_prepare_repos_repos(&url1, &url2, &base_path, &rev1, &rev2,
&anchor1, &anchor2, &target1, &target2,
+ &callback_baton->relative_to_dir,
&ra_session, ctx, path_or_url1, path_or_url2,
revision1, revision2, peg_revision,
pool));
@@ -2745,7 +2780,7 @@ diff_summarize_repos_repos(svn_client_diff_summari
/* Prepare info for the repos repos diff. */
SVN_ERR(diff_prepare_repos_repos(&url1, &url2, &base_path, &rev1, &rev2,
&anchor1, &anchor2, &target1, &target2,
- &ra_session, ctx,
+ NULL, &ra_session, ctx,
path_or_url1, path_or_url2, revision1, revision2,
peg_revision, pool));
Received on 2012-05-02 19:31:44 CEST