[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: Fri, 18 Feb 2011 16:49:02 +0000

On Fri, 2011-02-18 at 15:54 +0530, Noorul Islam K M wrote:
> Noorul Islam K M <noorul_at_collab.net> writes:
>
> > Julian Foad <julian.foad_at_wandisco.com> writes:
> >
> >> On Thu, 2011-02-17 at 21:04 +0530, Noorul Islam K M wrote:
> >>
> >>> Julian Foad <julian.foad_at_wandisco.com> writes:
> >>>
> >>> > Noorul Islam K M wrote:
> >>> >
> >>> >> Julian Foad <julian.foad_at_wandisco.com> writes:
> >>> >> > On Tue, 2011-02-15, Noorul Islam K M wrote:
> >>> >> >> + if ((kind == svn_node_file) && ! force)
> >>> >> >
> >>> >> > What if the obstruction is not a file but something else (dir or
> >>> >> > unknown)?
> >>> >>
> >>> >> Obstruction of directory is already taken care of.
> >>> >
> >>> > I'm asking about obstruction *by* a directory (or symlink or ...), in
> >>> > otyher words when (kind == svn_node_dir) or (kind == svn_node_unknown).
> >>> >
> >>>
> >>> Are you talking about this scenario?
> >>>
> >>> noorul_at_laptop:/tmp/wc/testrepo$ touch iota
> >>> noorul_at_laptop:/tmp/wc/testrepo$ ~/projects/subversion/src/trunk/vpath/subversion/svn/svn add iota
> >>> A iota
> >>> noorul_at_laptop:/tmp/wc/testrepo$ mkdir /tmp/iota
> >>> noorul_at_laptop:/tmp/wc/testrepo$ ~/projects/subversion/src/trunk/vpath/subversion/svn/svn export iota /tmp/iota
> >>> A /tmp/iota/iota
> >>> Export complete.
> >>> noorul_at_laptop:/tmp/wc/testrepo$
> >>
> >> Yes, this scenario and other scenarios similar to this where /tmp/iota
> >> could instead be a directory with children (and maybe one of the
> >> children is called 'iota'), or could be a symlink or a special file.
> >>
> >
> > I modified the patch so that if the target exists as a child directory
> > then throw the following error message.
> >
> > svn: E160020: Destination /tmp/iota/iota exists. Cannot overwrite
> > directory with non-directory

Ah, a child of a directory - I see. I hadn't appreciated that if the
specified target is a directory ('/tmp/iota' here) then it will try to
export into a child of that directory.

So now you have covered these cases:

If specified target ('/tmp/iota' in the example above) is a ...
  nothing -> fine
  file -> if 'force' then overwrite else error
  symlink -> ???
  special -> ???
  dir -> if sub-target ('/tmp/iota/iota') is a ...
    nothing -> fine
    file -> if 'force' then overwrite else error
    symlink -> ???
    special -> error
    dir -> error

Is that right?

- Julian

> > With the latest patch I tested the following
> >
> > noorul_at_noorul:/tmp/wc$ ~/projects/subversion/builds/trunk/bin/svnadmin create /tmp/testrepo
> >
> > noorul_at_noorul:/tmp/wc$ ~/projects/subversion/builds/trunk/bin/svn co file:///tmp/testrepo
> > Checked out revision 0.
> >
> > noorul_at_noorul:/tmp/wc$ cd testrepo
> >
> > noorul_at_noorul:/tmp/wc/testrepo$ touch iota
> >
> > noorul_at_noorul:/tmp/wc/testrepo$ ~/projects/subversion/builds/trunk/bin/svn add iota
> > A iota
> >
> > noorul_at_noorul:/tmp/wc/testrepo$ ~/projects/subversion/builds/trunk/bin/svn ci -m "Adding file"
> > Adding iota
> > Transmitting file data .
> > Committed revision 1.
> >
> > noorul_at_noorul:/tmp/wc/testrepo$ ~/projects/subversion/builds/trunk/bin/svn up
> > Updating '.' ...
> > At revision 1.
> > noorul_at_noorul:/tmp/wc/testrepo$ ~/projects/subversion/builds/trunk/bin/svn export iota /tmp
> > A /tmp/iota
> > Export complete.
> >
> > # Child directory
> > noorul_at_noorul:/tmp/wc/testrepo$ rm -r -f /tmp/iota
> > noorul_at_noorul:/tmp/wc/testrepo$ mkdir -p /tmp/iota/iota
> > noorul_at_noorul:/tmp/wc/testrepo$ ~/projects/subversion/builds/trunk/bin/svn export iota /tmp/iota
> > ../subversion/svn/export-cmd.c:123: (apr_err=160020)
> > ../subversion/libsvn_client/export.c:1188: (apr_err=160020)
> > ../subversion/libsvn_client/export.c:542: (apr_err=160020)
> > svn: E160020: Destination /tmp/iota/iota exists. Cannot overwrite directory with non-directory
> >
> > # Special file
> > noorul_at_noorul:/tmp/wc/testrepo$ rm -r -f /tmp/iota/iota
> > noorul_at_noorul:/tmp/wc/testrepo$ mknod /tmp/iota/iota p
> > noorul_at_noorul:/tmp/wc/testrepo$ ~/projects/subversion/builds/trunk/bin/svn export iota /tmp/iota
> > A /tmp/iota/iota
> > Export complete.
> >
>
> Sorry the above should read
>
> # Special file
> noorul_at_noorul:/tmp/wc/testrepo$ mknod /tmp/iota/iota p
>
> noorul_at_noorul:/tmp/wc/testrepo$ ~/projects/subversion/builds/trunk/bin/svn export iota /tmp/iota/
> ../subversion/svn/export-cmd.c:123: (apr_err=160020)
> ../subversion/libsvn_client/export.c:1188: (apr_err=160020)
> ../subversion/libsvn_client/export.c:535: (apr_err=160020)
> svn: E160020: Destination file '/tmp/iota/iota' exists, and will not be overwritten unless forced
>
> noorul_at_noorul:/tmp/wc/testrepo$ ~/projects/subversion/builds/trunk/bin/svn export iota /tmp/iota/ --force
> A /tmp/iota/iota
> Export complete.
>
> > For symlink it successfully overwrites.
> >
>
> I meant symlink type files behave the same as files. I forgot to attach
> the latest patch. Please find attached the latest one.
>
> > Update log message is here
> >
> > Log
> >
> > [[[
> > Fix for issue #3799. Make svn export display error when exporting file
> > tries to overwrite target.
> >
> > * 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): 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
>
> plain text document attachment (3799.txt)
> Index: subversion/tests/cmdline/externals_tests.py
> ===================================================================
> --- subversion/tests/cmdline/externals_tests.py (revision 1071880)
> +++ 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 1071880)
> +++ 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)
> + 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/libsvn_client/export.c
> ===================================================================
> --- subversion/libsvn_client/export.c (revision 1071880)
> +++ subversion/libsvn_client/export.c (working copy)
> @@ -524,7 +524,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));
Received on 2011-02-18 17:49:47 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.