You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@subversion.apache.org by "Peter N. Lundblad" <pe...@famlundblad.se> on 2005/06/19 19:29:53 UTC
Re: svn trunk r15103: FAIL (x86_64-unknown-linux-gnu shared ra_local
bdb)
On Sun, 19 Jun 2005 svntest@jaa.iki.fi wrote:
> FAIL: stat_tests.py 13: timestamp behaviour
> At least one test was SKIPPED, checking /svntest/obj-sh/tests.log
> SKIP: utf8_tests.py 1: conversion of paths and logs to/from utf8
> make: *** [check] Error 1
> Complete log saved in /srv/svntest/svntest/logs/svn_trunk/LOG_svn_check_shared_ra_local_bdb.15103.failed
> FAIL: make check
Might this be my work on status that broke this? Jani (or anyone else),
do you have time to investigate? It passes on my system.
Thanks,
//Peter
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: svn trunk r15103: FAIL (x86_64-unknown-linux-gnu shared ra_local
bdb)
Posted by "Peter N. Lundblad" <pe...@famlundblad.se>.
On Sun, 19 Jun 2005, Philip Martin wrote:
> "Peter N. Lundblad" <pe...@famlundblad.se> writes:
>
> > Ouch! How could I miss that? A missing SVN_ERR (err). I'll fix tomorrow
> > morning.
>
> That's not enough, checkout will fail, yes checkout! If the file is
> unversioned the new code gets SVN_ERR_ENTRY_NOT_FOUND but only handles
> ENOENT.
>
I choose a litle different approach (see r15116). The point is to optimize
the common case where timestamps are the same. The other cases are slow
anyway and an extra stat can't be measurable.
And Karl, I tried to describe the importance of the error codes in a
comment.
Thanks for clarifying the situation,
//Peter
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: svn trunk r15103: FAIL (x86_64-unknown-linux-gnu shared ra_local bdb)
Posted by kf...@collab.net.
"Peter N. Lundblad" <pe...@famlundblad.se> writes:
> Yes, the old code did svn_io_check_path before. That was the "stat" I
> eliminated in this function.
>
> > versioned, so it didn't see SVN_ERR_ENTRY_NOT_FOUND if the file did
> > not exist and returned no error. If the file was unversioned but did
> > exist then I think SVN_ERR_ENTRY_NOT_FOUND was returned. The
> > libsvn_wc code is so complicated it's not clear whether this was
> > accidental or deliberate, and it's not clear whether that exact
>
> OK. I didn't think of that case. I'll commit the patch above tommorrow if
> you don't.
Can you make sure to leave a comment explaining why the check is
necessary, so we don't do this again? :-)
-K
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: svn trunk r15103: FAIL (x86_64-unknown-linux-gnu shared ra_local
bdb)
Posted by "Peter N. Lundblad" <pe...@famlundblad.se>.
On Mon, 20 Jun 2005, Philip Martin wrote:
> "Peter N. Lundblad" <pe...@famlundblad.se> writes:
>
> > On Sun, 19 Jun 2005, Philip Martin wrote:
> >>
> >> versioned, so it didn't see SVN_ERR_ENTRY_NOT_FOUND if the file did
> >> not exist and returned no error. If the file was unversioned but did
> >> exist then I think SVN_ERR_ENTRY_NOT_FOUND was returned. The
> >> libsvn_wc code is so complicated it's not clear whether this was
> >> accidental or deliberate, and it's not clear whether that exact
> >
> > OK. I didn't think of that case. I'll commit the patch above tommorrow if
> > you don't.
>
> One of the reasons I haven't committed it is that I'm not sure that
> catching SVN_ERR_ENTRY_NOT_FOUND is the correct solution. Returning
> SVN_ERR_ENTRY_NOT_FOUND might be the right thing to do when the target
> is unversioned, it looks sensible to me. Why is checkout attempting
> to determine if unversioned items have text changes? Is that
> sensible? If you do commit it then note that the old behaviour was
> documented in svn_wc.h!
Which was the other reason I choose my approach instead. We can't mess
with the public API like I did, of course.
Regards,
//Peter
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: svn trunk r15103: FAIL (x86_64-unknown-linux-gnu shared
ra_local bdb)
Posted by Philip Martin <ph...@codematters.co.uk>.
"Peter N. Lundblad" <pe...@famlundblad.se> writes:
> On Sun, 19 Jun 2005, Philip Martin wrote:
>>
>> versioned, so it didn't see SVN_ERR_ENTRY_NOT_FOUND if the file did
>> not exist and returned no error. If the file was unversioned but did
>> exist then I think SVN_ERR_ENTRY_NOT_FOUND was returned. The
>> libsvn_wc code is so complicated it's not clear whether this was
>> accidental or deliberate, and it's not clear whether that exact
>
> OK. I didn't think of that case. I'll commit the patch above tommorrow if
> you don't.
One of the reasons I haven't committed it is that I'm not sure that
catching SVN_ERR_ENTRY_NOT_FOUND is the correct solution. Returning
SVN_ERR_ENTRY_NOT_FOUND might be the right thing to do when the target
is unversioned, it looks sensible to me. Why is checkout attempting
to determine if unversioned items have text changes? Is that
sensible? If you do commit it then note that the old behaviour was
documented in svn_wc.h!
--
Philip Martin
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: svn trunk r15103: FAIL (x86_64-unknown-linux-gnu shared ra_local
bdb)
Posted by "Peter N. Lundblad" <pe...@famlundblad.se>.
On Sun, 19 Jun 2005, Philip Martin wrote:
> "Peter N. Lundblad" <pe...@famlundblad.se> writes:
>
> > Ouch! How could I miss that? A missing SVN_ERR (err). I'll fix tomorrow
> > morning.
>
> That's not enough, checkout will fail, yes checkout! If the file is
> unversioned the new code gets SVN_ERR_ENTRY_NOT_FOUND but only handles
> ENOENT.
>
> +++ subversion/libsvn_wc/questions.c (working copy)
> @@ -398,13 +398,16 @@
> err = svn_wc__timestamps_equal_p (&equal_timestamps,
> filename, adm_access,
> svn_wc__text_time, subpool);
> - if (err && APR_STATUS_IS_ENOENT(err->apr_err))
> + if (err && (APR_STATUS_IS_ENOENT(err->apr_err)
> + || err->apr_err == SVN_ERR_ENTRY_NOT_FOUND))
> {
> /* If the file doesn't exist, its considered non-modified. */
> svn_error_clear (err);
> *modified_p = FALSE;
> goto cleanup;
> }
> + else if (err)
> + return err;
> if (equal_timestamps)
> {
> *modified_p = FALSE;
>
> I think the old code did a stat() before checking whether the file was
Yes, the old code did svn_io_check_path before. That was the "stat" I
eliminated in this function.
> versioned, so it didn't see SVN_ERR_ENTRY_NOT_FOUND if the file did
> not exist and returned no error. If the file was unversioned but did
> exist then I think SVN_ERR_ENTRY_NOT_FOUND was returned. The
> libsvn_wc code is so complicated it's not clear whether this was
> accidental or deliberate, and it's not clear whether that exact
OK. I didn't think of that case. I'll commit the patch above tommorrow if
you don't.
> behaviour needs to be reproduced. Welcome to libsvn_wc!
>
Yeah, I was warned this was complicated and messy. I want to get rid of
some of the worst performance problems. I think I do it early enough in
the 1.3 cycle to catch regressions before releas.e If you have time, I
appreciate any reviews in this area (and all other areas as well, only if
I try not to read mail to early in the morning:-).
Regards,
//Peter
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: svn trunk r15103: FAIL (x86_64-unknown-linux-gnu shared
ra_local bdb)
Posted by Philip Martin <ph...@codematters.co.uk>.
"Peter N. Lundblad" <pe...@famlundblad.se> writes:
> Ouch! How could I miss that? A missing SVN_ERR (err). I'll fix tomorrow
> morning.
That's not enough, checkout will fail, yes checkout! If the file is
unversioned the new code gets SVN_ERR_ENTRY_NOT_FOUND but only handles
ENOENT.
--- subversion/libsvn_wc/questions.c (revision 15111)
+++ subversion/libsvn_wc/questions.c (working copy)
@@ -398,13 +398,16 @@
err = svn_wc__timestamps_equal_p (&equal_timestamps,
filename, adm_access,
svn_wc__text_time, subpool);
- if (err && APR_STATUS_IS_ENOENT(err->apr_err))
+ if (err && (APR_STATUS_IS_ENOENT(err->apr_err)
+ || err->apr_err == SVN_ERR_ENTRY_NOT_FOUND))
{
/* If the file doesn't exist, its considered non-modified. */
svn_error_clear (err);
*modified_p = FALSE;
goto cleanup;
}
+ else if (err)
+ return err;
if (equal_timestamps)
{
*modified_p = FALSE;
I think the old code did a stat() before checking whether the file was
versioned, so it didn't see SVN_ERR_ENTRY_NOT_FOUND if the file did
not exist and returned no error. If the file was unversioned but did
exist then I think SVN_ERR_ENTRY_NOT_FOUND was returned. The
libsvn_wc code is so complicated it's not clear whether this was
accidental or deliberate, and it's not clear whether that exact
behaviour needs to be reproduced. Welcome to libsvn_wc!
--
Philip Martin
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: svn trunk r15103: FAIL (x86_64-unknown-linux-gnu shared ra_local
bdb)
Posted by "Peter N. Lundblad" <pe...@famlundblad.se>.
On Sun, 19 Jun 2005, Philip Martin wrote:
> "Peter N. Lundblad" <pe...@famlundblad.se> writes:
>
> > On Sun, 19 Jun 2005 svntest@jaa.iki.fi wrote:
> >
> {
> /* If the file doesn't exist, its considered non-modified. */
> svn_error_clear (err);
> *modified_p = FALSE;
> goto cleanup;
> }
> if (equal_timestamps)
> {
> *modified_p = FALSE;
> goto cleanup;
> }
>
> If err != ENOENT then equal_timestamps might be uninitialised.
>
>
Ouch! How could I miss that? A missing SVN_ERR (err). I'll fix tomorrow
morning.
Thanks,
//Peter
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: svn trunk r15103: FAIL (x86_64-unknown-linux-gnu shared
ra_local bdb)
Posted by Philip Martin <ph...@codematters.co.uk>.
"Peter N. Lundblad" <pe...@famlundblad.se> writes:
> On Sun, 19 Jun 2005 svntest@jaa.iki.fi wrote:
>
>> FAIL: stat_tests.py 13: timestamp behaviour
>> At least one test was SKIPPED, checking /svntest/obj-sh/tests.log
>> SKIP: utf8_tests.py 1: conversion of paths and logs to/from utf8
>> make: *** [check] Error 1
>> Complete log saved in /srv/svntest/svntest/logs/svn_trunk/LOG_svn_check_shared_ra_local_bdb.15103.failed
>> FAIL: make check
>
> Might this be my work on status that broke this? Jani (or anyone else),
> do you have time to investigate? It passes on my system.
Valgrind reports this:
CMD: svn "co" "--username" "jrandom" "--password" "rayjandom" "file:///home/pm/sw/subversion/obj/subversion/tests/clients/cmdline/svn-test-work/repositories/stat_tests-13" "svn-test-work/working_copies/stat_tests-13" "--config-dir" "/home/pm/sw/subversion/obj/subversion/tests/clients/cmdline/svn-test-work/local_tmp/config" <TIME = 14.317706>
==3282== Conditional jump or move depends on uninitialised value(s)
==3282== at 0x1B95CA63: svn_wc_text_modified_p (questions.c:408)
==3282== by 0x1B964919: install_file (update_editor.c:2040)
==3282== by 0x1B96554C: close_file (update_editor.c:2404)
==3282== by 0x1B975EBC: close_file (cancel.c:235)
==3282== by 0x1BD52B46: update_entry (reporter.c:669)
==3282== by 0x1BD53126: delta_dirs (reporter.c:791)
==3282== by 0x1BD529BB: update_entry (reporter.c:653)
==3282== by 0x1BD53126: delta_dirs (reporter.c:791)
==3282== by 0x1BD529BB: update_entry (reporter.c:653)
==3282== by 0x1BD53126: delta_dirs (reporter.c:791)
==3282== by 0x1BD5348E: drive (reporter.c:847)
==3282== by 0x1BD5386D: finish_report (reporter.c:908)
if (err && APR_STATUS_IS_ENOENT(err->apr_err))
{
/* If the file doesn't exist, its considered non-modified. */
svn_error_clear (err);
*modified_p = FALSE;
goto cleanup;
}
if (equal_timestamps)
{
*modified_p = FALSE;
goto cleanup;
}
If err != ENOENT then equal_timestamps might be uninitialised.
--
Philip Martin
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org