On Tue, 2011-05-31 at 14:52 +0530, Noorul Islam K M wrote:
> Julian Foad <julian.foad_at_wandisco.com> writes:
>
> > 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,
>
> I started a new thread http://svn.haxx.se/dev/archive-2011-05/1045.shtml
> for this.
Thanks.
> Now is it ok to mark the failing test as XFail and proceed with this
> patch?
Yes, please do. With that change, I think the patch is finished and can
be committed.
- Julian
Received on 2011-05-31 11:37:44 CEST