On Tue, 2010-05-04, hwright_at_apache.org wrote:
> Author: hwright
> Date: Tue May 4 09:45:43 2010
> New Revision: 940786
>
> URL: http://svn.apache.org/viewvc?rev=940786&view=rev
> Log:
> Add a callback to the public patch API, to allow consumers to collect
> information about the patch targets. This replaces the reject_tempfiles
> and patched_tempfiles return parameters.
>
> * subversion/tests/libsvn_client/client-test.c
> (patch_collection_baton, patch_collection_func): New.
> (test_patch): Use the baton to collect the tested information.
>
> * subversion/svn/patch-cmd.c
> (svn_cl__patch): Remove the tempfiles, and don't implement a patch callback.
>
> * subversion/include/svn_client.h
> (svn_client_patch_func_t): New.
> (svn_client_patch): Remove the output hashes, and add a callback and baton.
>
> * subversion/libsvn_client/patch.c
> (init_patch_target): Pass through the REMOVE_TEMPFILES param.
> (apply_one_patch): Adjust parameters, and call the callback, where
> appropriate.
> (apply_patches_baton_t): Adjust members to refer to the updated parameters.
> (apply_patches): Pass through parameters to apply_one_patch().
> (svn_client_patch): Set the updated baton parameters.
>
> Modified:
> subversion/trunk/subversion/include/svn_client.h
> subversion/trunk/subversion/libsvn_client/patch.c
> subversion/trunk/subversion/svn/patch-cmd.c
> subversion/trunk/subversion/tests/libsvn_client/client-test.c
Just looking at the API change...
> Modified: subversion/trunk/subversion/include/svn_client.h
> URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/include/svn_client.h?rev=940786&r1=940785&r2=940786&view=diff
> ==============================================================================
> --- subversion/trunk/subversion/include/svn_client.h (original)
> +++ subversion/trunk/subversion/include/svn_client.h Tue May 4 09:45:43 2010
> @@ -4833,6 +4833,25 @@ svn_client_info(const char *path_or_url,
> */
>
> /**
> + * The callback invoked by svn_client_patch(). For each patch target,
> + * call this function for @a local_abspath, and return the @a patch_abspath
> + * and @a reject_abspath. Neither @a patch_abspath or @a reject_abspath are
> + * guaranteed to exist (depending on the @a remove_tempfiles parameter for
> + * svn_client_patch() ).
Jumping in at this point, I can't grok this doc string. Can you write
it as a statement of what this function (a function of this type) is
required to do or promises to do or is expected to do? Try not to
assume the meanings of the parameters are totally obvious, and avoid
saying "return" when describing a parameter. ("const char *" is not
normally associated with "return" - are those parameters outputs?)
For example, is it guaranteed to be called before or after the target is
patched? Is function able to examine the patch? Is it allowed to
modify the target file before patching? Should it or can it attempt to
apply the patch itself, or adjust the patch, or reject the patch, or
cause this patch to get applied to a different target? How can this
function, or how can the caller of svn_client_patch(), know whether
patch application has been successful?
> + *
> + * The const char * parameters may be allocated in @a scratch_pool which
> + * will be cleared after each invocation.
> + *
> + * @since New in 1.7.
> + */
> +typedef svn_error_t *(*svn_client_patch_func_t)(
> + void *baton,
> + const char *local_abspath,
> + const char *patch_abspath,
> + const char *reject_abspath,
> + apr_pool_t *scratch_pool);
> +
> +/**
> * Apply a unidiff patch that's located at absolute path
> * @a abs_patch_path to the working copy at @a local_abspath.
> *
> @@ -4867,28 +4886,16 @@ svn_client_info(const char *path_or_url,
> * the @a include_patterns are applied first, i.e. the @a exclude_patterns
> * are applied to all targets which matched one of the @a include_patterns.
> *
> - * If @a patched_tempfiles is not NULL, return in @a *patched_tempfiles
> - * a mapping {target path -> path to temporary file containing patched result}
> - * for all patched targets which were neither skipped nor excluded via
> - * @a include_patterns or @a exclude_patterns.
> - * Note that if all hunks were rejected, the patched result will look just
> - * like the target file, unmodified.
> - * If @a reject_tempfiles is not NULL, return in @a *reject_tempfiles
> - * a mapping {target path -> path to temporary file containing rejected hunks}
> - * Both @a *patched_tempfiles and @a *reject_tempfiles are allocated in
> - * @a result_pool, and the key (target path) used is the path as parsed
> - * from the patch, but in canonicalized form. The value (path to temporary
> - * file) is an absolute path, also in canonicalized form.
> - * The temporary files are closed, and it is the caller's responsibility
> - * to remove them when they are no longer needed.
> - * Using @a patched_tempfiles and @a reject_tempfiles in combination with
> - * @a dry_run = TRUE makes it possible to generate a preview of the result
> - * of the patching process, e.g. for display purposes, without actually
> - * modifying the working copy.
> - *
> * If @a ignore_whitespaces is TRUE, allow patches to be applied if they
> * only differ from the target by whitespaces.
> *
> + * If @a remove_tempfiles is TRUE, the temporary patch and reject files will
> + * be removed upon pool cleanup, otherwise, the caller should take ownership
Pool cleanup - which pool?
> + * of these files.
> + *
> + * If @a patch_func is non-NULL, invoke @a patch_func with @a patch_baton
> + * for each patch target processed.
Wasn't the idea that the caller could choose whether the patched result
should be written straight to the target file or to somewhere else? I'm
not sure but I thought specifying @a patched_tempfiles != NULL caused
that to happen. Is that right and if so does the callback support that
feature?
> * If @a ctx->notify_func2 is non-NULL, invoke @a ctx->notify_func2 with
> * @a ctx->notify_baton2 as patching progresses.
> *
> @@ -4907,9 +4914,10 @@ svn_client_patch(const char *abs_patch_p
> svn_boolean_t reverse,
> const apr_array_header_t *include_patterns,
> const apr_array_header_t *exclude_patterns,
> - apr_hash_t **patched_tempfiles,
> - apr_hash_t **reject_tempfiles,
> svn_boolean_t ignore_whitespaces,
> + svn_boolean_t remove_tempfiles,
> + svn_client_patch_func_t patch_func,
> + void *patch_baton,
> svn_client_ctx_t *ctx,
> apr_pool_t *result_pool,
> apr_pool_t *scratch_pool);
- Julian
Received on 2010-05-04 12:37:29 CEST