[svn.haxx.se] · SVN Dev · SVN Users · SVN Org · TSVN Dev · TSVN Users · Subclipse Dev · Subclipse Users · this month's index

Re: svn commit: r28187 - in trunk/subversion: include libsvn_fs libsvn_fs_base libsvn_fs_fs tests/libsvn_fs

From: David Glasser <glasser_at_davidglasser.net>
Date: 2007-12-02 22:10:13 CET

Are you sure? 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. What does svn_fs_change_mergeinfo offer
that svn_fs_change_node_prop doesn't?

--dave

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.)
> >
> > 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
Received on Sun Dec 2 22:10:27 2007

This is an archived mail posted to the Subversion Dev mailing list.

This site is subject to the Apache Privacy Policy and the Apache Public Forum Archive Policy.