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

Re: [PATCH] Fix for issue 3799 - exporting file should not overwrite

From: Julian Foad <julian.foad_at_wandisco.com>
Date: Thu, 17 Feb 2011 12:06:59 +0000

On Tue, 2011-02-15, Noorul Islam K M wrote:
> More details about the issue can be found
> here. http://subversion.tigris.org/issues/show_bug.cgi?id=3799
>
> Using trunk when exporting a file, if the file already exists at target
> location then operation overwrites the file without giving any
> warning. But in the case of directory it throws an error saying
>
> "Destination directory exists, and will not be overwritten unless
> forced"
>
> This patch makes export throw an error in the case of file
> overwriting and ask to use --force option.
>
> Log
> [[[
>
> Make svn export display error when exporting file tries to overwrite
> target.

Please include the issue number in the log message.

> * subversion/svn/export-cmd.c
> (svn_cl__export): Generalise error message to include file.
>
> * subversion/libsvn_client/export.c
> (copy_versioned_files): Return SVN_ERR_WC_OBSTRUCTED_UPDATE if export
> tries to overwrite existing file.
>
> * subversion/tests/cmdline/export_tests.py
> (export_file_overwrite_fails): Remove XFail marker
> (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/export_tests.py
> ===================================================================
> --- subversion/tests/cmdline/export_tests.py (revision 1070515)
> +++ subversion/tests/cmdline/export_tests.py (working copy)
> @@ -456,7 +456,6 @@
> '.', expected_output,
> expected_disk)
>
> -_at_XFail()
> @Issue(3799)
> def export_file_overwrite_fails(sbox):
> "exporting a file refuses to silently overwrite"
> @@ -703,6 +702,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)

Should we test a non-file obstruction (a directory and/or a symlink?) as
well?

> + 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 +757,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/svn/export-cmd.c
> ===================================================================
> --- subversion/svn/export-cmd.c (revision 1070515)
> +++ subversion/svn/export-cmd.c (working copy)
> @@ -112,8 +112,8 @@
> opt_state->native_eol, ctx, pool);
> if (err && err->apr_err == SVN_ERR_WC_OBSTRUCTED_UPDATE && !opt_state->force)
> SVN_ERR_W(err,
> - _("Destination directory exists; please remove "
> - "the directory or use --force to overwrite"));
> + _("Destination directory/file already exists; please remove "
> + "it or use --force to overwrite"));
>
> if (nwb.had_externals_error)
> return svn_error_create(SVN_ERR_CL_ERROR_PROCESSING_EXTERNALS, NULL,
> Index: subversion/libsvn_client/export.c
> ===================================================================
> --- subversion/libsvn_client/export.c (revision 1070515)
> +++ subversion/libsvn_client/export.c (working copy)
> @@ -524,7 +524,16 @@
> }
> else if (from_kind == svn_node_file)
> {
> + svn_node_kind_t kind;
> +
> 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) && ! force)

What if the obstruction is not a file but something else (dir or
unknown)?

> + return svn_error_createf(SVN_ERR_WC_OBSTRUCTED_UPDATE, NULL,
> + _("'%s' already exists"),
> + svn_dirent_local_style(to_abspath, pool));

Why not make the wording of this error message similar to the one that
is given when exporting a directory hits an obstruction?

    SVN_ERR_W(err, _("Destination directory exists, and will not be "
                     "overwritten unless forced"));
 
(Including the path in the error message is good; I think we should
tweak the directory-hits-an-obstruction error message to do so as well.)

> SVN_ERR(copy_one_versioned_file(from_abspath, to_abspath, ctx->wc_ctx,
> revision, native_eol, ignore_keywords,
> pool));

- Julian
Received on 2011-02-17 13:07:41 CET

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.