[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: Wed, 25 May 2011 15:46:48 +0100

Noorul Islam K M wrote:
> Noorul Islam K M <noorul_at_collab.net> writes:
> > Julian Foad <julian.foad_at_wandisco.com> writes:
[...]
> >> * 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?)
> >
> > Incorporated you review comments. Please find attached updated
> > patch. Here is the log message.

Thanks. I confirm those fixes.

[...]
> * subversion/tests/cmdline/externals_tests.py
> (export_wc_with_externals): Fix failing test by passing --force.

> >> * Why does that externals test (number 10) need "--force"? Without it,
> >> it fails like this, but I don't understand why:
> >> 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
> >
> > A/B/gamma is part of working copy and also part of the externals. This
> > makes this path to be exported twice. During the second time it is
> > failing with the above message.

A/B/gamma is only an external: it does not appear in the WC until
Subversion processes the external definitions.

It looks to me like that failure was showing us a bug. If I run the
test, without your patch, in verbose mode, I see:

CMD: svn export svn-test-work/working_copies/externals_tests-10
svn-test-work/working_copies/externals_tests-10.export [...]
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/gamma
[...]
CMD: svn export --ignore-externals
svn-test-work/working_copies/externals_tests-10
svn-test-work/working_copies/externals_tests-10.export [...]
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
[...]

There is a comment in the test about --ignore-externals not ignoring
A/B/gamma. That's a bug. And the first export (without
--ignore-externals) is also buggy. It shouldn't export A/B/gamma twice.

We shouldn't just quietly tweak the test to hide the bug. We should
write a new test specifically to check for that bug, or fix the bug, or
file an issue, or write to the dev@ list about it. Something.

- Julian
Received on 2011-05-25 16:47:21 CEST

This is an archived mail posted to the Subversion Dev mailing list.