You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@subversion.apache.org by Bert Huijben <be...@qqmail.nl> on 2016/01/11 10:48:28 UTC
RE: svn commit: r1723900 - in /subversion/branches/fs-node-api/subversion: include/svn_fs.h libsvn_fs/fs-loader.c libsvn_fs/fs-loader.h libsvn_fs/node_compat.c libsvn_fs_fs/node.c tests/libsvn_fs/fs-test.c
A quick review showed one minor issue, see inline.
> -----Original Message-----
> From: kotkov@apache.org [mailto:kotkov@apache.org]
> Sent: zondag 10 januari 2016 04:21
> To: commits@subversion.apache.org
> Subject: svn commit: r1723900 - in /subversion/branches/fs-node-
> api/subversion: include/svn_fs.h libsvn_fs/fs-loader.c libsvn_fs/fs-loader.h
> libsvn_fs/node_compat.c libsvn_fs_fs/node.c tests/libsvn_fs/fs-test.c
>
> Author: kotkov
> Date: Sun Jan 10 03:20:57 2016
> New Revision: 1723900
>
> URL: http://svn.apache.org/viewvc?rev=1723900&view=rev
> Log:
> On 'fs-node-api' branch: Lay the groundwork for switching the update
> reporter
> to the new FS node API.
>
> Provide new versions for almost all existing FS API functions used by the
> reporter that accept a svn_fs_node_t instead of a (svn_fs_root, path) pair,
> and thus avoid a DAG walk:
>
> svn_fs_node_relation2()
> svn_fs_props_different2()
> svn_fs_file_checksum2()
> svn_fs_file_contents2()
> svn_fs_contents_different2()
>
> Add native implementations for these functions in libsvn_fs_fs/node.c,
> and add a compatible implementation in libsvn_fs/node_compat.c.
>
> * subversion/include/svn_fs.h
> (svn_fs_node_relation2, svn_fs_props_different2, svn_fs_file_checksum2,
> svn_fs_file_contents2, svn_fs_contents_different2): New, work with
> svn_fs_node_t instead of a svn_fs_root_t and a path. Revved from
> these ...
> (svn_fs_node_relation, svn_fs_props_different, svn_fs_file_checksum,
> svn_fs_file_contents, svn_fs_contents_different): ...functions.
>
> * subversion/libsvn_fs/fs-loader.h
> (struct node_vtable_t): Add new node_relation, props_changed,
> file_checksum,
> file_contents and contents_changed vtable members.
>
> * subversion/libsvn_fs/fs-loader.c
> (svn_fs_node_relation2, svn_fs_props_different2, svn_fs_file_checksum2,
> svn_fs_file_contents2, svn_fs_contents_different2): Implement these
> new
> functions.
>
> * subversion/libsvn_fs/node_compat.c
> (compat_fs_node_relation, compat_fs_node_props_changed,
> compat_fs_node_file_checksum, compat_fs_node_file_contents,
> compat_fs_node_contents_changed): Implement these compatibility
> wrappers.
> Open a svn_fs_root_t and call the corresponding function from its vtable.
> (compat_node_vtable): Update this vtable instance.
>
> * subversion/libsvn_fs_fs/node.c
> (fs_node_relation, fs_node_props_changed, fs_node_file_checksum,
> fs_node_file_contents, fs_node_contents_changed): Implement these
> natively
> by forwarding the call to corresponding svn_fs_fs__dag functions.
> (fs_node_vtable): Update this vtable instance.
>
> * subversion/tests/libsvn_fs/fs-test.c
> (check_related, check_txn_related): Extend these tests to also call and
> verify the result of svn_fs_node_relation2().
>
> Modified:
> subversion/branches/fs-node-api/subversion/include/svn_fs.h
> subversion/branches/fs-node-api/subversion/libsvn_fs/fs-loader.c
> subversion/branches/fs-node-api/subversion/libsvn_fs/fs-loader.h
> subversion/branches/fs-node-api/subversion/libsvn_fs/node_compat.c
> subversion/branches/fs-node-api/subversion/libsvn_fs_fs/node.c
> subversion/branches/fs-node-api/subversion/tests/libsvn_fs/fs-test.c
>
> Modified: subversion/branches/fs-node-api/subversion/include/svn_fs.h
> URL: http://svn.apache.org/viewvc/subversion/branches/fs-node-
> api/subversion/include/svn_fs.h?rev=1723900&r1=1723899&r2=1723900&vie
> w=diff
> ==========================================================
> ====================
> --- subversion/branches/fs-node-api/subversion/include/svn_fs.h (original)
> +++ subversion/branches/fs-node-api/subversion/include/svn_fs.h Sun Jan
> 10 03:20:57 2016
<snip>
> @@ -2399,6 +2422,17 @@ svn_fs_file_length(svn_filesize_t *lengt
> * @since New in 1.6.
> */
> svn_error_t *
> +svn_fs_file_checksum2(svn_checksum_t **checksum,
> + svn_checksum_kind_t kind,
> + svn_fs_node_t *root,
> + svn_boolean_t force,
> + apr_pool_t *pool);
Wrong argument name here.
(And the @since should have moved to the legacy function, etc.)
> +
> +/**
> + * Same as svn_fs_file_checksum2(), but reference node by @a root and
> + * @a path.
> + */
> +svn_error_t *
> svn_fs_file_checksum(svn_checksum_t **checksum,
> svn_checksum_kind_t kind,
> svn_fs_root_t *root,
> @@ -2422,21 +2456,29 @@ svn_fs_file_md5_checksum(unsigned char d
Bert
Re: svn commit: r1723900 - in /subversion/branches/fs-node-api/subversion:
include/svn_fs.h libsvn_fs/fs-loader.c libsvn_fs/fs-loader.h
libsvn_fs/node_compat.c libsvn_fs_fs/node.c tests/libsvn_fs/fs-test.c
Posted by Evgeny Kotkov <ev...@visualsvn.com>.
Bert Huijben <be...@qqmail.nl> writes:
> A quick review showed one minor issue, see inline.
[...]
>> @@ -2399,6 +2422,17 @@ svn_fs_file_length(svn_filesize_t *lengt
>> * @since New in 1.6.
>> */
>> svn_error_t *
>> +svn_fs_file_checksum2(svn_checksum_t **checksum,
>> + svn_checksum_kind_t kind,
>> + svn_fs_node_t *root,
>> + svn_boolean_t force,
>> + apr_pool_t *pool);
>
> Wrong argument name here.
> (And the @since should have moved to the legacy function, etc.)
Thanks for the review. Should be fixed by r1724002 and r1724003.
Regards,
Evgeny Kotkov