You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@subversion.apache.org by Mark Phippard <ma...@gmail.com> on 2009/12/17 15:11:05 UTC
Failing JavaHL test
One of the JavaHL tests has been failing for a while now:
1) testBasicMerge(org.tigris.subversion.javahl.BasicTests)junit.framework.AssertionFailedError:
Missing eligible merge info on
'/home/bt/slaves/x64-centos/build/subversion/bindings/javahl/test-work/working_copies/basic_test33/branches/A/mu'
at org.tigris.subversion.javahl.BasicTests.acquireMergeinfoAndAssertEquals(BasicTests.java:2234)
at org.tigris.subversion.javahl.BasicTests.testBasicMerge(BasicTests.java:2402)
at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
at sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:39)
at sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:25)
at org.tigris.subversion.javahl.RunTests.main(RunTests.java:116)
I have debugged this. The problem is happening in a method we run in
the tests after merges to verify that the svn mergeinfo functions
return the right values for merged and eligible revisions. In this
case, it is failing when retrieving the eligible revisions.
If I stop the debugger and go out to the command line and run the
command, I get this:
$ svn mergeinfo --show-revs=eligible --depth=empty
file:///Users/mphippard/work/src-trunk/repositories/basic_test1/A/mu
/Users/mphippard/work/src-trunk/working_copies/basic_test1/branches/A/mu
r6
Which is the right value and what the JavaHL tests look for. So the
test expectations are still correct.
However, when the JavaHL method runs, it gets an Exception.
org.tigris.subversion.javahl.ClientException: Bogus revision information given
svn: traced call
svn: traced call
svn: traced call
svn: subversion/libsvn_client/mergeinfo.c:1740: (apr_err=195002)
at org.tigris.subversion.javahl.SVNClient.getMergeinfoLog(Native Method)
at org.tigris.subversion.javahl.SVNClientSynchronized.getMergeinfoLog(SVNClientSynchronized.java:1082)
at org.tigris.subversion.javahl.BasicTests.getMergeinfoRevisions(BasicTests.java:2275)
at org.tigris.subversion.javahl.BasicTests.acquireMergeinfoAndAssertEquals(BasicTests.java:2232)
at org.tigris.subversion.javahl.BasicTests.testBasicMerge(BasicTests.java:2400)
I am going to commit a change to the tests so that this exception is
logged as the failure instead of just catching it and reporting the
error differently. The exception seems to provide a better clue.
I visually walked through the Java code into the C++ and into the C to
see if there was an argument transposition or something. It all looks
right to me though. Hopefully the clue this exception gives of
showing the line where it fails in mergeinfo.c will help someone fix
the problem.
--
Thanks
Mark Phippard
http://markphip.blogspot.com/
svn_client__entry_location() [was: Failing JavaHL test]
Posted by Julian Foad <ju...@btopenworld.com>.
On Mon, 2009-12-21, Paul Burba wrote:
> This is replicable from the command line when the svn mergeinfo target
> is a working copy path pegged at HEAD or DATE, e.g.:
>
> C:\SVN\src-trunk-2\Release\subversion\tests\cmdline\svn-test-work\working_copies\mergeinfo_tests-8>svn
> mergeinfo --show-revs eligible A A_COPY@HEAD
> svn: Bogus revision information given
>
> I fixed this in r892902 and also expanded mergeinfo_tests.py 8 so it
> also covers this bug.
> ------------------------------------------------------------------------
> r892902 | pburba | 2009-12-21 17:45:34 +0000 (Mon, 21 Dec 2009) | 17 lines
> Changed paths:
> M /subversion/trunk/subversion/libsvn_client/url.c
> M /subversion/trunk/subversion/tests/cmdline/mergeinfo_tests.py
>
> Fix 'svn mergeinfo' bug when target is a WC path pegged at HEAD or DATE.
>
> This fixes the failing JavaHL test BasicTests.java:testBasicMerge.
> See http://svn.haxx.se/dev/archive-2009-12/0346.shtml.
>
> * subversion/libsvn_client/url.c
> (svn_client__derive_location): Follow-up to r886880; contact the server if
> asking about a working copy path pegged at DATE or HEAD. Previously we
> tried to resolve the peg rev in this case with a call to
> svn_client__entry_location(), but since r886880 that function has
> returned an error if asked about revisions only the repos knows about.
>
> * subversion/tests/cmdline/mergeinfo_tests.py
> (mergeinfo_on_pegged_wc_path): Fix typo, this test should have tested
> WC paths pegged at HEAD, but was instead twice testing WC paths pegged
> at BASE. Also add --show-revs=eligible variants of the tests.
>
> ------------------------------------------------------------------------
Thank you for identifying and fixing that bug.
I looked at the function svn_client__entry_location() which is involved
in this logic, and it is totally crazy. I can't fathom its purpose.
What is this function for? I think it's time to forget about what it was
originally for, and to examine what we want it for now.
I looked briefly at the six callers, and I couldn't see easily what all
of them expected from this function, so some more investigation is
needed.
Your log message says "since r886880 that function has returned an error
if asked about revisions only the repos knows about." However, the
function doesn't mention that in its doc string, and it still claims to
handle requests for revision "previous" which only the repos knows
about.
Keep in mind that a node's path (URL) exists in a particular revision
and might not exist in another revision.
[[[
> /* Get the repository URL and revision number for LOCAL_ABSPATH and put them
> in *URL and *REVNUM. REVNUM may be null, in which case it is ignored.
Get the URL and revnum of what version of the node? LOCAL_ABSPATH
identifies a "working" node, and thus a line of history; we can identify
locations along this line of history at various revisions, some of which
could be specified by the PEG_REV_KIND parameter.
I think the word "peg" is getting in the way again. A "peg revision" is
a revision that goes along with a URL in order to identify a line of
history along which other revisions can be found. In the context of this
function, the line of history can be identified solely by LOCAL_ABSPATH,
and so the revision we want to look up is not a peg but an operative
revision. (We might not want to use the word "operative" either,
reserving it just for high-level operations. If, in the context of the
caller, the revision determined here is in turn going to act as a peg
for a subsequent look-up, that is none of our business here.) So delete
the word "peg".
> If PEG_REV_KIND is svn_opt_revision_working, then use the LOCAL_ABSPATH's
> copyfrom info to populate *URL and *REVNUM.
What if it isn't a copy/move?
> If PEG_REV_KIND is svn_opt_revision_date or svn_opt_revision_head then
> return SVN_ERR_CLIENT_BAD_REVISION.
>
> If PEG_REV_KIND is svn_opt_revision_committed or svn_opt_revision_previous
> then set *REVNUM to the last committed or previous revision respectively.
And what about *URL? We can work out the (or a) previous revision number
from local WC info, but we can't work out the previous URL, so I don't
think this is a sensible API if we want to avoid contacting the
repository.
> If PEG_REV_NUM is svn_opt_revision_unspecified, svn_opt_revision_number,
> svn_opt_revision_base, or svn_opt_revision_working then set *REVNUM
> to the base revision. */
The param is called PEG_REV_KIND.
There's no point in defining such a meaning (or any meaning) to the kind
"_unspecified".
It's silly to map "_number" to mean "_base".
It's sensible that "_base" means "_base".
It's a typo/error that "_working" is mentioned again here.
> svn_error_t *
> svn_client__entry_location(const char **url,
> svn_revnum_t *revnum,
> svn_wc_context_t *wc_ctx,
> const char *local_abspath,
> enum svn_opt_revision_kind peg_rev_kind,
> apr_pool_t *result_pool,
> apr_pool_t *scratch_pool);
]]]
Note that it may be good to split up this API into multiple functions,
if we don't need the ability for the revision kind to be passed in as a
parameter.
- Julian
Re: Failing JavaHL test
Posted by Paul Burba <pt...@gmail.com>.
On Thu, Dec 17, 2009 at 10:11 AM, Mark Phippard <ma...@gmail.com> wrote:
> One of the JavaHL tests has been failing for a while now:
>
> 1) testBasicMerge(org.tigris.subversion.javahl.BasicTests)junit.framework.AssertionFailedError:
> Missing eligible merge info on
> '/home/bt/slaves/x64-centos/build/subversion/bindings/javahl/test-work/working_copies/basic_test33/branches/A/mu'
> at org.tigris.subversion.javahl.BasicTests.acquireMergeinfoAndAssertEquals(BasicTests.java:2234)
> at org.tigris.subversion.javahl.BasicTests.testBasicMerge(BasicTests.java:2402)
> at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
> at sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:39)
> at sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:25)
> at org.tigris.subversion.javahl.RunTests.main(RunTests.java:116)
>
>
> I have debugged this. The problem is happening in a method we run in
> the tests after merges to verify that the svn mergeinfo functions
> return the right values for merged and eligible revisions. In this
> case, it is failing when retrieving the eligible revisions.
>
> If I stop the debugger and go out to the command line and run the
> command, I get this:
>
> $ svn mergeinfo --show-revs=eligible --depth=empty
> file:///Users/mphippard/work/src-trunk/repositories/basic_test1/A/mu
> /Users/mphippard/work/src-trunk/working_copies/basic_test1/branches/A/mu
> r6
>
> Which is the right value and what the JavaHL tests look for. So the
> test expectations are still correct.
>
> However, when the JavaHL method runs, it gets an Exception.
>
> org.tigris.subversion.javahl.ClientException: Bogus revision information given
> svn: traced call
> svn: traced call
> svn: traced call
> svn: subversion/libsvn_client/mergeinfo.c:1740: (apr_err=195002)
> at org.tigris.subversion.javahl.SVNClient.getMergeinfoLog(Native Method)
> at org.tigris.subversion.javahl.SVNClientSynchronized.getMergeinfoLog(SVNClientSynchronized.java:1082)
> at org.tigris.subversion.javahl.BasicTests.getMergeinfoRevisions(BasicTests.java:2275)
> at org.tigris.subversion.javahl.BasicTests.acquireMergeinfoAndAssertEquals(BasicTests.java:2232)
> at org.tigris.subversion.javahl.BasicTests.testBasicMerge(BasicTests.java:2400)
>
> I am going to commit a change to the tests so that this exception is
> logged as the failure instead of just catching it and reporting the
> error differently. The exception seems to provide a better clue.
>
> I visually walked through the Java code into the C++ and into the C to
> see if there was an argument transposition or something. It all looks
> right to me though. Hopefully the clue this exception gives of
> showing the line where it fails in mergeinfo.c will help someone fix
> the problem.
Mark,
This is replicable from the command line when the svn mergeinfo target
is a working copy path pegged at HEAD or DATE, e.g.:
C:\SVN\src-trunk-2\Release\subversion\tests\cmdline\svn-test-work\working_copies\mergeinfo_tests-8>svn
mergeinfo --show-revs eligible A A_COPY@HEAD
svn: Bogus revision information given
I fixed this in r892902 and also expanded mergeinfo_tests.py 8 so it
also covers this bug.
Paul