Julian Foad <julian.f...@wandisco.com> writes:
> Noorul Islam K M wrote:
>
>> Noorul Islam K M <noo...@collab.net> writes:
>> > Julian Foad <julian.f...@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,
I started a new thread http://svn.haxx.se/dev/archive-2011-05/1045.shtml
for this.
Now is it ok to mark the failing test as XFail and proceed with this
patch?
Thanks and Regards
Noorul
