> -----Original Message-----
> From: stsp_at_apache.org [mailto:stsp_at_apache.org]
> Sent: donderdag 25 maart 2010 23:44
> To: commits_at_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
> };
>
Received on 2010-03-26 11:51:24 CET