subversion-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From s...@apache.org
Subject svn commit: r1201824 - in /subversion/trunk/subversion: libsvn_client/diff.c libsvn_ra/ra_loader.c tests/cmdline/diff_tests.py
Date Mon, 14 Nov 2011 18:59:06 GMT
Author: stsp
Date: Mon Nov 14 18:59:06 2011
New Revision: 1201824

URL: http://svn.apache.org/viewvc?rev=1201824&view=rev
Log:
Fix issue #2873, "svn diff -c N on a file added in revision N does not
create patch".

When adding a file in rN and trying to diff this file in the range -rN-1:N,
a client-side existence-check prevented a diff from being generated
because of a "file does not exist at rN-1" error. The client was overly
cautios here, because the server-side already supports the case where one
of the diff targets doesn't exist. The only requirement imposed by the server
is that the _anchor_ of the diff operation must exist. So we can fix issue
#2873 by choosing an appropriate anchor for the diff operation.

* subversion/libsvn_ra/ra_loader.c
  (svn_ra_do_diff3): Stop asserting that the diff target is a direct
   child of the anchor. This assertion is present (and possibly useful)
   in the update and merge cases. But during a diff operation, a target
   which exists within a newly added tree might require an anchor that
   is not a direct parent.

* subversion/libsvn_client/diff.c
  (check_diff_target_exists): New helper function which checks the existence
   of a URL at a given revision and raises an error if it doesn't exist.
  (diff_prepare_repos_repos): Tolerate a missing target at one side of the
   diff. If one side is missing, try to find a common parent to anchor the
   diff operation on, and adjust the diff target paths relative to this anchor.
   This can only fail due to authz restrictions, since one side of the diff
   is guaranteed to exist (in the worst case the common parent is the
   repository root).

* subversion/tests/cmdline/diff_tests.py
  (diff_targets): Stop expecting an error when diffing a file which was newly
   added in revision N in the range -rN-1:N. Expect to see an 'add' diff for
   the file instead of an error.

Modified:
    subversion/trunk/subversion/libsvn_client/diff.c
    subversion/trunk/subversion/libsvn_ra/ra_loader.c
    subversion/trunk/subversion/tests/cmdline/diff_tests.py

Modified: subversion/trunk/subversion/libsvn_client/diff.c
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_client/diff.c?rev=1201824&r1=1201823&r2=1201824&view=diff
==============================================================================
--- subversion/trunk/subversion/libsvn_client/diff.c (original)
+++ subversion/trunk/subversion/libsvn_client/diff.c Mon Nov 14 18:59:06 2011
@@ -1432,6 +1432,46 @@ check_paths(svn_boolean_t *is_repos1,
   return SVN_NO_ERROR;
 }
 
+/* Raise an error if the diff target URL does not exist at REVISION.
+ * If REVISION does not equal OTHER_REVISION, mention both revisions in
+ * the error message. Use RA_SESSION to contact the repository.
+ * Use POOL for temporary allocations. */
+static svn_error_t *
+check_diff_target_exists(const char *url,
+                         svn_revnum_t revision,
+                         svn_revnum_t other_revision,
+                         svn_ra_session_t *ra_session,
+                         apr_pool_t *pool)
+{
+  svn_node_kind_t kind;
+  const char *session_url;
+
+  SVN_ERR(svn_ra_get_session_url(ra_session, &session_url, pool));
+
+  if (strcmp(url, session_url) != 0)
+    SVN_ERR(svn_ra_reparent(ra_session, url, pool));
+
+  SVN_ERR(svn_ra_check_path(ra_session, "", revision, &kind, pool));
+  if (kind == svn_node_none)
+    {
+      if (revision == other_revision)
+        return svn_error_createf(SVN_ERR_FS_NOT_FOUND, NULL,
+                                 _("Diff target '%s' was not found in the "
+                                   "repository at revision '%ld'"),
+                                 url, revision);
+      else
+        return svn_error_createf(SVN_ERR_FS_NOT_FOUND, NULL,
+                                 _("Diff target '%s' was not found in the "
+                                   "repository at revision '%ld' or '%ld'"),
+                                 url, revision, other_revision);
+     }
+
+  if (strcmp(url, session_url) != 0)
+    SVN_ERR(svn_ra_reparent(ra_session, session_url, pool));
+
+  return SVN_NO_ERROR;
+}
+
 /** Prepare a repos repos diff between PATH_OR_URL1 and
  * PATH_OR_URL2@PEG_REVISION, in the revision range REVISION1:REVISION2.
  * Return URLs and peg revisions in *URL1, *REV1 and in *URL2, *REV2.
@@ -1498,17 +1538,34 @@ diff_prepare_repos_repos(const char **ur
      actual URLs will be. */
   if (peg_revision->kind != svn_opt_revision_unspecified)
     {
-      SVN_ERR(svn_client__repos_locations(url1, NULL,
-                                          url2, NULL,
-                                          *ra_session,
-                                          path_or_url2,
-                                          peg_revision,
-                                          revision1,
-                                          revision2,
-                                          ctx, pool));
-      /* Reparent the session, since *URL2 might have changed as a result
-         the above call. */
-      SVN_ERR(svn_ra_reparent(*ra_session, *url2, pool));
+      svn_error_t *err;
+
+      err = svn_client__repos_locations(url1, NULL,
+                                        url2, NULL,
+                                        *ra_session,
+                                        path_or_url2,
+                                        peg_revision,
+                                        revision1,
+                                        revision2,
+                                        ctx, pool);
+      if (err)
+        {
+          if (err->apr_err == SVN_ERR_CLIENT_UNRELATED_RESOURCES)
+            {
+              /* Don't give up just yet. A missing path might translate
+               * into an addition in the diff. Below, we verify that each
+               * URL exists on at least one side of the diff. */
+              svn_error_clear(err);
+            }
+          else
+            return svn_error_trace(err);
+        }
+      else
+        {
+          /* Reparent the session, since *URL2 might have changed as a result
+             the above call. */
+          SVN_ERR(svn_ra_reparent(*ra_session, *url2, pool));
+        }
     }
 
   /* Resolve revision and get path kind for the second target. */
@@ -1516,11 +1573,6 @@ diff_prepare_repos_repos(const char **ur
            (path_or_url2 == *url2) ? NULL : abspath_or_url2,
            *ra_session, revision2, pool));
   SVN_ERR(svn_ra_check_path(*ra_session, "", *rev2, &kind2, pool));
-  if (kind2 == svn_node_none)
-    return svn_error_createf
-      (SVN_ERR_FS_NOT_FOUND, NULL,
-       _("'%s' was not found in the repository at revision %ld"),
-       *url2, *rev2);
 
   /* Do the same for the first target. */
   SVN_ERR(svn_ra_reparent(*ra_session, *url1, pool));
@@ -1528,18 +1580,84 @@ diff_prepare_repos_repos(const char **ur
            (strcmp(path_or_url1, *url1) == 0) ? NULL : abspath_or_url1,
            *ra_session, revision1, pool));
   SVN_ERR(svn_ra_check_path(*ra_session, "", *rev1, &kind1, pool));
-  if (kind1 == svn_node_none)
-    return svn_error_createf
-      (SVN_ERR_FS_NOT_FOUND, NULL,
-       _("'%s' was not found in the repository at revision %ld"),
-       *url1, *rev1);
+
+  /* Either both URLs must exist at their respective revisions,
+   * or one of them may be missing from one side of the diff. */
+  if (kind1 == svn_node_none && kind2 == svn_node_none)
+    {
+      if (strcmp(*url1, *url2) == 0)
+        return svn_error_createf(SVN_ERR_FS_NOT_FOUND, NULL,
+                                 _("Diff target '%s' was not found in the "
+                                   "repository at revisions '%ld' and '%ld'"),
+                                 *url1, *rev1, *rev2);
+      else
+        return svn_error_createf(SVN_ERR_FS_NOT_FOUND, NULL,
+                                 _("Diff targets '%s and '%s' were not found "
+                                   "in the repository at revisions '%ld' and "
+                                   "'%ld'"),
+                                 *url1, *url2, *rev1, *rev2);
+    }
+  else if (kind1 == svn_node_none)
+    SVN_ERR(check_diff_target_exists(*url1, *rev2, *rev1, *ra_session, pool));
+  else if (kind2 == svn_node_none)
+    SVN_ERR(check_diff_target_exists(*url2, *rev1, *rev2, *ra_session, pool));
 
   /* Choose useful anchors and targets for our two URLs. */
   *anchor1 = *url1;
   *anchor2 = *url2;
   *target1 = "";
   *target2 = "";
-  if ((kind1 == svn_node_file) || (kind2 == svn_node_file))
+  if ((kind1 == svn_node_none) || (kind2 == svn_node_none))
+    {
+      svn_node_kind_t kind;
+      const char *repos_root;
+      const char *new_anchor;
+      svn_revnum_t rev;
+
+      /* The diff target does not exist on one side of the diff.
+       * This can happen if the target was added or deleted within the
+       * revision range being diffed.
+       * However, we don't know how deep within a added/deleted subtree the
+       * diff target is. Find a common parent that exists on both sides of
+       * the diff and use it as anchor for the diff operation.
+       *
+       * ### This can fail due to authz restrictions (like in issue #3242).
+       * ### But it is the only option we have right now to try to get
+       * ### a usable diff in this situation. */
+
+      SVN_ERR(svn_ra_get_repos_root2(*ra_session, &repos_root, pool));
+
+      /* Since we already know that one of the URLs does exist,
+       * look for an existing parent of the URL which doesn't exist. */
+      new_anchor = (kind1 == svn_node_none ? *anchor1 : *anchor2);
+      rev = (kind1 == svn_node_none ? *rev1 : *rev2);
+      do
+        {
+          if (strcmp(new_anchor, repos_root) != 0)
+            {
+              new_anchor = svn_path_uri_decode(svn_uri_dirname(new_anchor,
+                                                               pool),
+                                               pool);
+              if (*base_path)
+                *base_path = svn_dirent_dirname(*base_path, pool);
+            }
+
+          SVN_ERR(svn_ra_reparent(*ra_session, new_anchor, pool));
+          SVN_ERR(svn_ra_check_path(*ra_session, "", rev, &kind, pool));
+
+        }
+      while (kind != svn_node_dir);
+      *anchor1 = *anchor2 = new_anchor;
+      /* 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);
+      SVN_ERR_ASSERT(*target1 && *target2);
+      if (kind1 == svn_node_none)
+        kind1 = svn_node_dir;
+      else
+        kind2 = svn_node_dir;
+    }
+  else if ((kind1 == svn_node_file) || (kind2 == svn_node_file))
     {
       svn_uri_split(anchor1, target1, *url1, pool);
       svn_uri_split(anchor2, target2, *url2, pool);

Modified: subversion/trunk/subversion/libsvn_ra/ra_loader.c
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_ra/ra_loader.c?rev=1201824&r1=1201823&r2=1201824&view=diff
==============================================================================
--- subversion/trunk/subversion/libsvn_ra/ra_loader.c (original)
+++ subversion/trunk/subversion/libsvn_ra/ra_loader.c Mon Nov 14 18:59:06 2011
@@ -876,8 +876,6 @@ svn_error_t *svn_ra_do_diff3(svn_ra_sess
                              void *diff_baton,
                              apr_pool_t *pool)
 {
-  SVN_ERR_ASSERT(svn_path_is_empty(diff_target)
-                 || svn_path_is_single_path_component(diff_target));
   return session->vtable->do_diff(session,
                                   reporter, report_baton,
                                   revision, diff_target,

Modified: subversion/trunk/subversion/tests/cmdline/diff_tests.py
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/tests/cmdline/diff_tests.py?rev=1201824&r1=1201823&r2=1201824&view=diff
==============================================================================
--- subversion/trunk/subversion/tests/cmdline/diff_tests.py (original)
+++ subversion/trunk/subversion/tests/cmdline/diff_tests.py Mon Nov 14 18:59:06 2011
@@ -1263,30 +1263,20 @@ def diff_targets(sbox):
                                                             update_path,
                                                             add_path)
 
-  regex = 'svn: E195012: Unable to find repository location for \'.*\''
-  for line in err_output:
-    if re.match(regex, line):
-      break
-  else:
+  if check_update_a_file(diff_output) or check_add_a_file(diff_output):
     raise svntest.Failure
 
   exit_code, diff_output, err_output = svntest.main.run_svn(1,
                                                             'diff', '-r1:2',
                                                             add_path)
-  for line in err_output:
-    if re.match(regex, line):
-      break
-  else:
+
+  if not check_update_a_file(diff_output) or check_add_a_file(diff_output):
     raise svntest.Failure
 
   exit_code, diff_output, err_output = svntest.main.run_svn(
     1, 'diff', '-r1:2', '--old', parent_path, 'alpha', 'theta')
 
-  regex = 'svn: E160013: \'.*\' was not found in the repository'
-  for line in err_output:
-    if re.match(regex, line):
-      break
-  else:
+  if check_update_a_file(diff_output) or check_add_a_file(diff_output):
     raise svntest.Failure
 
   exit_code, diff_output, err_output = svntest.main.run_svn(



Mime
View raw message