[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: Noorul Islam K M <noorul_at_collab.net>
Date: Thu, 24 Feb 2011 08:28:28 +0530

Noorul Islam K M <noorul_at_collab.net> writes:

> Julian Foad <julian.foad_at_wandisco.com> writes:
>
>> 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
>>
>>
>
> 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
>
> Thanks and Regards
> Noorul
>
>>> > 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));

Pinging to get some attention to this thread.

Thanks and Regards
Noorul
Received on 2011-02-24 03:59:01 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.