You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@subversion.apache.org by st...@apache.org on 2010/03/25 23:44:07 UTC

svn commit: r927620 - in /subversion/trunk/subversion: include/svn_client.h libsvn_client/patch.c svn/patch-cmd.c tests/libsvn_client/ tests/libsvn_client/client-test.c

Author: stsp
Date: Thu Mar 25 22:44:06 2010
New Revision: 927620

URL: http://svn.apache.org/viewvc?rev=927620&view=rev
Log:
Fix issue #3598, "Allow access to patched temporary files via svn patch API"

* subversion/include/svn_client.h
  (svn_client_patch): Add patched_tempfiles and reject_tempfiles output
   parameters.  Switch this function to dual-pool style since it now
   has output parameters.

* subversion/libsvn_client/patch.c
  (init_patch_target, apply_one_patch, svn_client_patch): Add
   patched_tempfiles and reject_tempfiles parameters. If provided,
   put paths to temporary files into them.
  (apply_patches_baton_t): Add patched_tempfiles and reject_tempfiles
   member fields.

* subversion/svn/patch-cmd.c
 (svn_cl__patch): Track parameter changes to svn_client_patch().

* subversion/tests/libsvn_client: Ignore test-patch*

* subversion/tests/libsvn_client/client-test.c
  (): Include some headers.
  (check_patch_result): Helper for new test.
  (test_patch): New test which checks whether tempfiles are returned
   properly, are not deleted, and have expected content. We cannot test
   this feature via cmdline tests because the CLI client does not expose it.
  (test_funcs): Add new test.

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/   (props changed)
    subversion/trunk/subversion/tests/libsvn_client/client-test.c

Modified: subversion/trunk/subversion/include/svn_client.h
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/include/svn_client.h?rev=927620&r1=927619&r2=927620&view=diff
==============================================================================
--- subversion/trunk/subversion/include/svn_client.h (original)
+++ subversion/trunk/subversion/include/svn_client.h Thu Mar 25 22:44:06 2010
@@ -4861,12 +4861,33 @@ 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
+ * @ignore_patterns or @a exlude_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 ctx->notify_func2 is non-NULL, invoke @a ctx->notify_func2 with
  * @a ctx->notify_baton2 as patching progresses.
  *
  * If @a ctx->cancel_func is non-NULL, invoke it passing @a
  * ctx->cancel_baton at various places during the operation.
  *
+ * Use @a scratch_pool for temporary allocations.
+ *
  * @since New in 1.7.
  */
 svn_error_t *
@@ -4877,8 +4898,11 @@ 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_client_ctx_t *ctx,
-                 apr_pool_t *pool);
+                 apr_pool_t *result_pool,
+                 apr_pool_t *scratch_pool);
 
 /** @} */
 

Modified: subversion/trunk/subversion/libsvn_client/patch.c
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_client/patch.c?rev=927620&r1=927619&r2=927620&view=diff
==============================================================================
--- subversion/trunk/subversion/libsvn_client/patch.c (original)
+++ subversion/trunk/subversion/libsvn_client/patch.c Thu Mar 25 22:44:06 2010
@@ -327,6 +327,9 @@ resolve_target_path(patch_target_t *targ
  * Else, set *target to NULL.
  * If a target does not match a glob in INCLUDE_PATTERNS, mark it as filtered.
  * If a target matches a glob in EXCLUDE_PATTERNS, mark it as filtered.
+ * If PATCHED_TEMPFILES or REJECT_TEMPFILES are not NULL, add the path
+ * to temporary patched/reject files to them, keyed by the target's path
+ * as parsed from the patch file (after canonicalization).
  * Use SCRATCH_POOL for all other allocations. */
 static svn_error_t *
 init_patch_target(patch_target_t **patch_target,
@@ -335,6 +338,8 @@ init_patch_target(patch_target_t **patch
                   svn_wc_context_t *wc_ctx, int strip_count,
                   const apr_array_header_t *include_patterns,
                   const apr_array_header_t *exclude_patterns,
+                  apr_hash_t *patched_tempfiles,
+                  apr_hash_t *reject_tempfiles,
                   apr_pool_t *result_pool, apr_pool_t *scratch_pool)
 {
   patch_target_t *target;
@@ -466,8 +471,14 @@ init_patch_target(patch_target_t **patch
       /* Create a temporary file to write the patched result to. */
       SVN_ERR(svn_stream_open_unique(&target->patched_raw,
                                      &target->patched_path, NULL,
-                                     svn_io_file_del_on_pool_cleanup,
+                                     patched_tempfiles ?
+                                       svn_io_file_del_none :
+                                       svn_io_file_del_on_pool_cleanup,
                                      result_pool, scratch_pool));
+      if (patched_tempfiles)
+        apr_hash_set(patched_tempfiles, target->canon_path_from_patchfile,
+                     APR_HASH_KEY_STRING, target->patched_path);
+
       /* Expand keywords in the patched file.
        * Repair newlines if svn:eol-style dictates a particular style. */
       repair_eol = (target->eol_style == svn_subst_eol_style_fixed ||
@@ -481,8 +492,13 @@ init_patch_target(patch_target_t **patch
        * in reject files. */
       SVN_ERR(svn_stream_open_unique(&target->reject,
                                      &target->reject_path, NULL,
-                                     svn_io_file_del_on_pool_cleanup,
+                                     reject_tempfiles ?
+                                       svn_io_file_del_none :
+                                       svn_io_file_del_on_pool_cleanup,
                                      result_pool, scratch_pool));
+      if (reject_tempfiles)
+        apr_hash_set(reject_tempfiles, target->canon_path_from_patchfile,
+                     APR_HASH_KEY_STRING, target->reject_path);
 
       /* The reject stream needs a diff header. */
       diff_header = apr_psprintf(scratch_pool, "--- %s%s+++ %s%s",
@@ -1082,6 +1098,9 @@ send_patch_notification(const patch_targ
  * which should be stripped from target paths in the patch.
  * If a target does not match a glob in INCLUDE_PATTERNS, mark it as filtered.
  * If a target matches a glob in EXCLUDE_PATTERNS, mark it as filtered.
+ * If PATCHED_TEMPFILES or REJECT_TEMPFILES are not NULL, add the path
+ * to temporary patched/reject files to them, keyed by the target's path
+ * as parsed from the patch file (after canonicalization).
  * Do temporary allocations in SCRATCH_POOL. */
 static svn_error_t *
 apply_one_patch(patch_target_t **patch_target, svn_patch_t *patch,
@@ -1089,6 +1108,8 @@ apply_one_patch(patch_target_t **patch_t
                 int strip_count,
                 const apr_array_header_t *include_patterns,
                 const apr_array_header_t *exclude_patterns,
+                apr_hash_t *patched_tempfiles,
+                apr_hash_t *reject_tempfiles,
                 apr_pool_t *result_pool, apr_pool_t *scratch_pool)
 {
   patch_target_t *target;
@@ -1098,6 +1119,7 @@ apply_one_patch(patch_target_t **patch_t
 
   SVN_ERR(init_patch_target(&target, patch, abs_wc_path, wc_ctx, strip_count,
                             include_patterns, exclude_patterns,
+                            patched_tempfiles, reject_tempfiles,
                             result_pool, scratch_pool));
 
   if (target->skipped || target->filtered)
@@ -1413,6 +1435,13 @@ typedef struct {
   /* Files matching any of these patterns won't be patched. */
   const apr_array_header_t *exclude_patterns;
 
+  /* Mapping patch target path -> path to tempfile with patched result. */
+  apr_hash_t *patched_tempfiles;
+
+  /* Mapping patch target path -> path to tempfile with rejected hunks. */
+  apr_hash_t *reject_tempfiles;
+
+
   /* The client context. */
   svn_client_ctx_t *ctx;
 } apply_patches_baton_t;
@@ -1466,7 +1495,8 @@ apply_patches(void *baton,
           SVN_ERR(apply_one_patch(&target, patch, btn->abs_wc_path,
                                   btn->ctx->wc_ctx, btn->strip_count,
                                   btn->include_patterns, btn->exclude_patterns,
-                                  scratch_pool, iterpool));
+                                  btn->patched_tempfiles, btn->reject_tempfiles,
+                                  result_pool, iterpool));
           if (target->filtered)
             SVN_ERR(svn_diff_close_patch(patch));
           else
@@ -1508,8 +1538,11 @@ 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_client_ctx_t *ctx,
-                 apr_pool_t *pool)
+                 apr_pool_t *result_pool,
+                 apr_pool_t *scratch_pool)
 {
   apply_patches_baton_t baton;
 
@@ -1525,10 +1558,24 @@ svn_client_patch(const char *abs_patch_p
   baton.reverse = reverse;
   baton.include_patterns = include_patterns;
   baton.exclude_patterns = exclude_patterns;
+  if (patched_tempfiles)
+    {
+      (*patched_tempfiles) = apr_hash_make(result_pool);
+      baton.patched_tempfiles = (*patched_tempfiles);
+    }
+  else
+    baton.patched_tempfiles = NULL;
+  if (reject_tempfiles)
+    {
+      (*reject_tempfiles) = apr_hash_make(result_pool);
+      baton.reject_tempfiles = (*reject_tempfiles);
+    }
+  else
+    baton.reject_tempfiles = NULL;
 
   SVN_ERR(svn_wc__call_with_write_lock(apply_patches, &baton,
                                        ctx->wc_ctx, local_abspath,
-                                       pool, pool));
+                                       result_pool, scratch_pool));
 
   return SVN_NO_ERROR;
 }

Modified: subversion/trunk/subversion/svn/patch-cmd.c
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/svn/patch-cmd.c?rev=927620&r1=927619&r2=927620&view=diff
==============================================================================
--- subversion/trunk/subversion/svn/patch-cmd.c (original)
+++ subversion/trunk/subversion/svn/patch-cmd.c Thu Mar 25 22:44:06 2010
@@ -80,7 +80,8 @@ svn_cl__patch(apr_getopt_t *os,
                            opt_state->dry_run, opt_state->strip_count,
                            opt_state->reverse_diff,
                            opt_state->include_patterns,
-                           opt_state->exclude_patterns, ctx, pool));
+                           opt_state->exclude_patterns,
+                           NULL, NULL, ctx, pool, pool));
 
   if (! opt_state->quiet)
     SVN_ERR(svn_cl__print_conflict_stats(ctx->notify_baton2, pool));

Propchange: subversion/trunk/subversion/tests/libsvn_client/
------------------------------------------------------------------------------
--- svn:ignore (original)
+++ svn:ignore Thu Mar 25 22:44:06 2010
@@ -1,3 +1,4 @@
 .libs
 client-test
 *.lo
+test-patch*

Modified: subversion/trunk/subversion/tests/libsvn_client/client-test.c
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/tests/libsvn_client/client-test.c?rev=927620&r1=927619&r2=927620&view=diff
==============================================================================
--- subversion/trunk/subversion/tests/libsvn_client/client-test.c (original)
+++ subversion/trunk/subversion/tests/libsvn_client/client-test.c Thu Mar 25 22:44:06 2010
@@ -22,12 +22,17 @@
  */
 
 
+#include <unistd.h>
+#include <limits.h>
 #include "svn_mergeinfo.h"
 #include "../../libsvn_client/mergeinfo.h"
 #include "svn_pools.h"
 #include "svn_client.h"
+#include "svn_repos.h"
+#include "svn_subst.h"
 
 #include "../svn_test.h"
+#include "../svn_test_fs.h"
 
 typedef struct {
   const char *path;
@@ -199,6 +204,163 @@ test_args_to_target_array(apr_pool_t *po
   return SVN_NO_ERROR;
 }
 
+
+/* A helper function for test_patch().
+ * It compares a patched or reject file against expected content.
+ * It also deletes the file if the check was successful. */
+static svn_error_t *
+check_patch_result(const char *path, const char **expected_lines,
+                   int num_expected_lines, apr_pool_t *pool)
+{
+  svn_string_t *path_svnstr;
+  svn_string_t *path_utf8;
+  apr_file_t *patched_file;
+  svn_stream_t *stream;
+  apr_pool_t *iterpool;
+  int i;
+
+  path_svnstr = svn_string_create(path, pool);
+  SVN_ERR(svn_subst_translate_string(&path_utf8, path_svnstr, NULL, pool));
+  SVN_ERR(svn_io_file_open(&patched_file, path_utf8->data,
+                           APR_READ, APR_OS_DEFAULT, pool));
+  stream = svn_stream_from_aprfile2(patched_file, TRUE, pool);
+  i = 0;
+  iterpool = svn_pool_create(pool);
+  while (TRUE)
+    {
+      svn_boolean_t eof;
+      svn_stringbuf_t *line;
+
+      svn_pool_clear(iterpool);
+
+      SVN_ERR(svn_stream_readline(stream, &line, APR_EOL_STR, &eof, pool));
+      if (i < num_expected_lines)
+        SVN_ERR_ASSERT(strcmp(expected_lines[i++], line->data) == 0);
+      if (eof)
+        break;
+    }
+  svn_pool_destroy(iterpool);
+
+  SVN_ERR(svn_io_file_close(patched_file, pool));
+  SVN_ERR_ASSERT(i == num_expected_lines);
+  SVN_ERR(svn_io_remove_file2(path_utf8->data, FALSE, pool));
+
+  return SVN_NO_ERROR;
+}
+
+static svn_error_t *
+test_patch(const svn_test_opts_t *opts,
+           apr_pool_t *pool)
+{
+  svn_repos_t *repos;
+  svn_fs_t *fs;
+  svn_fs_txn_t *txn;
+  svn_fs_root_t *txn_root;
+  apr_hash_t *patched_tempfiles;
+  apr_hash_t *reject_tempfiles;
+  const char *repos_url;
+  const char *wc_path;
+  char cwd[PATH_MAX];
+  svn_string_t *cwd_svnstr;
+  svn_string_t *cwd_utf8;
+  svn_revnum_t committed_rev;
+  svn_opt_revision_t rev;
+  svn_opt_revision_t peg_rev;
+  svn_client_ctx_t *ctx;
+  apr_file_t *patch_file;
+  const char *patch_file_path;
+  const char *patched_tempfile_path;
+  const char *reject_tempfile_path;
+  const char *key;
+  int i;
+#define NL APR_EOL_STR
+#define UNIDIFF_LINES 7
+  const char *unidiff_patch[UNIDIFF_LINES] =  {
+    "Index: A/D/gamma" NL,
+    "===================================================================\n",
+    "--- A/D/gamma\t(revision 1)" NL,
+    "+++ A/D/gamma\t(working copy)" NL,
+    "@@ -1 +1 @@" NL,
+    "-This is really the file 'gamma'." NL,
+    "+It is really the file 'gamma'." NL
+  };
+#define EXPECTED_GAMMA_LINES 1
+  const char *expected_gamma[EXPECTED_GAMMA_LINES] = {
+    "This is the file 'gamma'."
+  };
+#define EXPECTED_GAMMA_REJECT_LINES 5
+  const char *expected_gamma_reject[EXPECTED_GAMMA_REJECT_LINES] = {
+    "--- A/D/gamma",
+    "+++ A/D/gamma",
+    "@@ -1,1 +1,1 @@",
+    "-This is really the file 'gamma'.",
+    "+It is really the file 'gamma'."
+  };
+
+  /* Create a filesytem and repository. */
+  SVN_ERR(svn_test__create_repos(&repos, "test-patch-repos",
+                                 opts, pool));
+  fs = svn_repos_fs(repos);
+
+  /* Prepare a txn to receive the greek tree. */
+  SVN_ERR(svn_fs_begin_txn2(&txn, fs, 0, 0, pool));
+  SVN_ERR(svn_fs_txn_root(&txn_root, txn, pool));
+  SVN_ERR(svn_test__create_greek_tree(txn_root, pool));
+  SVN_ERR(svn_repos_fs_commit_txn(NULL, repos, &committed_rev, txn, pool));
+
+  /* Check out the HEAD revision */
+  getcwd(cwd, sizeof(cwd));
+  cwd_svnstr = svn_string_create(cwd, pool);
+  SVN_ERR(svn_subst_translate_string(&cwd_utf8, cwd_svnstr, NULL, pool));
+  repos_url = apr_pstrcat(pool, "file://", cwd_utf8->data,
+                          "/test-patch-repos", NULL);
+  repos_url = svn_uri_canonicalize(repos_url, pool);
+  wc_path = svn_dirent_join(cwd_utf8->data, "test-patch-wc", pool);
+  wc_path = svn_dirent_canonicalize(wc_path, pool);
+  SVN_ERR(svn_io_remove_dir2(wc_path, FALSE, NULL, NULL, pool));
+  rev.kind = svn_opt_revision_head;
+  peg_rev.kind = svn_opt_revision_unspecified;
+  SVN_ERR(svn_client_create_context(&ctx, pool));
+  SVN_ERR(svn_client_checkout3(NULL, repos_url, wc_path,
+                               &peg_rev, &rev, svn_depth_infinity,
+                               TRUE, FALSE, ctx, pool));
+
+  /* Create the patch file. */
+  patch_file_path = svn_dirent_join(cwd_utf8->data, "test-patch.diff", pool);
+  patch_file_path = svn_dirent_canonicalize(patch_file_path, pool);
+  SVN_ERR(svn_io_file_open(&patch_file, patch_file_path,
+                           (APR_READ | APR_WRITE | APR_CREATE | APR_TRUNCATE),
+                           APR_OS_DEFAULT, pool));
+  for (i = 0; i < UNIDIFF_LINES; i++)
+    {
+      apr_size_t len = strlen(unidiff_patch[i]);
+      SVN_ERR(svn_io_file_write(patch_file, unidiff_patch[i], &len, pool));
+      SVN_ERR_ASSERT(len == strlen(unidiff_patch[i]));
+    }
+  SVN_ERR(svn_io_file_flush_to_disk(patch_file, pool));
+
+  /* Apply the patch. */
+  SVN_ERR(svn_client_patch(patch_file_path, wc_path, FALSE, 0, FALSE,
+                           NULL, NULL, &patched_tempfiles, &reject_tempfiles,
+                           ctx, pool, pool));
+  SVN_ERR(svn_io_file_close(patch_file, pool));
+
+  SVN_ERR_ASSERT(apr_hash_count(patched_tempfiles) == 1);
+  key = "A/D/gamma";
+  patched_tempfile_path = apr_hash_get(patched_tempfiles, key,
+                                       APR_HASH_KEY_STRING);
+  SVN_ERR(check_patch_result(patched_tempfile_path, expected_gamma,
+                             EXPECTED_GAMMA_LINES, pool));
+  SVN_ERR_ASSERT(apr_hash_count(reject_tempfiles) == 1);
+  key = "A/D/gamma";
+  reject_tempfile_path = apr_hash_get(reject_tempfiles, key,
+                                     APR_HASH_KEY_STRING);
+  SVN_ERR(check_patch_result(reject_tempfile_path, expected_gamma_reject,
+                             EXPECTED_GAMMA_REJECT_LINES, pool));
+
+  return SVN_NO_ERROR;
+}
+
 /* ========================================================================== */
 
 struct svn_test_descriptor_t test_funcs[] =
@@ -208,5 +370,6 @@ struct svn_test_descriptor_t test_funcs[
                    "test svn_client__elide_mergeinfo_catalog"),
     SVN_TEST_PASS2(test_args_to_target_array,
                    "test svn_client_args_to_target_array"),
+    SVN_TEST_OPTS_PASS(test_patch, "test svn_client_patch"),
     SVN_TEST_NULL
   };



Re: svn commit: r927620 - in /subversion/trunk/subversion: include/svn_client.h libsvn_client/patch.c svn/patch-cmd.c tests/libsvn_client/ tests/libsvn_client/client-test.c

Posted by Philip Martin <ph...@wandisco.com>.
"Bert Huijben" <be...@qqmail.nl> writes:

>> +  repos_url = apr_pstrcat(pool, "file://", cwd_utf8->data,
>> +                          "/test-patch-repos", NULL);
>
> This won't work for cwd on Windows. That would give "file://C:/dir",
> while it should be "file:///C:/dir" (Fixed in r927764)

What about using svn_test__current_directory_url?

-- 
Philip

Re: svn commit: r927620 - in /subversion/trunk/subversion: include/svn_client.h libsvn_client/patch.c svn/patch-cmd.c tests/libsvn_client/ tests/libsvn_client/client-test.c

Posted by Stefan Sperling <st...@elego.de>.
Hey Bert, Philip,

Thanks for fixing these.
I knew the test was rough and I was meaning to revisit it,
but you found and fixed more problems than I was aware of :)
I'll check back on the test on monday the latest.

Stefan

RE: svn commit: r927620 - in /subversion/trunk/subversion: include/svn_client.h libsvn_client/patch.c svn/patch-cmd.c tests/libsvn_client/ tests/libsvn_client/client-test.c

Posted by Bert Huijben <be...@qqmail.nl>.

> -----Original Message-----
> From: stsp@apache.org [mailto:stsp@apache.org]
> Sent: donderdag 25 maart 2010 23:44
> To: commits@subversion.apache.org
> Subject: svn commit: r927620 - in /subversion/trunk/subversion:
> include/svn_client.h libsvn_client/patch.c svn/patch-cmd.c
> tests/libsvn_client/ tests/libsvn_client/client-test.c
> 
> Author: stsp
> Date: Thu Mar 25 22:44:06 2010
> New Revision: 927620
> 
> URL: http://svn.apache.org/viewvc?rev=927620&view=rev
> Log:
> Fix issue #3598, "Allow access to patched temporary files via svn patch API"
> 
> * subversion/include/svn_client.h
>   (svn_client_patch): Add patched_tempfiles and reject_tempfiles output
>    parameters.  Switch this function to dual-pool style since it now
>    has output parameters.
> 
> * subversion/libsvn_client/patch.c
>   (init_patch_target, apply_one_patch, svn_client_patch): Add
>    patched_tempfiles and reject_tempfiles parameters. If provided,
>    put paths to temporary files into them.
>   (apply_patches_baton_t): Add patched_tempfiles and reject_tempfiles
>    member fields.
> 
> * subversion/svn/patch-cmd.c
>  (svn_cl__patch): Track parameter changes to svn_client_patch().
> 
> * subversion/tests/libsvn_client: Ignore test-patch*
> 
> * subversion/tests/libsvn_client/client-test.c
>   (): Include some headers.
>   (check_patch_result): Helper for new test.
>   (test_patch): New test which checks whether tempfiles are returned
>    properly, are not deleted, and have expected content. We cannot test
>    this feature via cmdline tests because the CLI client does not expose it.
>   (test_funcs): Add new test.
> 
> 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/   (props changed)
>     subversion/trunk/subversion/tests/libsvn_client/client-test.c
> 

<snip>

> Modified: subversion/trunk/subversion/tests/libsvn_client/client-test.c
> URL:
> http://svn.apache.org/viewvc/subversion/trunk/subversion/tests/libsvn_cli
> ent/client-test.c?rev=927620&r1=927619&r2=927620&view=diff
> ==========================================================
> ====================
> --- subversion/trunk/subversion/tests/libsvn_client/client-test.c (original)
> +++ subversion/trunk/subversion/tests/libsvn_client/client-test.c Thu Mar
> 25 22:44:06 2010
> @@ -22,12 +22,17 @@
>   */
> 
>  

> +#include <unistd.h>

This is a unix only include file. 
Please use some appropriate check (E.g. APR_HAVE_UNISTD_H) or just use an apr equivalent.
(Fixed in r 927764)

> +#include <limits.h>
>  #include "svn_mergeinfo.h"
>  #include "../../libsvn_client/mergeinfo.h"
>  #include "svn_pools.h"
>  #include "svn_client.h"
> +#include "svn_repos.h"
> +#include "svn_subst.h"
> 
>  #include "../svn_test.h"
> +#include "../svn_test_fs.h"
> 
>  typedef struct {
>    const char *path;
> @@ -199,6 +204,163 @@ test_args_to_target_array(apr_pool_t *po
>    return SVN_NO_ERROR;
>  }
> 
> +
> +/* A helper function for test_patch().
> + * It compares a patched or reject file against expected content.
> + * It also deletes the file if the check was successful. */
> +static svn_error_t *
> +check_patch_result(const char *path, const char **expected_lines,
> +                   int num_expected_lines, apr_pool_t *pool)
> +{
> +  svn_string_t *path_svnstr;
> +  svn_string_t *path_utf8;
> +  apr_file_t *patched_file;
> +  svn_stream_t *stream;
> +  apr_pool_t *iterpool;
> +  int i;
> +
> +  path_svnstr = svn_string_create(path, pool);
> +  SVN_ERR(svn_subst_translate_string(&path_utf8, path_svnstr, NULL,
> pool));
> +  SVN_ERR(svn_io_file_open(&patched_file, path_utf8->data,
> +                           APR_READ, APR_OS_DEFAULT, pool));
> +  stream = svn_stream_from_aprfile2(patched_file, TRUE, pool);
> +  i = 0;
> +  iterpool = svn_pool_create(pool);
> +  while (TRUE)
> +    {
> +      svn_boolean_t eof;
> +      svn_stringbuf_t *line;
> +
> +      svn_pool_clear(iterpool);
> +
> +      SVN_ERR(svn_stream_readline(stream, &line, APR_EOL_STR, &eof,
> pool));
> +      if (i < num_expected_lines)
> +        SVN_ERR_ASSERT(strcmp(expected_lines[i++], line->data) == 0);
> +      if (eof)
> +        break;
> +    }
> +  svn_pool_destroy(iterpool);
> +
> +  SVN_ERR(svn_io_file_close(patched_file, pool));
> +  SVN_ERR_ASSERT(i == num_expected_lines);
> +  SVN_ERR(svn_io_remove_file2(path_utf8->data, FALSE, pool));
> +
> +  return SVN_NO_ERROR;
> +}
> +
> +static svn_error_t *
> +test_patch(const svn_test_opts_t *opts,
> +           apr_pool_t *pool)
> +{
> +  svn_repos_t *repos;
> +  svn_fs_t *fs;
> +  svn_fs_txn_t *txn;
> +  svn_fs_root_t *txn_root;
> +  apr_hash_t *patched_tempfiles;
> +  apr_hash_t *reject_tempfiles;
> +  const char *repos_url;
> +  const char *wc_path;
> +  char cwd[PATH_MAX];

You don't want to use PATH_MAX on Windows. It is about 256 there, but APR has no problems handling paths that are much longer. APR_PATH_MAX is more appropriate, but we don't need this.

> +  svn_string_t *cwd_svnstr;
> +  svn_string_t *cwd_utf8;
> +  svn_revnum_t committed_rev;
> +  svn_opt_revision_t rev;
> +  svn_opt_revision_t peg_rev;
> +  svn_client_ctx_t *ctx;
> +  apr_file_t *patch_file;
> +  const char *patch_file_path;
> +  const char *patched_tempfile_path;
> +  const char *reject_tempfile_path;
> +  const char *key;
> +  int i;
> +#define NL APR_EOL_STR
> +#define UNIDIFF_LINES 7
> +  const char *unidiff_patch[UNIDIFF_LINES] =  {
> +    "Index: A/D/gamma" NL,
> +
> "==========================================================
> =========\n",
> +    "--- A/D/gamma\t(revision 1)" NL,
> +    "+++ A/D/gamma\t(working copy)" NL,
> +    "@@ -1 +1 @@" NL,
> +    "-This is really the file 'gamma'." NL,
> +    "+It is really the file 'gamma'." NL
> +  };
> +#define EXPECTED_GAMMA_LINES 1
> +  const char *expected_gamma[EXPECTED_GAMMA_LINES] = {
> +    "This is the file 'gamma'."
> +  };
> +#define EXPECTED_GAMMA_REJECT_LINES 5
> +  const char *expected_gamma_reject[EXPECTED_GAMMA_REJECT_LINES]
> = {
> +    "--- A/D/gamma",
> +    "+++ A/D/gamma",
> +    "@@ -1,1 +1,1 @@",
> +    "-This is really the file 'gamma'.",
> +    "+It is really the file 'gamma'."
> +  };
> +
> +  /* Create a filesytem and repository. */
> +  SVN_ERR(svn_test__create_repos(&repos, "test-patch-repos",
> +                                 opts, pool));
> +  fs = svn_repos_fs(repos);
> +
> +  /* Prepare a txn to receive the greek tree. */
> +  SVN_ERR(svn_fs_begin_txn2(&txn, fs, 0, 0, pool));
> +  SVN_ERR(svn_fs_txn_root(&txn_root, txn, pool));
> +  SVN_ERR(svn_test__create_greek_tree(txn_root, pool));
> +  SVN_ERR(svn_repos_fs_commit_txn(NULL, repos, &committed_rev, txn,
> pool));
> +
> +  /* Check out the HEAD revision */
> +  getcwd(cwd, sizeof(cwd));

You can use svn_dirent_get_absolute(&cwd, "", ...) to do this and the conversion for you. ("" is our canonical form of ".")
(Fixed in r927764)

> +  cwd_svnstr = svn_string_create(cwd, pool);
> +  SVN_ERR(svn_subst_translate_string(&cwd_utf8, cwd_svnstr, NULL,
> pool));

This is the wrong translation function. APR uses a different charset for the filesystem and the files itself. (E.g. on Mac OS/X and Windows the filesystem is UTF-8, but the charset is user defined)

Use svn_path_cstring_to_utf8() for translating paths to utf-8.

> +  repos_url = apr_pstrcat(pool, "file://", cwd_utf8->data,
> +                          "/test-patch-repos", NULL);

This won't work for cwd on Windows. That would give "file://C:/dir", while it should be "file:///C:/dir"
(Fixed in r927764)

> +  repos_url = svn_uri_canonicalize(repos_url, pool);
> +  wc_path = svn_dirent_join(cwd_utf8->data, "test-patch-wc", pool);
> +  wc_path = svn_dirent_canonicalize(wc_path, pool);

svn_dirent_join() always returns canonical paths.

> +  SVN_ERR(svn_io_remove_dir2(wc_path, FALSE, NULL, NULL, pool));

This won't work if you never ran the tests before. philip made it pass TRUE for ignore_enoent in r927760)

> +  rev.kind = svn_opt_revision_head;
> +  peg_rev.kind = svn_opt_revision_unspecified;
> +  SVN_ERR(svn_client_create_context(&ctx, pool));
> +  SVN_ERR(svn_client_checkout3(NULL, repos_url, wc_path,
> +                               &peg_rev, &rev, svn_depth_infinity,
> +                               TRUE, FALSE, ctx, pool));
> +
> +  /* Create the patch file. */
> +  patch_file_path = svn_dirent_join(cwd_utf8->data, "test-patch.diff",
> pool);
> +  patch_file_path = svn_dirent_canonicalize(patch_file_path, pool);

Same here.

> +  SVN_ERR(svn_io_file_open(&patch_file, patch_file_path,
> +                           (APR_READ | APR_WRITE | APR_CREATE | APR_TRUNCATE),
> +                           APR_OS_DEFAULT, pool));
> +  for (i = 0; i < UNIDIFF_LINES; i++)
> +    {
> +      apr_size_t len = strlen(unidiff_patch[i]);
> +      SVN_ERR(svn_io_file_write(patch_file, unidiff_patch[i], &len, pool));
> +      SVN_ERR_ASSERT(len == strlen(unidiff_patch[i]));
> +    }
> +  SVN_ERR(svn_io_file_flush_to_disk(patch_file, pool));

You can just close the patch file here. That would flush it to disk.
> +
> +  /* Apply the patch. */
> +  SVN_ERR(svn_client_patch(patch_file_path, wc_path, FALSE, 0, FALSE,
> +                           NULL, NULL, &patched_tempfiles, &reject_tempfiles,
> +                           ctx, pool, pool));
> +  SVN_ERR(svn_io_file_close(patch_file, pool));

Instead of here. (You don't use the patch file before this, and it is no auto delete file)

> +
> +  SVN_ERR_ASSERT(apr_hash_count(patched_tempfiles) == 1);
> +  key = "A/D/gamma";
> +  patched_tempfile_path = apr_hash_get(patched_tempfiles, key,
> +                                       APR_HASH_KEY_STRING);
> +  SVN_ERR(check_patch_result(patched_tempfile_path, expected_gamma,
> +                             EXPECTED_GAMMA_LINES, pool));
> +  SVN_ERR_ASSERT(apr_hash_count(reject_tempfiles) == 1);
> +  key = "A/D/gamma";
> +  reject_tempfile_path = apr_hash_get(reject_tempfiles, key,
> +                                     APR_HASH_KEY_STRING);
> +  SVN_ERR(check_patch_result(reject_tempfile_path,
> expected_gamma_reject,
> +                             EXPECTED_GAMMA_REJECT_LINES, pool));
> +
> +  return SVN_NO_ERROR;
> +}
> +
>  /*
> ==========================================================
> ================ */
>  

>  struct svn_test_descriptor_t test_funcs[] =
> @@ -208,5 +370,6 @@ struct svn_test_descriptor_t test_funcs[
>                     "test svn_client__elide_mergeinfo_catalog"),
>      SVN_TEST_PASS2(test_args_to_target_array,
>                     "test svn_client_args_to_target_array"),
> +    SVN_TEST_OPTS_PASS(test_patch, "test svn_client_patch"),
>      SVN_TEST_NULL
>    };
>