[svn.haxx.se] · SVN Dev · SVN Users · SVN Org · TSVN Dev · TSVN Users · Subclipse Dev · Subclipse Users · this month's index

Re: svn trunk r21278: FAIL (x86-macosx-gnu shared ra_local fsfs)

From: Garrett Rooney <rooneg_at_electricjellyfish.net>
Date: 2006-08-28 22:39:39 CEST

On 8/28/06, Lieven Govaerts <lgo@mobsol.be> wrote:
> Lieven Govaerts wrote:
> > buildbot@mobsol.be wrote:
> >> Full details are available at:
> >> http://www.mobsol.be/buildbot/x86-macosx-gnu%2520shared%2520ra_local%2520fsfs/builds/967
> >>
> >>
> >> Author list: lgo
> >>
> >> Build Slave: osx10.4-gcc4.0.1-ia32
> >>
> >>
> >> Subversion Buildbot
> >> http://www.mobsol.be/buildbot/
> >
> > This test failure in subversion/tests/libsvn_subr/target-test.py is
> > caused by a change in path.c:get_path_ancestor_length to make it support
> > that 'H:/' and 'H:/A' have a common part 'H:/'.
> >
> > Unfortunately, this function is used for URLs too, in the tests at
> > least, so now it starts finding 'file:///' is a common part for
> > 'file:///A' and 'file:///B'.
> >
> > The comment of function get_path_ancestor_length says this:
> >
> > libsvn_subr/path.c line 505:
> > * This function handles everything except the URL-handling logic
> > * of svn_path_get_longest_ancestor, and assumes that PATH1 and
> > * PATH2 are *not* URLs.
> >
> > Based on this comment I conclude that the tests in target-tests.py
> > (which are unit tests) are testing some behavior which the function
> > doesn't support.
> >
> > So, what do I do:
> > - remove the tests 6, 7, 8 & 9 from target-test.py.
> > or
> > - fix the function get_path_ancestor_length so it supports getting the
> > common ancestor of URLs, even if it's never used in Subversion?
>
> I reviewed the usage of svn_path_condense_targets in the svn code, and I
> noticed that it is used multiple times with URLs, ex.: in
> libsvn_client/add.c mkdir_urls.
>
> The comment in path.c (line 505) is therefore wrong. Attached patch
> removes the comment and fixes the problem. The new behaviour is this:
> file:///A/B/C + file:///B/G/H -> no common part
> H:/A/B/C + H:/B/G/H -> H:/ is common part
>
> I tried to add this scenario in target-test.py, but that fails because
> svn (apr_filepath_merge) checks that the H: drive actually exists.
>
> If someone finds to time to review the patch, please do, the buildbot
> has been failing for to long now.

This looks reasonable to me, +1 to commit.

-garrett

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Received on Mon Aug 28 22:40:23 2006

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.