subversion-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Bert Huijben" <b...@qqmail.nl>
Subject 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
Date Fri, 26 Mar 2010 10:50:44 GMT


> -----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
>    };
> 



Mime
View raw message