On Fri, Dec 13, 2024 at 4:07 PM Timofei Zhakov <t...@chemodax.net> wrote: > > > However, I noticed a little problem in the implementation of patching; The > > svn_client_patch() function accesses the patchfile by an absolute path > > instead of a handle to a file. This follows that the file will be opened > > twice. Is it possible to modify it, so the svn_client_patch() function > > will accept apr_file_t or a svn_stream_t instead of an absolute path to > > this file? > > Hi, > > I made a work-in-progress patch in which svn_client_patch [now > svn_client_patch2] function will read the patch, which has already > been opened at the top of the callstack. > > Attaching the patch to the email as 'svn-patch-by-file-wip-v1.patch.txt'. > > What do you think, should I continue this work? > > PS: this is a patch of svn-patch :-) > > -- > Timofei Zhakov
Hi again, I'm continuing this patch. The second version of the patch has been attached to the email as 'svn-patch-by-file-v2.patch.txt'. Differentiating with 'v1': - Updated the dostrings. - Using newer versions of the functions in the implementation of the `svn patch` commands and unit tests. - Few minor improvements. -- Timofei Zhakov
Index: subversion/include/svn_client.h =================================================================== --- subversion/include/svn_client.h (revision 1922465) +++ subversion/include/svn_client.h (working copy) @@ -7718,8 +7718,9 @@ apr_pool_t *scratch_pool); /** - * Apply a unidiff patch that's located at absolute path - * @a patch_abspath to the working copy directory at @a wc_dir_abspath. + * Apply a unidiff patch, described in @a apr_file which should have + * @c APR_READ and @c APR_BUFFERED capabilities to the working copy + * directory at @a wc_dir_abspath. * * This function makes a best-effort attempt at applying the patch. * It might skip patch targets which cannot be patched (e.g. targets @@ -7756,9 +7757,28 @@ * * Use @a scratch_pool for temporary allocations. * - * @since New in 1.7. + * @since New in 1.15. */ svn_error_t * +svn_client_patch2(apr_file_t *apr_file, + const char *wc_dir_abspath, + svn_boolean_t dry_run, + int strip_count, + svn_boolean_t reverse, + svn_boolean_t ignore_whitespace, + svn_boolean_t remove_tempfiles, + svn_client_patch_func_t patch_func, + void *patch_baton, + svn_client_ctx_t *ctx, + apr_pool_t *scratch_pool); + +/** + * Similar to svn_client_patch2(), but accessing the patch by an absolute + * path. + * + * @deprecated Provided for backward compatibility with the 1.7 API. + */ +svn_error_t * svn_client_patch(const char *patch_abspath, const char *wc_dir_abspath, svn_boolean_t dry_run, Index: subversion/include/svn_diff.h =================================================================== --- subversion/include/svn_diff.h (revision 1922465) +++ subversion/include/svn_diff.h (working copy) @@ -1333,11 +1333,25 @@ * @since New in 1.7. */ typedef struct svn_patch_file_t svn_patch_file_t; -/** Open @a patch_file at @a local_abspath. +/** + * Open @a patch_file at @a local_abspath. * Allocate @a patch_file in @a result_pool. * - * @since New in 1.7. */ + * @since New in 1.15. + */ svn_error_t * +svn_diff_open_patch_file2(svn_patch_file_t **patch_file, + apr_file_t *file, + apr_pool_t *result_pool); + +/** + * Similar to svn_diff_open_patch_file2(), but accessing the patch by an + * absolute path. + * + * @deprecated Provided for backward compatibility with the 1.7 API + */ +SVN_DEPRECATED +svn_error_t * svn_diff_open_patch_file(svn_patch_file_t **patch_file, const char *local_abspath, apr_pool_t *result_pool); @@ -1365,7 +1379,10 @@ * Use @a scratch_pool for all temporary allocations. * * @since New in 1.7. + * @deprecated Provided for backward compatibility with the 1.7 API; Since + * the 1.15 API, the patch-files can be closed using svn_io_file_close(). */ +SVN_DEPRECATED svn_error_t * svn_diff_close_patch_file(svn_patch_file_t *patch_file, apr_pool_t *scratch_pool); Index: subversion/libsvn_client/deprecated.c =================================================================== --- subversion/libsvn_client/deprecated.c (revision 1922465) +++ subversion/libsvn_client/deprecated.c (working copy) @@ -3290,3 +3290,42 @@ return svn_error_trace(svn_client_upgrade2(NULL, path, NULL, ctx, NULL, scratch_pool)); } + +svn_error_t * +svn_client_patch(const char *patch_abspath, + const char *wc_dir_abspath, + svn_boolean_t dry_run, + int strip_count, + svn_boolean_t reverse, + svn_boolean_t ignore_whitespace, + svn_boolean_t remove_tempfiles, + svn_client_patch_func_t patch_func, + void *patch_baton, + svn_client_ctx_t *ctx, + apr_pool_t *scratch_pool) +{ + svn_node_kind_t kind; + apr_file_t *apr_file; + + SVN_ERR(svn_io_check_path(patch_abspath, &kind, scratch_pool)); + if (kind == svn_node_none) + return svn_error_createf(SVN_ERR_ILLEGAL_TARGET, NULL, + _("'%s' does not exist"), + svn_dirent_local_style(patch_abspath, + scratch_pool)); + if (kind != svn_node_file) + return svn_error_createf(SVN_ERR_ILLEGAL_TARGET, NULL, + _("'%s' is not a file"), + svn_dirent_local_style(patch_abspath, + scratch_pool)); + + SVN_ERR(svn_io_file_open(&apr_file, patch_abspath, + APR_READ | APR_BUFFERED, APR_OS_DEFAULT, + scratch_pool)); + + return svn_error_trace(svn_client_patch2(apr_file, wc_dir_abspath, + dry_run, strip_count, reverse, + ignore_whitespace, remove_tempfiles, + patch_func, patch_baton, + ctx, scratch_pool)); +} Index: subversion/libsvn_client/patch.c =================================================================== --- subversion/libsvn_client/patch.c (revision 1922465) +++ subversion/libsvn_client/patch.c (working copy) @@ -3613,8 +3613,8 @@ /* This function is the main entry point into the patch code. */ static svn_error_t * -apply_patches(/* The path to the patch file. */ - const char *patch_abspath, +apply_patches(/* The patch file. */ + apr_file_t *apr_file, /* The abspath to the working copy the patch should be applied to. */ const char *root_abspath, /* Indicates whether we're doing a dry run. */ @@ -3640,7 +3640,7 @@ apr_array_header_t *targets_info; /* Try to open the patch file. */ - SVN_ERR(svn_diff_open_patch_file(&patch_file, patch_abspath, scratch_pool)); + SVN_ERR(svn_diff_open_patch_file2(&patch_file, apr_file, scratch_pool)); /* Apply patches. */ targets_info = apr_array_make(scratch_pool, 0, @@ -3720,7 +3720,6 @@ } while (patch); - SVN_ERR(svn_diff_close_patch_file(patch_file, iterpool)); svn_pool_destroy(iterpool); return SVN_NO_ERROR; @@ -3727,17 +3726,17 @@ } svn_error_t * -svn_client_patch(const char *patch_abspath, - const char *wc_dir_abspath, - svn_boolean_t dry_run, - int strip_count, - svn_boolean_t reverse, - svn_boolean_t ignore_whitespace, - svn_boolean_t remove_tempfiles, - svn_client_patch_func_t patch_func, - void *patch_baton, - svn_client_ctx_t *ctx, - apr_pool_t *scratch_pool) +svn_client_patch2(apr_file_t *apr_file, + const char *wc_dir_abspath, + svn_boolean_t dry_run, + int strip_count, + svn_boolean_t reverse, + svn_boolean_t ignore_whitespace, + svn_boolean_t remove_tempfiles, + svn_client_patch_func_t patch_func, + void *patch_baton, + svn_client_ctx_t *ctx, + apr_pool_t *scratch_pool) { svn_node_kind_t kind; @@ -3751,18 +3750,6 @@ svn_dirent_local_style(wc_dir_abspath, scratch_pool)); - SVN_ERR(svn_io_check_path(patch_abspath, &kind, scratch_pool)); - if (kind == svn_node_none) - return svn_error_createf(SVN_ERR_ILLEGAL_TARGET, NULL, - _("'%s' does not exist"), - svn_dirent_local_style(patch_abspath, - scratch_pool)); - if (kind != svn_node_file) - return svn_error_createf(SVN_ERR_ILLEGAL_TARGET, NULL, - _("'%s' is not a file"), - svn_dirent_local_style(patch_abspath, - scratch_pool)); - SVN_ERR(svn_io_check_path(wc_dir_abspath, &kind, scratch_pool)); if (kind == svn_node_none) return svn_error_createf(SVN_ERR_ILLEGAL_TARGET, NULL, @@ -3776,7 +3763,7 @@ scratch_pool)); SVN_WC__CALL_WITH_WRITE_LOCK( - apply_patches(patch_abspath, wc_dir_abspath, dry_run, strip_count, + apply_patches(apr_file, wc_dir_abspath, dry_run, strip_count, reverse, ignore_whitespace, remove_tempfiles, patch_func, patch_baton, ctx, scratch_pool), ctx->wc_ctx, wc_dir_abspath, FALSE /* lock_anchor */, scratch_pool); Index: subversion/libsvn_diff/deprecated.c =================================================================== --- subversion/libsvn_diff/deprecated.c (revision 1922465) +++ subversion/libsvn_diff/deprecated.c (working copy) @@ -462,3 +462,18 @@ NULL, NULL, pool)); } + +svn_error_t * +svn_diff_open_patch_file(svn_patch_file_t **patch_file, + const char *local_abspath, + apr_pool_t *result_pool) +{ + apr_file_t *file; + + SVN_ERR(svn_io_file_open(&file, local_abspath, + APR_READ | APR_BUFFERED, APR_OS_DEFAULT, + result_pool)); + + return svn_error_trace(svn_diff_open_patch_file2(patch_file, file, + result_pool)); +} Index: subversion/libsvn_diff/parse-diff.c =================================================================== --- subversion/libsvn_diff/parse-diff.c (revision 1922465) +++ subversion/libsvn_diff/parse-diff.c (working copy) @@ -1957,16 +1957,14 @@ }; svn_error_t * -svn_diff_open_patch_file(svn_patch_file_t **patch_file, - const char *local_abspath, - apr_pool_t *result_pool) +svn_diff_open_patch_file2(svn_patch_file_t **patch_file, + apr_file_t *file, + apr_pool_t *result_pool) { svn_patch_file_t *p; p = apr_palloc(result_pool, sizeof(*p)); - SVN_ERR(svn_io_file_open(&p->apr_file, local_abspath, - APR_READ | APR_BUFFERED, APR_OS_DEFAULT, - result_pool)); + p->apr_file = file; p->next_patch_offset = 0; *patch_file = p; Index: subversion/svn/patch-cmd.c =================================================================== --- subversion/svn/patch-cmd.c (revision 1922465) +++ subversion/svn/patch-cmd.c (working copy) @@ -51,8 +51,10 @@ apr_array_header_t *targets; const char *abs_patch_path; const char *patch_path; + apr_file_t *patch_file; const char *abs_target_path; const char *target_path; + svn_node_kind_t kind; opt_state = ((svn_cl__cmd_baton_t *)baton)->opt_state; ctx = ((svn_cl__cmd_baton_t *)baton)->ctx; @@ -74,6 +76,19 @@ SVN_ERR(svn_dirent_get_absolute(&abs_patch_path, patch_path, pool)); + SVN_ERR(svn_io_check_path(abs_patch_path, &kind, pool)); + if (kind == svn_node_none) + return svn_error_createf(SVN_ERR_ILLEGAL_TARGET, NULL, + _("'%s' does not exist"), + svn_dirent_local_style(abs_patch_path, pool)); + if (kind != svn_node_file) + return svn_error_createf(SVN_ERR_ILLEGAL_TARGET, NULL, + _("'%s' is not a file"), + svn_dirent_local_style(abs_patch_path, pool)); + + SVN_ERR(svn_io_file_open(&patch_file, abs_patch_path, + APR_READ | APR_BUFFERED, APR_OS_DEFAULT, pool)); + if (targets->nelts == 1) target_path = ""; /* "" is the canonical form of "." */ else @@ -84,11 +99,11 @@ } SVN_ERR(svn_dirent_get_absolute(&abs_target_path, target_path, pool)); - SVN_ERR(svn_client_patch(abs_patch_path, abs_target_path, - opt_state->dry_run, opt_state->strip, - opt_state->reverse_diff, - opt_state->ignore_whitespace, - TRUE, NULL, NULL, ctx, pool)); + SVN_ERR(svn_client_patch2(patch_file, abs_target_path, + opt_state->dry_run, opt_state->strip, + opt_state->reverse_diff, + opt_state->ignore_whitespace, + TRUE, NULL, NULL, ctx, pool)); if (! opt_state->quiet) Index: subversion/tests/libsvn_client/client-test.c =================================================================== --- subversion/tests/libsvn_client/client-test.c (revision 1922465) +++ subversion/tests/libsvn_client/client-test.c (working copy) @@ -415,9 +415,9 @@ pcb.patched_tempfiles = apr_hash_make(pool); pcb.reject_tempfiles = apr_hash_make(pool); pcb.state_pool = pool; - SVN_ERR(svn_client_patch(patch_file_path, wc_path, FALSE, 0, FALSE, - FALSE, FALSE, patch_collection_func, &pcb, - ctx, pool)); + SVN_ERR(svn_client_patch2(patch_file, wc_path, FALSE, 0, FALSE, + FALSE, FALSE, patch_collection_func, &pcb, + ctx, pool)); SVN_ERR(svn_io_file_close(patch_file, pool)); SVN_TEST_ASSERT(apr_hash_count(pcb.patched_tempfiles) == 1); Index: subversion/tests/libsvn_diff/parse-diff-test.c =================================================================== --- subversion/tests/libsvn_diff/parse-diff-test.c (revision 1922465) +++ subversion/tests/libsvn_diff/parse-diff-test.c (working copy) @@ -303,8 +303,7 @@ if (len != bytes) return svn_error_createf(SVN_ERR_TEST_FAILED, NULL, "Cannot write to '%s'", path); - SVN_ERR(svn_io_file_close(apr_file, pool)); - SVN_ERR(svn_diff_open_patch_file(patch_file, path, pool)); + SVN_ERR(svn_diff_open_patch_file2(patch_file, apr_file, pool)); return SVN_NO_ERROR; }