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

Re: Issue #2873, diff for a single added file

From: Martin Hauner <martin.hauner_at_gmx.net>
Date: Sun, 27 Apr 2008 13:52:30 +0200

Hi,

Julian Foad wrote:
> Martin Hauner wrote (on 9th Feb):
>> i am looking at issue 2873
>> (http://subversion.tigris.org/issues/show_bug.cgi?id=2873 ).
>> It is about creating a diff for a single added file. This currently
>> fails with a file doesn't exist in revision error.
>
> Thanks for looking at this, Martin.

Thanks for your comments. Took me a while to work on it again.. I have created
a new patch for discussion on trunk including a new test case.

Initially there were two diff test failing:

12 diff_nonextant_urls
16 diff_targets

I got the first working by adding an additional condition to the path checks in
diff_prepare_repos_repos instead of completly removing the checks.

The second check looks for the error i'm trying to remove by running diff on two
unrelated path, so it should fail now..

Ideally the diff code should somehow recognize that it is running a diff on an
added file and just allow that. But the interesting part happens in the common
svn_client__repos_locations function and i have no better idea than faking the
missing path to skip the error checking to get it working.

Possibly a cleaner solution would be an svn_client__repos_locations_diff??

> Does the test suite pass with this patch?
>
> If so, this might be quite close to being ready.
>
> Thanks.
>
> - Julian

[[[
Fix Issue #2873. Allow diff on a single added file.

* subversion/libsvn_client/client.h
   (svn_client__repos_locations): added new parameter to allow missing
   repository locations.

* subversion/libsvn_client/ra.c
   (svn_client__ra_session_from_path): adjusted
   svn_client__repos_locations call.
   (svn_client__repos_locations): added new parameter to allow missing
   repository locations. Fake missing path if allowed.

* subversion/libsvn_client/diff.c
   (diff_prepare_repos_repos): error on missing node only if both paths
   are unrelated.

* subversion/libsvn_client/copy.c
   (repos_to_wc_copy): adjusted svn_client__repos_locations call.

* subversion/libsvn_client/info.c
   (same_resource_in_head): adjusted svn_client__repos_locations call.

* subversion/libsvn_client/merge.c
   (filter_self_referential_mergeinfo,calculate_remaining_ranges,
   get_full_mergeinfo,get_mergeinfo_walk_cb, normalize_merge_sources):
   adjusted svn_client__repos_locations calls.

* subversion/tests/cmdline/diff_tests.py
   (repo_diff_file): new helper function.
   (diff_a_single_added_file): new test.
   added diff_a_single_added_file test.
]]]

-- 
Martin
Subcommander 2.0.0 Beta 2, 1.2.4 - http://subcommander.tigris.org
a Win32/Unix/MacOSX subversion GUI client & diff/merge tool.

Index: subversion/libsvn_client/client.h
===================================================================
--- subversion/libsvn_client/client.h (revision 30791)
+++ subversion/libsvn_client/client.h (working copy)
@@ -151,6 +151,10 @@
 
    RA_SESSION should be an open RA session pointing at the URL of PATH,
    or NULL, in which case this function will open its own temporary session.
+
+ If must_exist ist set to TRUE an error is returned when the path does not
+ exist in one of the two revisions. If set to FALSE it is ok if path does
+ not exist in one revision.
 
    A NOTE ABOUT FUTURE REPORTING:
 
@@ -176,6 +180,7 @@
                             const svn_opt_revision_t *revision,
                             const svn_opt_revision_t *start,
                             const svn_opt_revision_t *end,
+ const svn_boolean_t must_exist,
                             svn_client_ctx_t *ctx,
                             apr_pool_t *pool);
 
Index: subversion/libsvn_client/info.c
===================================================================
--- subversion/libsvn_client/info.c (revision 30791)
+++ subversion/libsvn_client/info.c (working copy)
@@ -316,7 +316,7 @@
                                     ra_session,
                                     url, &peg_rev,
                                     &start_rev, &end_rev,
- ctx, pool);
+ TRUE, ctx, pool);
   if (err &&
       ((err->apr_err == SVN_ERR_CLIENT_UNRELATED_RESOURCES) ||
        (err->apr_err == SVN_ERR_FS_NOT_FOUND)))
Index: subversion/libsvn_client/merge.c
===================================================================
--- subversion/libsvn_client/merge.c (revision 30791)
+++ subversion/libsvn_client/merge.c (working copy)
@@ -422,6 +422,7 @@
                                                     &peg_rev,
                                                     &rev1_opt,
                                                     &rev2_opt,
+ TRUE,
                                                     merge_b->ctx,
                                                     pool);
                   if (err)
@@ -1676,7 +1677,7 @@
       err = svn_client__repos_locations(&start_url, &start_revision,
                                         NULL, NULL, ra_session, url1,
                                         &pegrev, &requested,
- &unspec, ctx, pool);
+ &unspec, TRUE, ctx, pool);
       if (err)
         {
           if (err->apr_err == SVN_ERR_FS_NOT_FOUND
@@ -1774,7 +1775,7 @@
       SVN_ERR(svn_client__repos_locations(&start_url, &start_revision,
                                           NULL, NULL, ra_session, url,
                                           &pegrev, &requested,
- &unspec, ctx, pool));
+ &unspec, TRUE, ctx, pool));
       /* ### FIXME: Having a low-brain moment. Shouldn't we check
          that START_URL matches our session URL at this point? */
       target_rev = start;
@@ -2893,7 +2894,7 @@
                                                 &end_url, &end_revision,
                                                 wb->ra_session, mergeinfo_url,
                                                 &peg_rev, &rev1_opt, &rev2_opt,
- wb->ctx, pool);
+ TRUE, wb->ctx, pool);
               if (err)
                 {
                   /* We might see any of these errors depending on the RA
@@ -3743,7 +3744,7 @@
                                           NULL, NULL,
                                           ra_session, source_url,
                                           &pegrev, &requested,
- &unspec, ctx, pool));
+ &unspec, TRUE, ctx, pool));
       peg_revnum = youngest_requested;
     }
 
Index: subversion/libsvn_client/ra.c
===================================================================
--- subversion/libsvn_client/ra.c (revision 30791)
+++ subversion/libsvn_client/ra.c (working copy)
@@ -458,7 +458,7 @@
                                       path_or_url, &peg_revision,
                                       /* search range: */
                                       &start_rev, &dead_end_rev,
- ctx, pool));
+ TRUE, ctx, pool));
   good_rev = (svn_opt_revision_t *)new_rev;
 
   /* Make the session point to the real URL. */
@@ -584,6 +584,7 @@
                             const svn_opt_revision_t *revision,
                             const svn_opt_revision_t *start,
                             const svn_opt_revision_t *end,
+ const svn_boolean_t must_exist,
                             svn_client_ctx_t *ctx,
                             apr_pool_t *pool)
 {
@@ -700,13 +701,21 @@
 
   /* We'd better have all the paths we were looking for! */
   start_path = apr_hash_get(rev_locs, &start_revnum, sizeof(svn_revnum_t));
+ end_path = apr_hash_get(rev_locs, &end_revnum, sizeof(svn_revnum_t));
+
+ /* *fake* the second path if it is ok that is does not exist. */
+ if (! must_exist && ! start_path && end_path)
+ start_path = apr_pstrdup(pool, end_path);
+
+ if (! must_exist && start_path && ! end_path)
+ end_path = apr_pstrdup(pool, start_path);
+
   if (! start_path)
     return svn_error_createf
       (SVN_ERR_CLIENT_UNRELATED_RESOURCES, NULL,
        _("Unable to find repository location for '%s' in revision %ld"),
        path, start_revnum);
 
- end_path = apr_hash_get(rev_locs, &end_revnum, sizeof(svn_revnum_t));
   if (! end_path)
     return svn_error_createf
       (SVN_ERR_CLIENT_UNRELATED_RESOURCES, NULL,
Index: subversion/libsvn_client/diff.c
===================================================================
--- subversion/libsvn_client/diff.c (revision 30791)
+++ subversion/libsvn_client/diff.c (working copy)
@@ -1014,7 +1014,7 @@
                                           params->peg_revision,
                                           params->revision1,
                                           params->revision2,
- ctx, pool));
+ FALSE, ctx, pool));
       /* Reparent the session, since drr->url2 might have changed as a result
          the above call. */
       SVN_ERR(svn_ra_reparent(ra_session, drr->url2, pool));
@@ -1025,7 +1025,8 @@
           (&drr->rev2, NULL, ra_session, params->revision2,
            (params->path2 == drr->url2) ? NULL : params->path2, pool));
   SVN_ERR(svn_ra_check_path(ra_session, "", drr->rev2, &kind2, pool));
- if (kind2 == svn_node_none)
+
+ if (!drr->same_urls && kind2 == svn_node_none)
     return svn_error_createf
       (SVN_ERR_FS_NOT_FOUND, NULL,
        _("'%s' was not found in the repository at revision %ld"),
@@ -1037,7 +1038,8 @@
           (&drr->rev1, NULL, ra_session, params->revision1,
            (params->path1 == drr->url1) ? NULL : params->path1, pool));
   SVN_ERR(svn_ra_check_path(ra_session, "", drr->rev1, &kind1, pool));
- if (kind1 == svn_node_none)
+
+ if (!drr->same_urls && kind1 == svn_node_none)
     return svn_error_createf
       (SVN_ERR_FS_NOT_FOUND, NULL,
        _("'%s' was not found in the repository at revision %ld"),
@@ -1303,7 +1305,7 @@
                                           path1,
                                           peg_revision,
                                           revision1, &end,
- ctx, pool));
+ TRUE, ctx, pool));
 
       if (!reverse)
         {
Index: subversion/libsvn_client/copy.c
===================================================================
--- subversion/libsvn_client/copy.c (revision 30791)
+++ subversion/libsvn_client/copy.c (working copy)
@@ -871,7 +871,7 @@
                                           NULL,
                                           pair->src, &pair->src_peg_revision,
                                           &pair->src_op_revision, &dead_end_rev,
- ctx, pool));
+ TRUE, ctx, pool));
 
       /* Get the portions of the SRC and DST URLs that are relative to
          TOP_URL, and URI-decode those sections. */
@@ -1513,7 +1513,7 @@
                                           &pair->src_peg_revision,
                                           &pair->src_op_revision,
                                           &dead_end_rev,
- ctx, iterpool));
+ TRUE, ctx, iterpool));
 
       pair->src_original = pair->src;
       pair->src = apr_pstrdup(pool, src);
Index: subversion/tests/cmdline/diff_tests.py
===================================================================
--- subversion/tests/cmdline/diff_tests.py (revision 30791)
+++ subversion/tests/cmdline/diff_tests.py (working copy)
@@ -377,6 +377,25 @@
   os.chdir(was_cwd)
 
 ######################################################################
+# check a pure repository rev1:rev2 diff on a single file
+
+def repo_diff_file(wc_dir, rev1, rev2, file, check_fn):
+ "check that the given pure repository diff of file is seen"
+
+ was_cwd = os.getcwd()
+ os.chdir(wc_dir)
+
+ exit_code, diff_output, err_output = svntest.main.run_svn(None,
+ 'diff', '-r',
+ `rev2` + ':'
+ + `rev1`,
+ file)
+ if check_fn(diff_output):
+ raise svntest.Failure
+
+ os.chdir(was_cwd)
+
+######################################################################
 # Tests
 #
 
@@ -2945,6 +2964,25 @@
   svntest.actions.run_and_verify_svn(None, [], expected_error,
                                      'diff', '-x', sbox.wc_dir, '-r', '1')
 
+#----------------------------------------------------------------------
+def diff_a_single_added_file(sbox):
+ "diff on a single added file"
+
+ sbox.build()
+
+ was_cwd = os.getcwd()
+ os.chdir(sbox.wc_dir)
+ svntest.main.file_append(os.path.join('A', 'B', 'E', 'theta'), "theta")
+ svntest.main.run_svn(None, 'add', os.path.join('A', 'B', 'E', 'theta'))
+ svntest.main.run_svn(None, 'ci', '-m', 'added single file')
+ svntest.main.run_svn(None, 'up')
+ os.chdir(was_cwd)
+
+ repo_diff_file(sbox.wc_dir, 2, 1, os.path.join('A', 'B', 'E', 'theta'),
+ check_add_a_file)
+
+
+
 ########################################################################
 #Run the tests
 
@@ -2997,7 +3035,8 @@
               diff_backward_repos_wc_copy,
               diff_summarize_xml,
               diff_file_depth_empty,
- diff_wrong_extension_type
+ diff_wrong_extension_type,
+ diff_a_single_added_file
               ]
 
 if __name__ == '__main__':

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe_at_subversion.tigris.org
For additional commands, e-mail: dev-help_at_subversion.tigris.org
Received on 2008-04-27 13:53:09 CEST

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

This site is subject to the Apache Privacy Policy and the Apache Public Forum Archive Policy.