On Fri, Dec 13, 2024 at 6:02 PM Timofei Zhakov <[email protected]> wrote:
>
> On Fri, Dec 13, 2024 at 4:07 PM Timofei Zhakov <[email protected]> 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
Follow-up: forgot to close the file in the patch-cmd.c.
Attaching v3 of this patch to the email as 'svn-patch-by-file-v3.patch.txt'.
--
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,13 +99,15 @@
}
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));
+ SVN_ERR(svn_io_file_close(patch_file, pool));
+
if (! opt_state->quiet)
SVN_ERR(svn_cl__notifier_print_conflict_stats(ctx->notify_baton2, pool));
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;
}
