Stefan Fuhrmann wrote on Sat, Oct 30, 2010 at 18:03:48 +0200:
> On 30.10.2010 17:01, Daniel Shahaf wrote:
>> Looks like at least one test fails:
>>
>> [[[
>> lp-shahaf,14:svn/performance% ../runctest fs fs 22
>> subversion/tests/libsvn_fs/fs-test.c:3411: (apr_err=160000)
>> subversion/tests/libsvn_fs/fs-test.c:3229: (apr_err=160000)
>> svn_tests: Error validating revision 1 (youngest is 3)
>> subversion/tests/svn_test_fs.c:492: (apr_err=160000)
>> svn_tests: Repository tree does not look as expected.
>> Corrupt entries:
>> A/D/G/rho
>> Missing entries:
>> Extra entries:
>>
>> FAIL: lt-fs-test 22: after each commit, check all revisions
>> zsh: exit 1 ../runctest fs fs 22
>> ]]]
>>
>> I haven't tested yet whether it's due to the commits this morning or to
>> something else.
> Hm. The tests run fine here. But I'm testing with FSFS only
> and I'm using plain "make -j && make check" with no magic
> environment variables set.
I ran 'make davautocheck' (which would have tested FSFS), and I build
with "-DSVN_FS_FS_DEFAULT_MAX_FILES_PER_DIR=4 -DPACK_AFTER_EVERY_COMMIT"
in CFLAGS.
Can you reproduce the FAIL with these CFLAGS set?
>> Stefan, can you guess what might be causing this?
>>
> This is typically caused by stream_readline() not detecting
> or handling EOF properly. You could try to replace just that
> function with /trunk code and see what happens.
It fails in the same way; see attached.
>> Also: how to detect this when it happens next? When I was working on
>> the atomic-revprops branch I asked to add it to the buildbot, perhaps we
>> should be doing this more regularly (for our feature branches) too?
> +1 for that. The closer we come to a 1.7 release, the more
> import it will be to have a certain level of confidence that
> the patch sources aren't broken.
>
Okay. I'll wait a bit for opinions, and if I don't hear objections I'll
ask infra to add this branch to the buildbot. Are there any other
branches we should consider adding to the buildbot?
> -- Stefan^2.
>
% .svn $PWD
dir=/home/daniel/src/svn/performance
% $svn di -x-p
Index: subversion/libsvn_subr/stream.c
===================================================================
--- subversion/libsvn_subr/stream.c (revision 1029087)
+++ subversion/libsvn_subr/stream.c (working copy)
@@ -459,7 +459,7 @@ stream_readline_chunky(svn_stringbuf_t **stringbuf
* If DETECT_EOL is FALSE, *EOL must point to the desired end-of-line
* indicator. STRINGBUF is allocated in POOL. */
static svn_error_t *
-stream_readline(svn_stringbuf_t **stringbuf,
+stream_readline_branch(svn_stringbuf_t **stringbuf,
svn_boolean_t *eof,
const char **eol,
svn_stream_t *stream,
@@ -499,6 +499,82 @@ static svn_error_t *
return SVN_NO_ERROR;
}
+/* Guts of svn_stream_readline() and svn_stream_readline_detect_eol().
+ * Returns the line read from STREAM in *STRINGBUF, and indicates
+ * end-of-file in *EOF. If DETECT_EOL is TRUE, the end-of-line indicator
+ * is detected automatically and returned in *EOL.
+ * If DETECT_EOL is FALSE, *EOL must point to the desired end-of-line
+ * indicator. STRINGBUF is allocated in POOL. */
+static svn_error_t *
+stream_readline_trunk(svn_stringbuf_t **stringbuf,
+ svn_boolean_t *eof,
+ const char **eol,
+ svn_stream_t *stream,
+ svn_boolean_t detect_eol,
+ apr_pool_t *pool)
+{
+ svn_stringbuf_t *str;
+ const char *eol_str;
+ apr_size_t numbytes;
+ const char *match;
+ char c;
+
+ /* Since we're reading one character at a time, let's at least
+ optimize for the 90% case. 90% of the time, we can avoid the
+ stringbuf ever having to realloc() itself if we start it out at
+ 80 chars. */
+ str = svn_stringbuf_create_ensure(80, pool);
+
+ if (detect_eol)
+ {
+ SVN_ERR(scan_eol(&eol_str, stream, pool));
+ if (eol)
+ *eol = eol_str;
+ if (! eol_str)
+ {
+ /* No newline until EOF, EOL_STR can be anything. */
+ eol_str = APR_EOL_STR;
+ }
+ }
+ else
+ eol_str = *eol;
+
+ /* Read into STR up to and including the next EOL sequence. */
+ match = eol_str;
+ while (*match)
+ {
+ numbytes = 1;
+ SVN_ERR(svn_stream_read(stream, &c, &numbytes));
+ if (numbytes != 1)
+ {
+ /* a 'short' read means the stream has run out. */
+ *eof = TRUE;
+ if (detect_eol && eol)
+ *eol = NULL;
+ *stringbuf = str;
+ return SVN_NO_ERROR;
+ }
+
+ if (c == *match)
+ match++;
+ else
+ match = eol_str;
+
+ svn_stringbuf_appendbyte(str, c);
+ }
+
+ *eof = FALSE;
+ svn_stringbuf_chop(str, match - eol_str);
+ *stringbuf = str;
+
+ return SVN_NO_ERROR;
+}
+
+#define stream_readline \
+ (getenv("USE_BRANCH_STREAM_READLINE") != NULL \
+ ? stream_readline_branch \
+ : stream_readline_trunk)
+
svn_error_t *
svn_stream_readline(svn_stream_t *stream,
svn_stringbuf_t **stringbuf,
% printenv | grep STREAM_READLINE
zsh: exit 1 grep STREAM_READLINE
% make fs-test
make: Nothing to be done for `fs-test'.
% ../runctest fs fs 22
subversion/tests/libsvn_fs/fs-test.c:3411: (apr_err=160000)
subversion/tests/libsvn_fs/fs-test.c:3229: (apr_err=160000)
svn_tests: Error validating revision 1 (youngest is 3)
subversion/tests/svn_test_fs.c:492: (apr_err=160000)
svn_tests: Repository tree does not look as expected.
Corrupt entries:
A/D/G/rho
Missing entries:
Extra entries:
FAIL: lt-fs-test 22: after each commit, check all revisions
zsh: exit 1 ../runctest fs fs 22
%
