On Sun, 02 Dec 2007, David Glasser wrote:
> Are you sure?
Just noticed this in passing. I am only sure that having a generic libsvn_fs
interface for changing mergeinfo is useful.
> As far as I can tell the API as currently implemented
> was pretty buggy. It would update the data that the FS implementation
> sent to the sqlite backend without actually updating the svn:mergeinfo
> properties in the FS itself.
svn log -r 19643 http://svn.collab.net/repos/svn/branches/merge-tracking/subversion/include/svn_fs.h@24027
------------------------------------------------------------------------
r19643 | dberlin | 2006-05-15 07:23:34 -0700 (Mon, 15 May 2006) | 8 lines
(I will propedit this later into a proper changelog)
Beginnings of merge tracking support
This is not a proper changelog yet, because this stuff is in massive flux.
Added configure support for sqlite.
Added support for recording when merge info has changed.
Added mergeinfo types.
Added fs calls for mergeinfo setting and getting.
------------------------------------------------------------------------
Yeah, okay. And it looks pretty old.
> What does svn_fs_change_mergeinfo offer
> that svn_fs_change_node_prop doesn't?
That sounds like a sufficient interface to me. One suggestion: instead of
removing that C test case, keep it around, and have it call
svn_fs_change_node_prop().
Thanks, Dan
p.s. One more comment inline below.
> On Dec 2, 2007 1:29 PM, Daniel Rall <dlr@collab.net> wrote:
> > Dave, IIRC, this API is necessary for third-party FS backends to offer
> > support for modifying mergeinfo. We know of at least two such FS backends,
> > code.google.com's, and an Oracle-based implementation.
> >
> >
> > On Sun, 02 Dec 2007, glasser@tigris.org wrote:
> >
> > > Author: glasser
> > > Date: Sun Dec 2 11:54:28 2007
> > > New Revision: 28187
> > >
> > > Log:
> > > Remove the almost-completely-unused svn_fs_change_mergeinfo API.
> > > (This should be done on trunk anyway if this branch is rejected.)
Dave, did you mean to do this on a branch? The commit was to trunk. (Perhaps
the change log is just out of date.)
> > > Why "almost"? Well, there's a C test that uses it, but that C test is
> > > pretty wacky already (the mergeinfo it sets is invalid, it does a
> > > svn_fs_get_mergeinfo on a non-existing path, etc). And it doesn't
> > > actually verify anything about the return value from
> > > svn_fs_get_mergeinfo. So ditch the test too.
> > >
> > > (This change is similar to r28146 on the
> > > sqlite-mergeinfo-without-mergeinfo branch; I should have done it on
> > > trunk in the first place.)
> > >
> > > * subversion/include/svn_fs.h
> > > (svn_fs_change_mergeinfo): Remove.
> > >
> > > * subversion/libsvn_fs/fs-loader.c
> > > (svn_fs_change_mergeinfo): Remove.
> > >
> > > * subversion/libsvn_fs/fs-loader.h
> > > (root_vtable_t): Remove change_mergeinfo.
> > >
> > > * subversion/libsvn_fs_base/tree.c
> > > (change_mergeinfo_args, txn_body_change_mergeinfo,
> > > base_change_mergeinfo): Remove.
> > > (root_vtable): Remove base_change_mergeinfo.
> > >
> > > * subversion/libsvn_fs_fs/tree.c
> > > (fs_change_mergeinfo): Remove.
> > > (root_vtable): Remove fs_change_mergeinfo.
> > >
> > > * subversion/tests/libsvn_fs/fs-test.c
> > > (get_mergeinfo): Remove.
> > > (test_funcs): Remove get_mergeinfo.
> > >
> > >
> > > Modified:
> > > trunk/subversion/include/svn_fs.h
> > > trunk/subversion/libsvn_fs/fs-loader.c
> > > trunk/subversion/libsvn_fs/fs-loader.h
> > > trunk/subversion/libsvn_fs_base/tree.c
> > > trunk/subversion/libsvn_fs_fs/tree.c
> > > trunk/subversion/tests/libsvn_fs/fs-test.c
> > >
> > > Modified: trunk/subversion/include/svn_fs.h
> > > URL: http://svn.collab.net/viewvc/svn/trunk/subversion/include/svn_fs.h?pathrev=28187&r1=28186&r2=28187
> > > ==============================================================================
> > > --- trunk/subversion/include/svn_fs.h (original)
> > > +++ trunk/subversion/include/svn_fs.h Sun Dec 2 11:54:28 2007
> > > @@ -1222,22 +1222,6 @@
> > > const char *path,
> > > apr_pool_t *pool);
> > >
> > > -/** Change a node's mergeinfo
> > > - *
> > > - * - @a root and @a path indicate the node whose property should change.
> > > - * @a root must be the root of a transaction, not the root of a
> > > - * revision.
> > > - * - @a mergeinhash is the new value of the mergeinfo for PATH, or NULL if
> > > - * the mergeinfo for that path should be removed altogether.
> > > - *
> > > - * Do any necessary temporary allocation in @a pool.
> > > - *
> > > - * @since New in 1.5.
> > > - */
> > > -svn_error_t *svn_fs_change_mergeinfo(svn_fs_root_t *root,
> > > - const char *path,
> > > - apr_hash_t *mergeinhash,
> > > - apr_pool_t *pool);
> > >
> > > /** Retrieve mergeinfo for multiple nodes.
> > > *
> > >
> > > Modified: trunk/subversion/libsvn_fs/fs-loader.c
> > > URL: http://svn.collab.net/viewvc/svn/trunk/subversion/libsvn_fs/fs-loader.c?pathrev=28187&r1=28186&r2=28187
> > > ==============================================================================
> > > --- trunk/subversion/libsvn_fs/fs-loader.c (original)
> > > +++ trunk/subversion/libsvn_fs/fs-loader.c Sun Dec 2 11:54:28 2007
> > > @@ -814,14 +814,6 @@
> > > }
> > >
> > > svn_error_t *
> > > -svn_fs_change_mergeinfo(svn_fs_root_t *root, const char *path,
> > > - apr_hash_t *minfo,
> > > - apr_pool_t *pool)
> > > -{
> > > - return root->vtable->change_mergeinfo(root, path, minfo, pool);
> > > -}
> > > -
> > > -svn_error_t *
> > > svn_fs_get_mergeinfo(apr_hash_t **minfohash,
> > > svn_fs_root_t *root,
> > > const apr_array_header_t *paths,
> > >
> > > Modified: trunk/subversion/libsvn_fs/fs-loader.h
> > > URL: http://svn.collab.net/viewvc/svn/trunk/subversion/libsvn_fs/fs-loader.h?pathrev=28187&r1=28186&r2=28187
> > > ==============================================================================
> > > --- trunk/subversion/libsvn_fs/fs-loader.h (original)
> > > +++ trunk/subversion/libsvn_fs/fs-loader.h Sun Dec 2 11:54:28 2007
> > > @@ -307,9 +307,6 @@
> > > svn_fs_root_t *ancestor_root,
> > > const char *ancestor_path,
> > > apr_pool_t *pool);
> > > - svn_error_t *(*change_mergeinfo)(svn_fs_root_t *root, const char *path,
> > > - apr_hash_t *info,
> > > - apr_pool_t *pool);
> > > svn_error_t *(*get_mergeinfo)(apr_hash_t **minfohash,
> > > svn_fs_root_t *root,
> > > const apr_array_header_t *paths,
> > >
> > > Modified: trunk/subversion/libsvn_fs_base/tree.c
> > > URL: http://svn.collab.net/viewvc/svn/trunk/subversion/libsvn_fs_base/tree.c?pathrev=28187&r1=28186&r2=28187
> > > ==============================================================================
> > > --- trunk/subversion/libsvn_fs_base/tree.c (original)
> > > +++ trunk/subversion/libsvn_fs_base/tree.c Sun Dec 2 11:54:28 2007
> > > @@ -1196,47 +1196,6 @@
> > > return SVN_NO_ERROR;
> > > }
> > >
> > > -/* The input for txn_body_change_mergeinfo(). */
> > > -struct change_mergeinfo_args
> > > -{
> > > - svn_fs_root_t *root;
> > > - const char *path;
> > > - const svn_string_t *value;
> > > -};
> > > -
> > > -/* Set the mergeinfo on the transaction in BATON (expected to be of
> > > - type "struct change_mergeinfo_args"). Conforms to the callback
> > > - API used by svn_fs_base__retry_txn(). */
> > > -static svn_error_t *
> > > -txn_body_change_mergeinfo(void *baton,
> > > - trail_t *trail)
> > > -{
> > > - struct change_mergeinfo_args *args = baton;
> > > - SVN_ERR(svn_fs_base__set_txn_mergeinfo(args->root->fs, args->root->txn,
> > > - args->path, args->value,
> > > - trail, trail->pool));
> > > - return SVN_NO_ERROR;
> > > -}
> > > -
> > > -/* Change the mergeinfo for the specified PATH to MERGE_INFO. */
> > > -static svn_error_t *
> > > -base_change_mergeinfo(svn_fs_root_t *root,
> > > - const char *path,
> > > - apr_hash_t *mergeinfo,
> > > - apr_pool_t *pool)
> > > -{
> > > - struct change_mergeinfo_args args;
> > > -
> > > - if (! root->is_txn_root)
> > > - return SVN_FS__NOT_TXN(root);
> > > - args.root = root;
> > > - args.path = path;
> > > - SVN_ERR(svn_mergeinfo__to_string((svn_string_t **) &args.value, mergeinfo,
> > > - pool));
> > > - return svn_fs_base__retry_txn(root->fs, txn_body_change_mergeinfo, &args,
> > > - pool);
> > > -}
> > > -
> > > struct change_node_prop_args {
> > > svn_fs_root_t *root;
> > > const char *path;
> > > @@ -4632,7 +4591,6 @@
> > > base_contents_changed,
> > > base_get_file_delta_stream,
> > > base_merge,
> > > - base_change_mergeinfo,
> > > svn_fs_mergeinfo__get_mergeinfo,
> > > svn_fs_mergeinfo__get_mergeinfo_for_tree
> > > };
> > >
> > > Modified: trunk/subversion/libsvn_fs_fs/tree.c
> > > URL: http://svn.collab.net/viewvc/svn/trunk/subversion/libsvn_fs_fs/tree.c?pathrev=28187&r1=28186&r2=28187
> > > ==============================================================================
> > > --- trunk/subversion/libsvn_fs_fs/tree.c (original)
> > > +++ trunk/subversion/libsvn_fs_fs/tree.c Sun Dec 2 11:54:28 2007
> > > @@ -1020,31 +1020,6 @@
> > > }
> > >
> > >
> > > -/* Change the mergeinfo for a given path. */
> > > -static svn_error_t *
> > > -fs_change_mergeinfo(svn_fs_root_t *root,
> > > - const char *path,
> > > - apr_hash_t *mergeinfo,
> > > - apr_pool_t *pool)
> > > -{
> > > - const char *txn_id;
> > > - svn_string_t *mergeinfo_str;
> > > - svn_fs_txn_t *txn;
> > > -
> > > - if (! root->is_txn_root)
> > > - return SVN_FS__NOT_TXN(root);
> > > - txn_id = root->txn;
> > > - SVN_ERR(root->fs->vtable->open_txn(&txn, root->fs, txn_id, pool));
> > > - SVN_ERR(svn_mergeinfo__to_string(&mergeinfo_str, mergeinfo, pool));
> > > - SVN_ERR(svn_fs_fs__change_txn_mergeinfo(txn, path, mergeinfo_str, pool));
> > > -
> > > - SVN_ERR(svn_fs_fs__change_txn_prop(txn,
> > > - SVN_FS__PROP_TXN_CONTAINS_MERGEINFO,
> > > - svn_string_create("true", pool),
> > > - pool));
> > > - return SVN_NO_ERROR;
> > > -}
> > > -
> > > /* Change, add, or delete a node's property value. The affected node
> > > is PATH under ROOT, the property value to modify is NAME, and VALUE
> > > points to either a string value to set the new contents to, or NULL
> > > @@ -3323,7 +3298,6 @@
> > > fs_contents_changed,
> > > fs_get_file_delta_stream,
> > > fs_merge,
> > > - fs_change_mergeinfo,
> > > svn_fs_mergeinfo__get_mergeinfo,
> > > svn_fs_mergeinfo__get_mergeinfo_for_tree
> > > };
> > >
> > > Modified: trunk/subversion/tests/libsvn_fs/fs-test.c
> > > URL: http://svn.collab.net/viewvc/svn/trunk/subversion/tests/libsvn_fs/fs-test.c?pathrev=28187&r1=28186&r2=28187
> > > ==============================================================================
> > > --- trunk/subversion/tests/libsvn_fs/fs-test.c (original)
> > > +++ trunk/subversion/tests/libsvn_fs/fs-test.c Sun Dec 2 11:54:28 2007
> > > @@ -4556,78 +4556,6 @@
> > > return SVN_NO_ERROR;
> > > }
> > >
> > > -/* Test getting mergeinfo */
> > > -static svn_error_t *
> > > -get_mergeinfo(const char **msg,
> > > - svn_boolean_t msg_only,
> > > - svn_test_opts_t *opts,
> > > - apr_pool_t *pool)
> > > -{
> > > - svn_fs_t *fs;
> > > - svn_fs_txn_t *txn;
> > > - svn_fs_root_t *txn_root, *revision_root;
> > > - svn_revnum_t before_rev, after_rev;
> > > - apr_hash_t *result;
> > > - apr_array_header_t *paths;
> > > - apr_hash_t *mergeinfo;
> > > - const char *conflict;
> > > -
> > > - *msg = "get mergeinfo";
> > > -
> > > - if (msg_only)
> > > - return SVN_NO_ERROR;
> > > -
> > > -
> > > - /* Prepare a filesystem. */
> > > - SVN_ERR(svn_test__create_fs(&fs, "test-repo-get-mergeinfo",
> > > - opts->fs_type, pool));
> > > -
> > > - /* Save the current youngest revision. */
> > > - SVN_ERR(svn_fs_youngest_rev(&before_rev, fs, pool));
> > > -
> > > - /* Prepare a txn to receive the greek tree. */
> > > - SVN_ERR(svn_fs_begin_txn(&txn, fs, 0, pool));
> > > - SVN_ERR(svn_fs_txn_root(&txn_root, txn, pool));
> > > -
> > > - /* Paranoidly check that the current youngest rev is unchanged. */
> > > - SVN_ERR(svn_fs_youngest_rev(&after_rev, fs, pool));
> > > - if (after_rev != before_rev)
> > > - return svn_error_create
> > > - (SVN_ERR_FS_GENERAL, NULL,
> > > - "youngest revision changed unexpectedly");
> > > -
> > > - /* Create the greek tree. */
> > > - SVN_ERR(svn_test__create_greek_tree(txn_root, pool));
> > > -
> > > - SVN_ERR(svn_mergeinfo_parse(&mergeinfo, "/A/E: 1-5", pool));
> > > - SVN_ERR(svn_fs_change_mergeinfo(txn_root, "/A/B", mergeinfo, pool));
> > > -
> > > - /* Commit it. */
> > > - SVN_ERR(svn_fs_commit_txn(&conflict, &after_rev, txn, pool));
> > > -
> > > - /* Make sure it's a different revision than before. */
> > > - if (after_rev == before_rev)
> > > - return svn_error_create
> > > - (SVN_ERR_FS_GENERAL, NULL,
> > > - "youngest revision failed to change");
> > > -
> > > - /* Get root of the revision */
> > > - SVN_ERR(svn_fs_revision_root(&revision_root, fs, after_rev, pool));
> > > -
> > > - /* Check the tree. */
> > > - SVN_ERR(svn_test__check_greek_tree(revision_root, pool));
> > > -
> > > - paths = apr_array_make(pool, 1, sizeof (const char *));
> > > - APR_ARRAY_PUSH(paths, const char *) = "/A/E";
> > > - SVN_ERR(svn_fs_get_mergeinfo(&result, revision_root, paths,
> > > - svn_mergeinfo_inherited, pool));
> > > - paths = apr_array_make(pool, 1, sizeof (const char *));
> > > - APR_ARRAY_PUSH(paths, const char *) = "/A/B/E";
> > > - SVN_ERR(svn_fs_get_mergeinfo(&result, revision_root, paths,
> > > - svn_mergeinfo_inherited, pool));
> > > - return SVN_NO_ERROR;
> > > -}
> > > -
> > > static svn_error_t *
> > > root_revisions(const char **msg,
> > > svn_boolean_t msg_only,
> > > @@ -4999,7 +4927,6 @@
> > > SVN_TEST_PASS(branch_test),
> > > SVN_TEST_PASS(verify_checksum),
> > > SVN_TEST_PASS(closest_copy_test),
> > > - SVN_TEST_PASS(get_mergeinfo),
> > > SVN_TEST_PASS(root_revisions),
> > > SVN_TEST_PASS(unordered_txn_dirprops),
> > > SVN_TEST_PASS(set_uuid),
> > >
> > > ---------------------------------------------------------------------
> > > To unsubscribe, e-mail: svn-unsubscribe@subversion.tigris.org
> > > For additional commands, e-mail: svn-help@subversion.tigris.org
> >
> > --
> >
> > Daniel Rall
> >
>
>
>
> --
> David Glasser | glasser_at_davidglasser.net | http://www.davidglasser.net/
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
> For additional commands, e-mail: dev-help@subversion.tigris.org
--
Daniel Rall
- application/pgp-signature attachment: stored
Received on Mon Dec 3 06:39:34 2007