Wow. Thanks for the very detailed and on the spot explanation.
On Monday, August 11, 2014 01:50:48 PM Philip Martin wrote:
> Ben Reser <b...@reser.org> writes:
> > I haven't dug in to the specifics of how we broke this particular use
> > case.
> > But I suspect you'll find that if you create a transaction, call
> > svn_fs_change_node_prop() and commit the transaction that this call works
> > just fine. So I'd bet that the problem only happens when you try to do
> > this from the pre-commit hook.
>
> A post-commit should not change the versioned data because it confuses
> the client. However a script in general should be able to modify a
> versioned property and it's perfectly possible to write a python script
> that does it. I think the problem here is that the process committing
> the transaction has populated the fs_fs_data_t.txn_dir_cache and this
> cache gets outdated when the post-commit runs in a separate process.
>
> Consider the test I added in 1617263, it's one process and so doesn't
> demonstrate the problem:
>
> + /* Create txn with changes. */
> + SVN_ERR(svn_fs_begin_txn(&txn, fs, head_rev, pool));
> + SVN_ERR(svn_fs_txn_name(&txn_name, txn, pool));
> + SVN_ERR(svn_fs_txn_root(&root, txn, pool));
> + SVN_ERR(svn_fs_make_dir(root, "X", pool));
> +
> + /* Reopen, add more changes. */
> + SVN_ERR(svn_fs_open_txn(&reopen_txn, fs, txn_name, pool));
> + SVN_ERR(svn_fs_txn_root(&reopen_root, reopen_txn, pool));
> + SVN_ERR(svn_fs_change_node_prop(reopen_root, "A", "name",
> + svn_string_create("value", pool),
> + pool));
> +
> + /* Reopen, commit */
> + SVN_ERR(svn_fs_open_txn(&reopen_txn, fs, txn_name, pool));
> + SVN_ERR(test_commit_txn(&head_rev, reopen_txn, NULL, pool));
>
> If I remove the svn_fs_change_node_prop() line from the test, run the
> test in gdb and stop at the commit I can then run a python script in
> another process to make the equivalent property change. Then I resume
> the test and the commit includes the expected property change.
>
> However if I also remove the svn_fs_txn_root() call that reopens the
> transaction in addition to the svn_fs_change_node_prop() call then the
> commit will lose the property change made externally. In both cases the
> property change is present in the transaction on disk but if the
> txn_dir_cache is out-of-date the property change gets lost.
>
> svn_fs_txn_root() calls svn_fs_fs__initialize_txn_caches() and if that
> detects concurrent transactions it sets ffd->txn_dir_cache to NULL so
> the out-of-date cache doesn't get used and the property change on disk
> makes it into the revision.
Indeed, calling svn_fs_txn_root() after execution of the pre-commit hook in
svn_repos_fs_commit_txn() makes Subversion behave in the same way as it did in
1.6.
Still, as far as I understand, Subversion is not going to have
svn_repos_fs_commit_txn()
reload the directory cache for the transaction (unless some future release of
Subversion
allows the pre-commit script to modify the transaction and syncs the client
caches with
such modifications :P). So I would still appreciate suggestions as to how
Subversion 1.7+
hooks should be configured to handle this use case:
I want to have one file, /project/trunk/include/version.h, reflect the last
revision that
touched any file in the project. For that purpose, pre-commit hook (on a server
currently
using SVN 1.6) checks if any file under /project/trunk is modified and if it's
the case,
modifies a property on the /project/trunk/include/version.h. This in turn
causes $Revision$
in <version.h> to reflect the last revision when ANY file in the project was
modified, not
when the <version.h> itself was modified.
(the other use case I for 1.6 behavior that I previously described can be
easily modified to
use a combination of pre-commit and post-commit hooks, so that is not a big
issue).
Thanks,
Alexey.
