[svn.haxx.se] · SVN Dev · SVN Users · SVN Org · TSVN Dev · TSVN Users · Subclipse Dev · Subclipse Users · this month's index

[PATCH] Re: strange behavior when dealing with copied files repos/wc diffs

From: Charles Acknin <charlesacknin_at_gmail.com>
Date: 2007-09-11 01:10:25 CEST

OK, I've worked some more and here's an even simpler reprod recipe:

% svn cp iota iotacopy
% svn ci
(commits r2)
% svn up -r1
% svn diff -r2
!error here when looking for iotacopy text-base!

It turns out libsvn_wc/diff.c(close_file) doesn't check the
copied-file case (obviously because there's no copyfrom support yet)
and blindly call svn_wc__text_base_path() against a missing path (the
added/copied file), which raises the bug a few lines below. So
basically I did provide file_baton structure with copyfrom_path field
so that we know we're having a new copied file at hand and we're then
able to DTRT.

Here's a patch and its non-reg test coming along with. All 'make
check' tests pass.

[[[
Fix a repos-base diff bug with copied files.

* subversion/libsvn_wc/diff.c
  (file_baton): add copyfrom_path member.
  (make_file_baton): add copyfrom_path argument.
  (svnpatch_open_file, svnpatch_add_file, add_file, open_file): update
  calls to make_file_baton according to the new file_baton structure.
  (close_file): refine test conditions so that when addressing a copied
  file we don't use a missing path and prevent a "no such file" error.

* subversion/test/cmdline/diff_tests.py
  (diff_repos_base_copy): new reg-test against the bug this patch is
  fixing.
]]]

[[[
Index: subversion/libsvn_wc/diff.c
===================================================================
--- subversion/libsvn_wc/diff.c (revision 26511)
+++ subversion/libsvn_wc/diff.c (working copy)
@@ -249,6 +249,9 @@
   /* Set when dealing with svnpatch diff. */
   const char *token;

+ /* The path in the WC the current file was copied from. */
+ const char *copyfrom_path;
+
   apr_pool_t *pool;
 };

@@ -349,6 +352,7 @@
                 svn_boolean_t added,
                 struct dir_baton *parent_baton,
                 const char *token,
+ const char *copyfrom_path,
                 apr_pool_t *pool)
 {
   struct file_baton *file_baton = apr_pcalloc(pool, sizeof(*file_baton));
@@ -361,6 +365,7 @@
   file_baton->path = path;
   file_baton->token = token;
   file_baton->dir_baton = parent_baton;
+ file_baton->copyfrom_path = copyfrom_path;

   /* If the parent directory is added rather than replaced it does not
      exist in the working copy. Determine a working copy path whose
@@ -1341,7 +1346,7 @@
   SVN_ERR(svn_wc_write_cmd(eb->svnpatch_stream, eb->pool,
                            "open-file", "ccc", path, pb->token,
                            token));
- *file_baton = make_file_baton(path, FALSE, pb, token, file_pool);
+ *file_baton = make_file_baton(path, FALSE, pb, token, NULL, file_pool);
   return SVN_NO_ERROR;
 }

@@ -1360,7 +1365,7 @@
   SVN_ERR(svn_wc_write_cmd(eb->svnpatch_stream, eb->pool,
                            "add-file", "ccc(?c)", path, pb->token,
                            token, copyfrom_path));
- *file_baton = make_file_baton(path, TRUE, pb, token, file_pool);
+ *file_baton = make_file_baton(path, TRUE, pb, token, copyfrom_path,
file_pool);
   return SVN_NO_ERROR;
 }

@@ -2074,7 +2079,7 @@
   /* ### TODO: support copyfrom? */

   full_path = svn_path_join(pb->edit_baton->anchor_path, path, file_pool);
- b = make_file_baton(full_path, TRUE, pb, token, file_pool);
+ b = make_file_baton(full_path, TRUE, pb, token, copyfrom_path, file_pool);
   *file_baton = b;

   if (eb->svnpatch_stream && eb->reverse_order)
@@ -2105,7 +2110,7 @@
   const char *token = make_token('c', eb, file_pool);

   full_path = svn_path_join(pb->edit_baton->anchor_path, path, file_pool);
- b = make_file_baton(full_path, FALSE, pb, token, file_pool);
+ b = make_file_baton(full_path, FALSE, pb, token, NULL, file_pool);
   *file_baton = b;

   if (eb->svnpatch_stream)
@@ -2259,10 +2264,16 @@

   /* The repository version of the file is in the temp file we applied
      the BASE->repos delta to. If we haven't seen any changes, it's
- the same as BASE. */
+ the same as BASE. Additionally, if this file was copied there can't
+ be any text-base available and we don't want to display diffs. */
   temp_file_path = b->temp_file_path;
   if (!temp_file_path)
- temp_file_path = svn_wc__text_base_path(b->path, FALSE, b->pool);
+ {
+ if (b->added && b->copyfrom_path)
+ temp_file_path = empty_file;
+ else
+ temp_file_path = svn_wc__text_base_path(b->path, FALSE, b->pool);
+ }

   /* If the file isn't in the working copy (either because it was added
Index: subversion/tests/cmdline/diff_tests.py
===================================================================
--- subversion/tests/cmdline/diff_tests.py (revision 26511)
+++ subversion/tests/cmdline/diff_tests.py (working copy)
@@ -3145,6 +3145,28 @@
   svntest.actions.run_and_verify_svn(None, expected_infinity, [],
                                      'diff', '-rHEAD', '--depth', 'infinity')

+def diff_repos_base_copy(sbox):
+ "repos-base diff with copied file"
+
+ sbox.build()
+ wc_dir = sbox.wc_dir
+ os.chdir(wc_dir)
+
+ # copy a file
+ mu_path = os.path.join('A', 'mu')
+ mucp_path = os.path.join('A', 'mucopy')
+ svntest.main.run_svn(None, 'cp', mu_path, mucp_path)
+
+ # commit r2 and update back to r1
+ svntest.main.run_svn(None, 'ci', '-m', 'log msg')
+ svntest.main.run_svn(None, 'up', '-r1')
+
+ # diff r2 against base
+ # we ought to see no error and an empty output
+ svntest.actions.run_and_verify_svn(None, [], [],
+ 'diff', '-r' , '2:BASE')
+
+
 ########################################################################
 #Run the tests

@@ -3194,6 +3216,7 @@
               diff_in_renamed_folder,
               diff_svnpatch,
               diff_with_depth,
+ diff_repos_base_copy,
               ]

 if __name__ == '__main__':
]]]

BTW, should svn_wc__text_base_path() check for the existence of the
path it returns (and return NULL when missing)? I think it would be
pretty relevant to expect such a function to do so. And this could
also prevent such situations to show up.

Cheers,
Charles

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Received on Tue Sep 11 01:07:02 2007

This is an archived mail posted to the Subversion Dev mailing list.