Return-Path: X-Original-To: apmail-subversion-commits-archive@minotaur.apache.org Delivered-To: apmail-subversion-commits-archive@minotaur.apache.org Received: from mail.apache.org (hermes.apache.org [140.211.11.3]) by minotaur.apache.org (Postfix) with SMTP id 848C410DA4 for ; Tue, 18 Feb 2014 12:30:23 +0000 (UTC) Received: (qmail 98156 invoked by uid 500); 18 Feb 2014 12:30:23 -0000 Delivered-To: apmail-subversion-commits-archive@subversion.apache.org Received: (qmail 98054 invoked by uid 500); 18 Feb 2014 12:30:21 -0000 Mailing-List: contact commits-help@subversion.apache.org; run by ezmlm Precedence: bulk List-Help: List-Unsubscribe: List-Post: List-Id: Reply-To: dev@subversion.apache.org Delivered-To: mailing list commits@subversion.apache.org Received: (qmail 98042 invoked by uid 99); 18 Feb 2014 12:30:19 -0000 Received: from athena.apache.org (HELO athena.apache.org) (140.211.11.136) by apache.org (qpsmtpd/0.29) with ESMTP; Tue, 18 Feb 2014 12:30:19 +0000 X-ASF-Spam-Status: No, hits=-2000.0 required=5.0 tests=ALL_TRUSTED X-Spam-Check-By: apache.org Received: from [140.211.11.4] (HELO eris.apache.org) (140.211.11.4) by apache.org (qpsmtpd/0.29) with ESMTP; Tue, 18 Feb 2014 12:30:18 +0000 Received: from eris.apache.org (localhost [127.0.0.1]) by eris.apache.org (Postfix) with ESMTP id 0870523888D7; Tue, 18 Feb 2014 12:29:58 +0000 (UTC) Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit Subject: svn commit: r1569290 - in /subversion/trunk/subversion: libsvn_client/diff.c tests/cmdline/merge_authz_tests.py Date: Tue, 18 Feb 2014 12:29:57 -0000 To: commits@subversion.apache.org From: rhuijben@apache.org X-Mailer: svnmailer-1.0.9 Message-Id: <20140218122958.0870523888D7@eris.apache.org> X-Virus-Checked: Checked by ClamAV on apache.org Author: rhuijben Date: Tue Feb 18 12:29:57 2014 New Revision: 1569290 URL: http://svn.apache.org/r1569290 Log: Resolve the diff issue identified in r1569265, and extend the regression test to show why we do anchor the operation a bit higher than the diff. * subversion/libsvn_client/diff.c (diff_prepare_repos_repos): Detect the case where we try to parent the session on an unreadable ancestor and avoid this case by reparenting to the node itself. * subversion/tests/cmdline/merge_authz_tests.py (diff_unauth_parent): Remove XFail and extend test to ensure that we show as much of diffs as possible given the rights a user has. Modified: subversion/trunk/subversion/libsvn_client/diff.c subversion/trunk/subversion/tests/cmdline/merge_authz_tests.py Modified: subversion/trunk/subversion/libsvn_client/diff.c URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_client/diff.c?rev=1569290&r1=1569289&r2=1569290&view=diff ============================================================================== --- subversion/trunk/subversion/libsvn_client/diff.c (original) +++ subversion/trunk/subversion/libsvn_client/diff.c Tue Feb 18 12:29:57 2014 @@ -1521,12 +1521,38 @@ diff_prepare_repos_repos(const char **ur if (strcmp(*url1, repos_root_url) != 0 && strcmp(*url2, repos_root_url) != 0) { + svn_node_kind_t ignored_kind; + svn_error_t *err; svn_uri_split(anchor1, target1, *url1, pool); svn_uri_split(anchor2, target2, *url2, pool); if (*base_path && (*kind1 == svn_node_file || *kind2 == svn_node_file)) *base_path = svn_dirent_dirname(*base_path, pool); SVN_ERR(svn_ra_reparent(*ra_session, *anchor1, pool)); + + /* We might not have the necessary rights to read the root now. + (It is ok to pass a revision here where the node doesn't exist) */ + err = svn_ra_check_path(*ra_session, "", *rev1, &ignored_kind, pool); + + if (err && (err->apr_err == SVN_ERR_RA_DAV_FORBIDDEN + || err->apr_err == SVN_ERR_RA_NOT_AUTHORIZED)) + { + svn_error_clear(err); + + /* Ok, lets undo the reparent... + + We can't report replacements this way, but at least we can + report changes on the descendants */ + + *anchor1 = svn_path_url_add_component2(*anchor1, *target1, pool); + *anchor2 = svn_path_url_add_component2(*anchor2, *target2, pool); + *target1 = ""; + *target2 = ""; + + SVN_ERR(svn_ra_reparent(*ra_session, *anchor1, pool)); + } + else + SVN_ERR(err); } return SVN_NO_ERROR; Modified: subversion/trunk/subversion/tests/cmdline/merge_authz_tests.py URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/tests/cmdline/merge_authz_tests.py?rev=1569290&r1=1569289&r2=1569290&view=diff ============================================================================== --- subversion/trunk/subversion/tests/cmdline/merge_authz_tests.py (original) +++ subversion/trunk/subversion/tests/cmdline/merge_authz_tests.py Tue Feb 18 12:29:57 2014 @@ -737,28 +737,162 @@ def reintegrate_fails_if_no_root_access( None, True, True, '--reintegrate', A_path) -# ### Replace these XFails with @Skip(svntest.main.is_ra_type_file) when fixed. -@XFail(svntest.main.is_ra_type_dav) -@XFail(svntest.main.is_ra_type_svn) def diff_unauth_parent(sbox): "diff directory without reading parent" sbox.build(create_wc=False) + # Create r2: Change A a bit svntest.actions.run_and_verify_svnmucc(None, None, [], 'propset', 'k', 'v', sbox.repo_url + '/A', '-m', 'set prop') + # Create r3 Mark E and G + svntest.actions.run_and_verify_svnmucc(None, None, [], + 'propset', 'this-is', 'E', + sbox.repo_url + '/A/B/E', + 'propset', 'this-is', 'G', + sbox.repo_url + '/A/D/G', + '-m', 'set prop') + + # Create r4: Replace A/B/E with A/D/G + svntest.actions.run_and_verify_svnmucc(None, None, [], + 'rm', sbox.repo_url + '/A/B/E', + 'cp', '3', sbox.repo_url + '/A/D/G', + sbox.repo_url + '/A/B/E', + '-m', 'replace A/B/E') + + if is_ra_type_svn() or is_ra_type_dav(): write_restrictive_svnserve_conf(sbox.repo_dir) write_authz_file(sbox, {"/" : "* =", - "/A" : "* = rw"}) + "/A" : "* = rw"}) - svntest.actions.run_and_verify_svn(None, None, [], - 'diff', sbox.repo_url + '/A', - '-r', '1:2') + # Diff the property change + expected_output = [ + 'Index: .\n', + '===================================================================\n', + '--- .\t(revision 1)\n', + '+++ .\t(revision 2)\n', + '\n', + 'Property changes on: .\n', + '___________________________________________________________________\n', + 'Added: k\n', + '## -0,0 +1 ##\n', + '+v\n', + '\ No newline at end of property\n' + ] + svntest.actions.run_and_verify_svn(None, expected_output, [], + 'diff', sbox.repo_url + '/A', '-c', '2') + if is_ra_type_svn() or is_ra_type_dav(): + write_authz_file(sbox, {"/" : "* =", + "/A/B/E" : "* = rw"}) + + # Diff the replacement + expected_output = [ + 'Index: alpha\n', + '===================================================================\n', + '--- alpha\t(revision 3)\n', + '+++ alpha\t(revision 4)\n', + '@@ -1 +0,0 @@\n', + '-This is the file \'alpha\'.\n', + 'Index: beta\n', + '===================================================================\n', + '--- beta\t(revision 3)\n', + '+++ beta\t(revision 4)\n', + '@@ -1 +0,0 @@\n', + '-This is the file \'beta\'.\n', + 'Index: tau\n', + '===================================================================\n', + '--- tau\t(revision 0)\n', + '+++ tau\t(revision 4)\n', + '@@ -0,0 +1 @@\n', + '+This is the file \'tau\'.\n', + 'Index: rho\n', + '===================================================================\n', + '--- rho\t(revision 0)\n', + '+++ rho\t(revision 4)\n', + '@@ -0,0 +1 @@\n', + '+This is the file \'rho\'.\n', + 'Index: pi\n', + '===================================================================\n', + '--- pi\t(revision 0)\n', + '+++ pi\t(revision 4)\n', + '@@ -0,0 +1 @@\n', + '+This is the file \'pi\'.\n', + ] + + if is_ra_type_svn() or is_ra_type_dav(): + # Because we can't anchor above C we see just a changed C, not a + # replacement + expected_output += [ + 'Index: .\n', + '===================================================================\n', + '--- .\t(revision 3)\n', + '+++ .\t(revision 4)\n', + '\n', + 'Property changes on: .\n', + '___________________________________________________________________\n', + 'Modified: this-is\n', + '## -1 +1 ##\n', + '-E\n', + '\ No newline at end of property\n', + '+G\n', + '\ No newline at end of property\n', + ] + else: + # ### We should also see a property deletion here! + expected_output += [ + 'Index: .\n', + '===================================================================\n', + '--- .\t(revision 0)\n', + '+++ .\t(revision 4)\n', + '\n', + 'Property changes on: .\n', + '___________________________________________________________________\n', + 'Added: this-is\n', + '## -0,0 +1 ##\n', + '+G\n', + '\ No newline at end of property\n', + ] + + # Use two url diff, because 'svn diff url -c' uses copyfrom to diff against + expected_output = svntest.verify.UnorderedOutput(expected_output) + svntest.actions.run_and_verify_svn(None, expected_output, [], + 'diff', sbox.repo_url + '/A/B/E@3', + sbox.repo_url + '/A/B/E@4', + '--notice-ancestry') + + # Do the same thing with summarize to really see directory deletes and adds + if is_ra_type_svn() or is_ra_type_dav(): + # With no rights on the parent directory we just see a property change on E + expected_output = [ + 'D %s/A/B/E/alpha\n' % sbox.repo_url, + 'D %s/A/B/E/beta\n' % sbox.repo_url, + 'A %s/A/B/E/tau\n' % sbox.repo_url, + 'A %s/A/B/E/rho\n' % sbox.repo_url, + 'A %s/A/B/E/pi\n' % sbox.repo_url, + ' M %s/A/B/E\n' % sbox.repo_url, + ] + else: + # But with rights on the parent we see a replacement of E + expected_output = [ + 'D %s/A/B/E/alpha\n' % sbox.repo_url, + 'D %s/A/B/E/beta\n' % sbox.repo_url, + 'D %s/A/B/E\n' % sbox.repo_url, + 'A %s/A/B/E/tau\n' % sbox.repo_url, + 'A %s/A/B/E/rho\n' % sbox.repo_url, + 'A %s/A/B/E/pi\n' % sbox.repo_url, + 'A %s/A/B/E\n' % sbox.repo_url, + ] + + expected_output = svntest.verify.UnorderedOutput(expected_output) + svntest.actions.run_and_verify_svn(None, expected_output, [], + 'diff', sbox.repo_url + '/A/B/E@3', + sbox.repo_url + '/A/B/E@4', + '--notice-ancestry', '--summarize') ######################################################################## # Run the tests