Julian Foad <julian.f...@wandisco.com> writes: > On Sat, 2011-02-19, Noorul Islam K M wrote: > >> I think it is this way >> >> If specified target ('/tmp/iota' in the example above) is a ... >> nothing -> fine >> file -> if 'force' then overwrite else error >> symlink -> if 'force' then overwrite else error >> special -> if 'force' then overwrite else error >> dir -> if sub-target ('/tmp/iota/iota') is a ... >> nothing -> fine >> file -> if 'force' then overwrite else error >> symlink -> if 'force' then overwrite else error >> special -> if 'force' then overwrite else error >> dir -> error cannot force > > (That's all when the source is a file; a dir is handled differently.) > > OK, that looks good. I agree that that would be a good behaviour to > implement. > > I see that svn_client_export5() handles the URL case and the WC case in > completely different ways. A URL source is handled with an "editor", > whereas a WC source is handled by calling copy_versioned_files() which > tries to do the everything including the source tree-walking by itself. > > I studied the implementation of copy_one_versioned_file() and > copy_versioned_files(), and I added doc strings to them in r1074526. > copy_versioned_files() is a mess: there are lots of things wrong with > it, so I can't believe it works properly. > > Does this "overwrite else error" behaviour work the same for exporting > from a URL as exporting from a local WC? Please could you check whether > that's already being tested, and extend your test if not. >
Yes exporting from URL source also works similar to local WC. We need to fix that also and that should be another patch. But I extended the test to include this case and also left the XFail marker as such. Here is the updated patch and log message. Log [[[ Fix for issue #3799. Make svn export display error, when exporting local WC file, tries to overwrite target. There will be a follow-up patch to fix when export source is URL. * subversion/libsvn_client/export.c (copy_versioned_files): Return SVN_ERR_FS_ALREADY_EXISTS if export tries to overwrite existing file, child directory. * subversion/tests/cmdline/export_tests.py (export_file_overwrite_fails): Extend test for URL source. (export_file_overwrite_with_force): New test (test_list): Add reference to new test * subversion/tests/cmdline/externals_tests.py (export_wc_with_externals): Fix failing test by passing --force. Patch by: Noorul Islam K M <noorul{_AT_}collab.net> ]]] Thanks and Regards Noorul
Index: subversion/tests/cmdline/externals_tests.py =================================================================== --- subversion/tests/cmdline/externals_tests.py (revision 1074971) +++ 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 1074971) +++ subversion/tests/cmdline/export_tests.py (working copy) @@ -463,12 +463,13 @@ 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) @@ -479,6 +480,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() @@ -703,6 +715,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 @@ -735,6 +770,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 1074971) +++ subversion/libsvn_client/export.c (working copy) @@ -609,7 +609,26 @@ } else if (from_kind == svn_node_file) { + svn_node_kind_t kind; + svn_error_t *err; + SVN_ERR(append_basename_if_dir(&to_abspath, from_abspath, FALSE, pool)); + 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_FS_ALREADY_EXISTS, 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_FS_ALREADY_EXISTS, 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->wc_ctx, revision, native_eol, ignore_keywords, pool));