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 2014/09/05 13:00:09 UTC
RE: svn commit: r1622678 - in /subversion/branches/log-message-templates/subversion: include/ libsvn_client/ svn/
> -----Original Message-----
> From: stsp@apache.org [mailto:stsp@apache.org]
> Sent: vrijdag 5 september 2014 12:31
> To: commits@subversion.apache.org
> Subject: svn commit: r1622678 - in /subversion/branches/log-message-
> templates/subversion: include/ libsvn_client/ svn/
>
> Author: stsp
> Date: Fri Sep 5 10:31:11 2014
> New Revision: 1622678
>
> URL: http://svn.apache.org/r1622678
> Log:
> On the log-message-templates branch, rework the way log templates
> are passed to client API consumers.
>
> Before this change, the template texts were passed as part of the
> svn_client_commit_item3_t structure, duplicating the template text
> for each item. The API consumer was forced to loop over all commit
> items to correlate templates which apply to multiple commit items.
> Log message templates were collected during commit harvesting.
>
> Now, the templates are passed as an additional argument to the client's
> log message callback (svn_client_get_commit_log4_t). Each template has
> associated with it a list of pointers to commit items which the template
> applies to. The templates are collected during an extra post-processing
> step after commit harvesting and only for consumers of the new API.
>
> * subversion/include/svn_client.h
> (svn_client_commit_item3_t): Remove log_msg_template field.
> (svn_client_get_commit_log4_t): Revision of
> svn_client_get_commit_log3_t
> which takes a log_message_templates parameter.
> (svn_client_get_commit_log3_t): Deprecate.
> (svn_client_ctx_t): Add log_msg_func4 and log_msg_baton4 and
> deprecate log_msg_func3.
>
> * subversion/libsvn_client/client.h
> (SVN_CLIENT__HAS_LOG_MSG_FUNC): Update for log_msg_func4.
>
> * subversion/libsvn_client/commit_util.c
> (add_committable, harvest_not_present_for_copy,
> harvest_status_callback,
> handle_descendants): Remove now obsolete support for log message
> templates.
> (get_log_msg_template): Rename to ...
> (get_log_message_templates): ... this. Return a collection of templates
> which are associated with the commit items they apply to.
> (svn_client__get_log_msg): Collect log message templates if the client API
> user is providing a svn_client_get_commit_log4_t log message function.
>
> * subversion/svn/cl.h
> (svn_cl__get_log_message): This is now an implementation of
> svn_client_get_commit_log4_t.
>
> * subversion/svn/commit-cmd.c
> (svn_cl__commit): Use ctx->log_msg_baton4 instead of ctx-
> >log_msg_baton3.
>
> * subversion/svn/copy-cmd.c
> (svn_cl__copy): Use ctx->log_msg_baton4 instead of ctx->log_msg_baton3.
>
> * subversion/svn/delete-cmd.c
> (svn_cl__delete): Use ctx->log_msg_baton4 instead of ctx-
> >log_msg_baton3.
>
> * subversion/svn/import-cmd.c
> (svn_cl__import): Use ctx->log_msg_baton4 instead of ctx-
> >log_msg_baton3.
>
> * subversion/svn/mkdir-cmd.c
> (svn_cl__mkdir): Use ctx->log_msg_baton4 instead of ctx-
> >log_msg_baton3.
>
> * subversion/svn/move-cmd.c
> (svn_cl__move): Use ctx->log_msg_baton4 instead of ctx-
> >log_msg_baton3.
>
> * subversion/svn/propedit-cmd.c
> (svn_cl__propedit): Use ctx->log_msg_baton4 instead of ctx-
> >log_msg_baton3.
>
> * subversion/svn/svn.c
> (sub_main): Use ctx->log_msg_func4 instead of ctx->log_msg_func3.
>
> * subversion/svn/util.c
> (svn_cl__get_log_message): Implement the svn_client_get_commit_log4_t
> interface. Provide the same behaviour as get_log_message_template().
> (get_log_message_template): Remove.
>
> Modified:
> subversion/branches/log-message-
> templates/subversion/include/svn_client.h
> subversion/branches/log-message-
> templates/subversion/libsvn_client/client.h
> subversion/branches/log-message-
> templates/subversion/libsvn_client/commit_util.c
> subversion/branches/log-message-templates/subversion/svn/cl.h
> subversion/branches/log-message-templates/subversion/svn/commit-
> cmd.c
> subversion/branches/log-message-templates/subversion/svn/copy-cmd.c
> subversion/branches/log-message-templates/subversion/svn/delete-
> cmd.c
> subversion/branches/log-message-templates/subversion/svn/import-
> cmd.c
> subversion/branches/log-message-templates/subversion/svn/mkdir-
> cmd.c
> subversion/branches/log-message-templates/subversion/svn/move-
> cmd.c
> subversion/branches/log-message-templates/subversion/svn/propedit-
> cmd.c
> subversion/branches/log-message-templates/subversion/svn/svn.c
> subversion/branches/log-message-templates/subversion/svn/util.c
>
> Modified: subversion/branches/log-message-
> templates/subversion/include/svn_client.h
> URL: http://svn.apache.org/viewvc/subversion/branches/log-message-
> templates/subversion/include/svn_client.h?rev=1622678&r1=1622677&r2=1
> 622678&view=diff
> ==========================================================
> ====================
> --- subversion/branches/log-message-
> templates/subversion/include/svn_client.h (original)
> +++ subversion/branches/log-message-
> templates/subversion/include/svn_client.h Fri Sep 5 10:31:11 2014
> @@ -528,13 +528,6 @@ typedef struct svn_client_commit_item3_t
> */
> const char *moved_from_abspath;
>
> - /** Log message template associated with this item (if any; NULL
> - * otherwise).
> - *
> - * @since New in 1.9.
> - */
> - const svn_string_t *log_msg_template;
> -
> } svn_client_commit_item3_t;
>
> /** The commit candidate structure.
> @@ -660,11 +653,31 @@ svn_client_commit_item2_dup(const svn_cl
> * structures, which may be fully or only partially filled-in,
> * depending on the type of commit operation.
> *
> + * @a log_message_templates is a hash table containing one or more log
> + * message templates obtained from svn:log-message properties applicable
> + * to @a commit_items. It is @c NULL if no log message template is defined.
> + * The table is keyed by 'const char *' log templates. Its values are
> + * apr_array_header_t * arrays which contain pointers to those commit
> + * items (from @a commit_items) which the log template applies to.
> + *
> * @a baton is provided along with the callback for use by the handler.
> *
> * All allocations should be performed in @a pool.
> *
> + * @since New in 1.9.
> + */
> +typedef svn_error_t *(*svn_client_get_commit_log4_t)(
> + const char **log_msg,
> + const char **tmp_file,
> + const apr_array_header_t *commit_items,
> + const apr_hash_t * log_message_templates,
> + void *baton,
> + apr_pool_t *pool);
Why do we need a tmp file in the api? GUI clients in general don't need/want a temp file.
This looks like you are trying to implement 'svn' behavior in the svn_client layer.
I don't see why the tmp_file can't be passed via the baton... if needed by wrapping one baton in another at some helper function layer.
The documentation you added doesn't document tmp_file and the documentation for log_message_templates doesn't explain what it does.
E.g. If I'm committing changes to "C:\inetpub\wwwroot\some-file.aspx" and "F:\projects\libraries\some-file.c" what can I expect here?
The nodes are certainly not in the same working copy, but can be from the same repository and as such committed to a single revision. What keys can I expect in log_message_templates. (Which can't be const, as there are no apis that can consume const apr_hash_t * :( )... and what values?
Currently I would read the documentation as const char * "<template>" mapping to 'commit items'... But an apr_hash_t has a one to one relation, so it could only point to one.
Bert
Re: svn commit: r1622678 - in
/subversion/branches/log-message-templates/subversion: include/
libsvn_client/ svn/
Posted by Stefan Sperling <st...@elego.de>.
On Fri, Sep 05, 2014 at 01:00:09PM +0200, Bert Huijben wrote:
> Why do we need a tmp file in the api? GUI clients in general don't need/want a temp file.
>
> This looks like you are trying to implement 'svn' behavior in the svn_client layer.
>
> I don't see why the tmp_file can't be passed via the baton... if needed by wrapping one baton in another at some helper function layer.
>
> The documentation you added doesn't document tmp_file and the documentation for log_message_templates doesn't explain what it does.
The temp file already existed in svn_client_get_commit_log3_t.
I didn't add this parameter and I didn't add documentation for it.
> E.g. If I'm committing changes to "C:\inetpub\wwwroot\some-file.aspx" and "F:\projects\libraries\some-file.c" what can I expect here?
>
> The nodes are certainly not in the same working copy, but can be from the same repository and as such committed to a single revision. What keys can I expect in log_message_templates. (Which can't be const, as there are no apis that can consume const apr_hash_t * :( )... and what values?
What to do about multi-WC commits is a good question.
I'll think about that.
> Currently I would read the documentation as const char * "<template>" mapping to 'commit items'... But an apr_hash_t has a one to one relation, so it could only point to one.
cmpilato used a mapping from log template to a list of commit items.
I'm considering alternatives right now but wanted to change the way
the templates are passed to the API consumer first.
Do you agree that passing templates to the callback is better?
This commit didn't really want to change much more than that.