Just taking a quick look. * Please always include the log message with the patch.
* Use SVN_ERR instead of svn_error_clear. There 'kind' variable is not guaranteed to be set to a valid value if you the function throws an error. * Name the variable the same way ('to_kind') in both code paths. * Should export_file_overwrite_with_force() test exporting from a URL as well as from a local source? (If not, why not?) * Why does that externals test (number 10) need "--force"? Without it, it fails like this, but I don't understand why: [[[ CMD: /home/julianfoad/src/subversion-b/bin/svn export svn-test-work/working_copies/externals_tests-10 svn-test-work/working_copies/externals_tests-10.export --config-dir /home/julianfoad/build/subversion-b/subversion/tests/cmdline/svn-test-work/local_tmp/config --password rayjandom --no-auth-cache --username jrandom exited with 1 <TIME = 0.039988> A svn-test-work/working_copies/externals_tests-10.export/A A svn-test-work/working_copies/externals_tests-10.export/A/B A svn-test-work/working_copies/externals_tests-10.export/A/B/lambda A svn-test-work/working_copies/externals_tests-10.export/A/B/gamma A svn-test-work/working_copies/externals_tests-10.export/A/B/E A svn-test-work/working_copies/externals_tests-10.export/A/B/E/alpha A svn-test-work/working_copies/externals_tests-10.export/A/B/E/beta A svn-test-work/working_copies/externals_tests-10.export/A/B/F /home/julianfoad/src/subversion-b/subversion/svn/export-cmd.c:123: (apr_err=200009) /home/julianfoad/src/subversion-b/subversion/libsvn_client/export.c:1291: (apr_err=200009) /home/julianfoad/src/subversion-b/subversion/libsvn_client/export.c:499: (apr_err=200009) /home/julianfoad/src/subversion-b/subversion/libsvn_client/export.c:499: (apr_err=200009) /home/julianfoad/src/subversion-b/subversion/libsvn_client/export.c:563: (apr_err=200009) /home/julianfoad/src/subversion-b/subversion/libsvn_client/export.c:578: (apr_err=200009) svn: E200009: Destination file '/home/julianfoad/build/subversion-b/subversion/tests/cmdline/svn-test-work/working_copies/externals_tests-10.export/A/B/gamma' exists, and will not be overwritten unless forced ]]] Thanks. - Julian On Tue, 2011-05-24 at 16:52 +0530, Noorul Islam K M wrote: > Index: subversion/tests/cmdline/externals_tests.py > =================================================================== > --- subversion/tests/cmdline/externals_tests.py (revision 1126953) > +++ subversion/tests/cmdline/externals_tests.py (working copy) > @@ -752,7 +752,8 @@ > repo_url, wc_dir) > # Export the working copy. > svntest.actions.run_and_verify_svn(None, None, [], > - 'export', wc_dir, export_target) > + 'export', '--force', > + wc_dir, export_target) > > ### We should be able to check exactly the paths that > externals_test_setup() > ### set up; however, --ignore-externals fails to ignore 'A/B/gamma' so this > Index: subversion/tests/cmdline/export_tests.py > =================================================================== > --- subversion/tests/cmdline/export_tests.py (revision 1126953) > +++ subversion/tests/cmdline/export_tests.py (working copy) > @@ -589,19 +589,19 @@ > '.', expected_output, > expected_disk) > > -@XFail() > @Issue(3799) > def export_file_overwrite_fails(sbox): > "exporting a file refuses to silently overwrite" > sbox.build(create_wc = True, read_only = True) > > iota_path = os.path.abspath(os.path.join(sbox.wc_dir, 'iota')) > + iota_url = sbox.repo_url + '/iota' > not_iota_contents = "This obstructs 'iota'.\n" > > tmpdir = sbox.get_tempname('file-overwrites') > os.mkdir(tmpdir) > > - # Run it > + # Run it for source local > open(os.path.join(tmpdir, 'iota'), 'w').write(not_iota_contents) > svntest.actions.run_and_verify_svn(None, [], '.*exist.*', > 'export', iota_path, tmpdir) > @@ -612,6 +612,17 @@ > }) > svntest.actions.verify_disk(tmpdir, expected_disk) > > + # Run it for source URL > + open(os.path.join(tmpdir, 'iota'), 'w').write(not_iota_contents) > + svntest.actions.run_and_verify_svn(None, [], '.*exist.*', > + 'export', iota_url, tmpdir) > + > + # Verify it failed > + expected_disk = svntest.wc.State('', { > + 'iota': Item(contents=not_iota_contents), > + }) > + svntest.actions.verify_disk(tmpdir, expected_disk) > + > def export_ignoring_keyword_translation(sbox): > "export ignoring keyword translation" > sbox.build() > @@ -867,6 +878,29 @@ > > os.chdir(orig_dir) > > +def export_file_overwrite_with_force(sbox): > + "exporting a file with force option" > + sbox.build(create_wc = True, read_only = True) > + > + iota_path = os.path.abspath(os.path.join(sbox.wc_dir, 'iota')) > + not_iota_contents = "This obstructs 'iota'.\n" > + iota_contents = "This is the file 'iota'.\n" > + > + tmpdir = sbox.get_tempname('file-overwrites') > + os.mkdir(tmpdir) > + > + expected_disk = svntest.wc.State('', { > + 'iota': Item(contents=iota_contents), > + }) > + > + # Run it > + open(os.path.join(tmpdir, 'iota'), 'w').write(not_iota_contents) > + svntest.actions.run_and_verify_svn(None, svntest.verify.AnyOutput, > + [], 'export', '--force', > + iota_path, tmpdir) > + > + svntest.actions.verify_disk(tmpdir, expected_disk) > + > ######################################################################## > # Run the tests > > @@ -899,6 +933,7 @@ > export_working_copy_with_depths, > export_externals_with_native_eol, > export_to_current_dir, > + export_file_overwrite_with_force, > ] > > if __name__ == '__main__': > Index: subversion/libsvn_client/export.c > =================================================================== > --- subversion/libsvn_client/export.c (revision 1126953) > +++ subversion/libsvn_client/export.c (working copy) > @@ -569,6 +569,25 @@ > } > else if (from_kind == svn_node_file) > { > + svn_node_kind_t kind; > + > + svn_error_clear(svn_io_check_path(to_abspath, &kind, pool)); > + > + if ((kind == svn_node_file || kind == svn_node_unknown) && ! force) > + { > + return svn_error_createf(SVN_ERR_ILLEGAL_TARGET, NULL, > + _("Destination file '%s' exists, " > + "and will not be overwritten unless " > + "forced"), > + svn_dirent_local_style(to_abspath, pool)); > + } > + else if (kind == svn_node_dir) > + return svn_error_createf(SVN_ERR_ILLEGAL_TARGET, NULL, > + _("Destination %s exists. Cannot overwrite " > + "directory with non-directory"), > + svn_dirent_local_style(to_abspath, pool)); > + > + > SVN_ERR(copy_one_versioned_file(from_abspath, to_abspath, ctx, > revision, native_eol, ignore_keywords, > pool)); > @@ -1064,6 +1083,7 @@ > apr_hash_t *props; > apr_hash_index_t *hi; > struct file_baton *fb = apr_pcalloc(pool, sizeof(*fb)); > + svn_node_kind_t to_kind; > > if (svn_path_is_empty(to_path)) > { > @@ -1080,7 +1100,23 @@ > eb->root_path = to_path; > } > > + svn_error_clear(svn_io_check_path(to_path, &to_kind, pool)); > > + if ((to_kind == svn_node_file || to_kind == svn_node_unknown) && > + ! overwrite) > + { > + return svn_error_createf(SVN_ERR_ILLEGAL_TARGET, NULL, > + _("Destination file '%s' exists, " > + "and will not be overwritten unless > " > + "forced"), > + svn_dirent_local_style(to_path, > pool)); > + } > + else if (to_kind == svn_node_dir) > + return svn_error_createf(SVN_ERR_ILLEGAL_TARGET, NULL, > + _("Destination %s exists. Cannot > overwrite " > + "directory with non-directory"), > + svn_dirent_local_style(to_path, pool)); > + > /* Since you cannot actually root an editor at a file, we > * manually drive a few functions of our editor. */ >