Ben Reser wrote on Thu, Mar 13, 2014 at 16:51:58 -0700:
> Using this compile time definition causes two tests to fail in
> svnadmin_tests.py
>
> {{{
...
> }}}
>
> After some discussion on IRC I started trying to fix this by shifting the
> PACK_AFTER_EVERY_COMMIT to a fsfs.conf configuration (committed in r1577362).
> So that these tests could turn off packing after every commit to allow them to
> actually test svnadmin packing.
>
> Right now we have both PACK_AFTER_EVERY_COMMIT and a --fsfs-packing option to
> our tests. The first one inserts a pack operation as part of the transaction
> commit in the libsvn_fs library. The second one adds a post-commit hook that
> does the pack.
>
> According to Daniel, there was a bug found by PACK_AFTER_EVERY_COMMIT that the
> --fsfs-packing option didn't find. I honestly don't see how unless the test
> had its own post-commit hook, which there is a conflict. See:
> http://svn.apache.org/r875598
>
r875598 just adds PACK_AFTER_EVERY_COMMIT; the bug you refer to is the one
fixed by <http://svn.apache.org/r875597> (aka r35523). The reason post-commit
hooks didn't find that bug was that the repository in question had only one
commit, and that commit was r1, which sbox.build() created via 'svnadmin load'
--- which bypasses hooks but did trigger the C post-commit packing in
svn_fs_commit_txn().
> My inclination here is to change --fsfs-packing to simply use the new
> fsfs.conf
> option I set.
Hmm. My gut feeling is that there might be future bugs which will be caught by
running 'svnadmin pack' from a hook but not by running it directly in C; but I
don't recall if there were such bugs in the past.
In other words, I think the hook-based packing is still useful, despite
the promotion of the C-based packing (from compile-time knob in
libsvn_fs to unadvertised runtime knob in fs_fs.c).
> Change the failing tests to no longer not run with packing and instead change
> the conf to disable packing (i.e. allow individual tests to override things
> like --fsfs-packing).
+1, since those two tests concern packing specifically.
> The alternative would be to fix the test suite to support multiple bits of
> code
> for a single hook script. But that seems far more complicated.
AFAICT, the only tests that write a post-commit hook are 'hook_test' and
'post_commit_hook_test' in commit_tests.py, so we might be able to tweak those
two callsites and avoid implementing a general "Multiple post-commit callbacks"
feature in the test harness.
(I haven't checked in detail at our options: perhaps we make those tests
explicitly disable fsfs_packing's hook script, or perhaps we skip them when
fsfs_packing is set, etc.)
> Any other opinions here?
