From dev-return-2862-daniel=haxx.se@subversion.apache.org Fri Mar 26 11:51:24 2010 Return-Path: Received: from mail.apache.org (hermes.apache.org [140.211.11.3]) by giant.haxx.se (8.14.3/8.14.3/Debian-9) with SMTP id o2QApN7P024537 for ; Fri, 26 Mar 2010 11:51:23 +0100 Received: (qmail 56263 invoked by uid 500); 26 Mar 2010 10:51:18 -0000 Mailing-List: contact dev-help@subversion.apache.org; run by ezmlm Precedence: bulk List-Help: List-Unsubscribe: List-Post: List-Id: Delivered-To: mailing list dev@subversion.apache.org Received: (qmail 56250 invoked by uid 99); 26 Mar 2010 10:51:17 -0000 Received: from athena.apache.org (HELO athena.apache.org) (140.211.11.136) by apache.org (qpsmtpd/0.29) with ESMTP; Fri, 26 Mar 2010 10:51:17 +0000 X-ASF-Spam-Status: No, hits=-0.6 required=10.0 tests=AWL,RCVD_IN_DNSWL_NONE,SPF_NEUTRAL X-Spam-Check-By: apache.org Received-SPF: neutral (athena.apache.org: local policy) Received: from [209.85.218.223] (HELO mail-bw0-f223.google.com) (209.85.218.223) by apache.org (qpsmtpd/0.29) with ESMTP; Fri, 26 Mar 2010 10:51:09 +0000 Received: by bwz23 with SMTP id 23so1866595bwz.36 for ; Fri, 26 Mar 2010 03:50:46 -0700 (PDT) Received: by 10.204.74.32 with SMTP id s32mr798453bkj.148.1269600646352; Fri, 26 Mar 2010 03:50:46 -0700 (PDT) Received: from tcgws000 (183-019-045-062.static.caiway.nl [62.45.19.183]) by mx.google.com with ESMTPS id a11sm6680405bkc.9.2010.03.26.03.50.44 (version=TLSv1/SSLv3 cipher=RC4-MD5); Fri, 26 Mar 2010 03:50:45 -0700 (PDT) From: "Bert Huijben" To: , References: <20100325224407.4ED672388903@eris.apache.org> In-Reply-To: <20100325224407.4ED672388903@eris.apache.org> 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 11:50:44 +0100 Message-ID: <030501caccd2$2ff256d0$8fd70470$@qqmail.nl> MIME-Version: 1.0 Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: quoted-printable X-Mailer: Microsoft Outlook 14.0 Thread-Index: AQKg8Q4k4SSoTB0RkQolFKW6ojQddAMat1gq Content-Language: 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 >=20 > Author: stsp > Date: Thu Mar 25 22:44:06 2010 > New Revision: 927620 >=20 > URL: http://svn.apache.org/viewvc?rev=3D927620&view=3Drev > Log: > Fix issue #3598, "Allow access to patched temporary files via svn = patch API" >=20 > * 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. >=20 > * 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. >=20 > * subversion/svn/patch-cmd.c > (svn_cl__patch): Track parameter changes to svn_client_patch(). >=20 > * subversion/tests/libsvn_client: Ignore test-patch* >=20 > * 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. >=20 > 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 >=20 > 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=3D927620&r1=3D927619&r2=3D927620&view=3Ddiff > = =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D > =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D > --- 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 @@ > */ >=20 > =20 > +#include This is a unix only include file.=20 Please use some appropriate check (E.g. APR_HAVE_UNISTD_H) or just use = an apr equivalent. (Fixed in r 927764) > +#include > #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" >=20 > #include "../svn_test.h" > +#include "../svn_test_fs.h" >=20 > typedef struct { > const char *path; > @@ -199,6 +204,163 @@ test_args_to_target_array(apr_pool_t *po > return SVN_NO_ERROR; > } >=20 > + > +/* 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 =3D 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 =3D svn_stream_from_aprfile2(patched_file, TRUE, pool); > + i =3D 0; > + iterpool =3D 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) =3D=3D = 0); > + if (eof) > + break; > + } > + svn_pool_destroy(iterpool); > + > + SVN_ERR(svn_io_file_close(patched_file, pool)); > + SVN_ERR_ASSERT(i =3D=3D 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] =3D { > + "Index: A/D/gamma" NL, > + > = "=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D > =3D=3D=3D=3D=3D=3D=3D=3D=3D\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] =3D { > + "This is the file 'gamma'." > + }; > +#define EXPECTED_GAMMA_REJECT_LINES 5 > + const char *expected_gamma_reject[EXPECTED_GAMMA_REJECT_LINES] > =3D { > + "--- 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 =3D 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 =3D 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 =3D 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 =3D svn_uri_canonicalize(repos_url, pool); > + wc_path =3D svn_dirent_join(cwd_utf8->data, "test-patch-wc", pool); > + wc_path =3D 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 =3D svn_opt_revision_head; > + peg_rev.kind =3D 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 =3D svn_dirent_join(cwd_utf8->data, = "test-patch.diff", > pool); > + patch_file_path =3D 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 =3D 0; i < UNIDIFF_LINES; i++) > + { > + apr_size_t len =3D strlen(unidiff_patch[i]); > + SVN_ERR(svn_io_file_write(patch_file, unidiff_patch[i], &len, = pool)); > + SVN_ERR_ASSERT(len =3D=3D 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) =3D=3D 1); > + key =3D "A/D/gamma"; > + patched_tempfile_path =3D 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) =3D=3D 1); > + key =3D "A/D/gamma"; > + reject_tempfile_path =3D 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; > +} > + > /* > = =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D > =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D */ > =20 > struct svn_test_descriptor_t test_funcs[] =3D > @@ -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 > }; >=20