Stefan Sperling wrote:
> Author: stsp
> Date: Thu Jul 9 07:37:30 2009
> New Revision: 38383
>
> Log:
> Fix totally broken handling of paths in 'svn patch' in many places.
Sounds good but unfortunately seems to have broken the Windows
build-bot, something to do with Windows drive-letter-root paths.
<http://crest.ics.uci.edu/buildbot/builders/win32-xp%
20VS2005/builds/402>
FAIL: patch_tests.py 4: apply a unidiff patch containing absolute paths
[[[
CMD: svnadmin.exe create "svn-test-work\repositories\patch_tests-4"
--bdb-txn-nosync "--fs-type=fsfs"
<TIME = 0.625000>
CMD: svnadmin.exedump svn-test-work\local_tmp\repos | svnadmin.exeload svn-test-work\repositories\patch_tests-4 --ignore-uuid
<TIME = 0.047000>
CMD: svn.exe co "file:///M%3A/svn-auto-test/fsfs/subversion/tests/cmdline/svn-test-work/repositories/patch_tests-4" "svn-test-work\working_copies\patch_tests-4" --config-dir "M:\svn-auto-test\fsfs\subversion\tests\cmdline\svn-test-work\local_tmp\config" --password rayjandom --no-auth-cache --username jrandom
<TIME = 9.250000>
A svn-test-work\working_copies\patch_tests-4\A
A svn-test-work\working_copies\patch_tests-4\A\B
A svn-test-work\working_copies\patch_tests-4\A\B\lambda
A svn-test-work\working_copies\patch_tests-4\A\B\E
A svn-test-work\working_copies\patch_tests-4\A\B\E\alpha
A svn-test-work\working_copies\patch_tests-4\A\B\E\beta
A svn-test-work\working_copies\patch_tests-4\A\B\F
A svn-test-work\working_copies\patch_tests-4\A\mu
A svn-test-work\working_copies\patch_tests-4\A\C
A svn-test-work\working_copies\patch_tests-4\A\D
A svn-test-work\working_copies\patch_tests-4\A\D\gamma
A svn-test-work\working_copies\patch_tests-4\A\D\G
A svn-test-work\working_copies\patch_tests-4\A\D\G\pi
A svn-test-work\working_copies\patch_tests-4\A\D\G\rho
A svn-test-work\working_copies\patch_tests-4\A\D\G\tau
A svn-test-work\working_copies\patch_tests-4\A\D\H
A svn-test-work\working_copies\patch_tests-4\A\D\H\chi
A svn-test-work\working_copies\patch_tests-4\A\D\H\omega
A svn-test-work\working_copies\patch_tests-4\A\D\H\psi
A svn-test-work\working_copies\patch_tests-4\iota
Checked out revision 1.
CMD: svn.exe patch "M:\svn-auto-test\fsfs\subversion\tests\cmdline\svn-test-work\local_tmp\tmpksra8d" . --config-dir "M:\svn-auto-test\fsfs\subversion\tests\cmdline\svn-test-work\local_tmp\config" --password rayjandom --no-auth-cache --username jrandom
<TIME = 1.125000>
U A\B\E\alpha
Skipped 'M:\A\B\lambda'
=============================================================
'svn patch' unexpectedly skipped: __SVN_ROOT_NODE\M:
=============================================================
Traceback (most recent call last):
File "C:\svn-builder\djh-xp-vse2005\build\subversion\tests\cmdline\svntest\main.py", line 1149, in run
rc = self.pred.run(sandbox)
File "C:\svn-builder\djh-xp-vse2005\build\subversion\tests\cmdline\svntest\testcase.py", line 129, in run
return self.func(sandbox)
File "C:\svn-builder\djh-xp-vse2005\build\subversion/tests/cmdline/patch_tests.py", line 543, in patch_unidiff_absolute_paths
0) # dry-run
File "C:\svn-builder\djh-xp-vse2005\build\subversion\tests\cmdline\svntest\actions.py", line 1042, in run_and_verify_patch
extra_skip, None, missing_skip, None)
File "C:\svn-builder\djh-xp-vse2005\build\subversion\tests\cmdline\svntest\tree.py", line 675, in compare_trees
singleton_handler_a(a_child, a_baton)
File "C:\svn-builder\djh-xp-vse2005\build\subversion\tests\cmdline\svntest\actions.py", line 1036, in extra_skip
raise Failure
Failure
FAIL: patch_tests.py 4: apply a unidiff patch containing absolute paths
]]]
> * subversion/tests/cmdline/patch_tests.py
> (patch_unidiff): Switch this test to applying patches in a situation
> where the target working copy is not the current directory (which
> didn't work at all before this commit).
>
> * subversion/libsvn_client/patch.c
> (svn_client_patch): Make the target path absolute before doing anything
> else, and use the absolute form throughout.
> (patch_target_t): Rename PATH field to WC_PATH to reflect its meaning.
> Add new PATCH_PATH member which contains the target's path as it
> appeared in the patch file.
> (resolve_target_path): Take care to correctly resolve all the possible
> forms of the target path. Also, canonicalise the path before using it.
> (init_patch_target): Use the absolute target path throughout.
> (maybe_send_patch_target_notification): New parameter WC_DIR, which is
> prepended to TARGET->WC_PATH when sending notifications.
> If TARGET->WC_PATH is not known, fall back to other paths we know.
> Also change the return type to we can use SVN_ERR_ASSERT() here.
> (apply_one_patch): Use absolute paths of the target instead of what
> we read from the patch. Also, remove the adm_access dance when adding
> files to version control, it is not needed now that we open the
> adm_access with an absolute path in the first place. Also, pass the
> new WC_PATH parameter to maybe_send_patch_target_notification() and
> pass on any errors returned by it.
>
> Modified:
> trunk/subversion/libsvn_client/patch.c
> trunk/subversion/tests/cmdline/patch_tests.py
>
> Modified: trunk/subversion/libsvn_client/patch.c
> URL: http://svn.collab.net/viewvc/svn/trunk/subversion/libsvn_client/patch.c?pathrev=38383&r1=38382&r2=38383
> ==============================================================================
> --- trunk/subversion/libsvn_client/patch.c Thu Jul 9 05:27:41 2009 (r38382)
> +++ trunk/subversion/libsvn_client/patch.c Thu Jul 9 07:37:30 2009 (r38383)
> @@ -1734,8 +1734,10 @@ svn_client_patch(const char *patch_path,
> svn_wc_adm_access_t *adm_access;
> struct edit_baton *eb;
> svn_boolean_t dry_run = FALSE; /* disable dry_run for now */
> + const char *abs_target;
>
> - SVN_ERR(svn_wc_adm_open3(&adm_access, NULL, target,
> + SVN_ERR(svn_dirent_get_absolute(&abs_target, target, pool));
> + SVN_ERR(svn_wc_adm_open3(&adm_access, NULL, abs_target,
> TRUE, -1, ctx->cancel_func, ctx->cancel_baton,
> pool));
>
> @@ -1749,13 +1751,13 @@ svn_client_patch(const char *patch_path,
> patch_cmd_baton.force = force;
> patch_cmd_baton.dry_run = dry_run;
> patch_cmd_baton.added_path = NULL;
> - patch_cmd_baton.target = target;
> + patch_cmd_baton.target = abs_target;
> patch_cmd_baton.ctx = ctx;
> patch_cmd_baton.dry_run_deletions = (dry_run ? apr_hash_make(pool)
> : NULL);
> patch_cmd_baton.pool = pool;
>
> - eb = make_editor_baton(target, adm_access, dry_run, &patch_callbacks,
> + eb = make_editor_baton(abs_target, adm_access, dry_run, &patch_callbacks,
> &patch_cmd_baton, ctx->notify_func2,
> ctx->notify_baton2, &diff_editor, pool);
>
> @@ -1791,11 +1793,17 @@ typedef struct {
> /* The patch being applied. */
> const svn_patch_t *patch;
>
> + /* The target path, as it appeared in the patch file.
> + * This is always known. */
> + const char *patch_path;
> +
> /* The target path, relative to the working copy directory the
> - * patch is being applied to. */
> - const char *path;
> + * patch is being applied to. A patch strip count applies to this
> + * and only this path. Is not always known, so may be NULL. */
> + const char *wc_path;
>
> - /* The absolute path of the target */
> + /* The absolute path of the target.
> + * Is not always known, so it may be NULL. */
> char *abs_path;
>
> /* The target file, read-only, seekable. This is NULL in case the target
> @@ -1843,73 +1851,93 @@ typedef struct {
> svn_boolean_t local_mods;
> } patch_target_t;
>
> -/* Resolve the exact path for a patch TARGET at path TARGET_PATH,
> - * and set up TARGET->PATH, TARGET->ABS_PATH, and TARGET->KIND accordingly.
> - * Also indicate whether the target should be skipped in TARGET->SKIPPED.
> - *
> - * If TARGET_PATH is absolute, resolve symlinks it might contain,
> - * and make sure that it points somewhere inside of the working copy
> - * directory WC_PATH.
> - *
> - * Use RESULT_POOL for allocations of fields in TARGET.
> - * */
> +/* Resolve the exact path for a patch TARGET at path PATCH_TARGET_PATH,
> + * which is the path of the target as it appeared in the patch file.
> + * Set up TARGET->PATCH_PATH, TARGET->WC_PATH, TARGET->ABS_PATH, and
> + * TARGET->KIND accordingly.
> + * Indicate in TARGET->SKIPPED whether the target should be skipped.
> + * Use RESULT_POOL for allocations of fields in TARGET. */
> static svn_error_t *
> -resolve_target_path(patch_target_t *target, const char *target_path,
> +resolve_target_path(patch_target_t *target, const char *patch_target_path,
> const char *wc_path, apr_pool_t *result_pool,
> apr_pool_t *scratch_pool)
> {
> const char *abs_wc_path;
> - const char *child_path;
> + const char *canon_target_path;
>
> - /* If the path is a child of the working copy directory,
> - * pass a relative path to svn_dirent_is_under_root() below.
> - * Passing it an absolute path will always fail. */
> SVN_ERR(svn_dirent_get_absolute(&abs_wc_path, wc_path, scratch_pool));
> - child_path = svn_dirent_is_child(abs_wc_path, target_path, scratch_pool);
> + canon_target_path = svn_dirent_canonicalize(patch_target_path, scratch_pool);
>
> - /* Make sure the path is secure to use. We want the target to be inside
> - * of the working copy. */
> - if (svn_dirent_is_under_root(&target->abs_path, abs_wc_path,
> - child_path ? child_path : target_path,
> - result_pool))
> - {
> - target->path = svn_dirent_is_child(abs_wc_path, target->abs_path,
> - result_pool);
> - SVN_ERR_ASSERT(target->path);
> -
> - /* Find out if there is a suitable patch target at the target path,
> - * and determine if the target should be skipped. */
> - SVN_ERR(svn_io_check_path(target->abs_path, &target->kind, scratch_pool));
> - switch (target->kind)
> - {
> - case svn_node_file:
> - target->skipped = FALSE;
> - break;
> - case svn_node_none:
> - {
> - const char *dirname;
> - svn_node_kind_t kind;
> + if (strlen(canon_target_path) == 0)
> + {
> + /* An empty patch target path? What gives? Skip this. */
> + target->skipped = TRUE;
> + target->kind = svn_node_file;
> + target->patch_path = "";
> + target->abs_path = NULL;
> + target->wc_path = NULL;
> + return SVN_NO_ERROR;
> + }
>
> - /* The file is not there, that's fine. The patch might want to
> - * create it. But the containing directory of the target needs
> - * to exists. Otherwise we won't be able to apply the patch. */
> - dirname = svn_dirent_dirname(target->abs_path, scratch_pool);
> - SVN_ERR(svn_io_check_path(dirname, &kind, scratch_pool));
> - target->skipped = (kind != svn_node_dir);
> - break;
> - }
> - default:
> - target->skipped = TRUE;
> - break;
> + target->patch_path = apr_pstrdup(result_pool, patch_target_path);
> +
> + if (svn_dirent_is_absolute(canon_target_path))
> + {
> + /* TODO: strip count */
> + target->wc_path = svn_dirent_is_child(abs_wc_path, canon_target_path,
> + scratch_pool);
> + if (! target->wc_path)
> + {
> + /* The target path is either outside of the working copy
> + * or it is the working copy itself. Skip it. */
> + target->skipped = TRUE;
> + target->kind = svn_node_file;
> + target->abs_path = apr_pstrdup(result_pool, canon_target_path);
> + target->wc_path = NULL;
> + return SVN_NO_ERROR;
> }
> }
> else
> {
> + target->wc_path = canon_target_path; /* TODO: strip count */
> + }
> +
> + /* Make sure the path is secure to use. We want the target to be inside
> + * of the working copy and not be fooled by symlinks it might contain. */
> + if (! svn_dirent_is_under_root(&target->abs_path, abs_wc_path,
> + target->wc_path, result_pool))
> + {
> /* The target path is outside of the working copy. Skip it. */
> target->skipped = TRUE;
> target->kind = svn_node_file;
> - target->path = target_path;
> target->abs_path = NULL;
> + return SVN_NO_ERROR;
> + }
> +
> + /* Find out if there is a suitable patch target at the target path,
> + * and determine if the target should be skipped. */
> + SVN_ERR(svn_io_check_path(target->abs_path, &target->kind, scratch_pool));
> + switch (target->kind)
> + {
> + case svn_node_file:
> + target->skipped = FALSE;
> + break;
> + case svn_node_none:
> + {
> + const char *dirname;
> + svn_node_kind_t kind;
> +
> + /* The file is not there, that's fine. The patch might want to
> + * create it. But the containing directory of the target needs
> + * to exists. Otherwise we won't be able to apply the patch. */
> + dirname = svn_dirent_dirname(target->abs_path, scratch_pool);
> + SVN_ERR(svn_io_check_path(dirname, &kind, scratch_pool));
> + target->skipped = (kind != svn_node_dir);
> + break;
> + }
> + default:
> + target->skipped = TRUE;
> + break;
> }
>
> return SVN_NO_ERROR;
> @@ -1981,8 +2009,6 @@ init_patch_target(patch_target_t **targe
> *target = NULL;
> new_target = apr_pcalloc(result_pool, sizeof(*new_target));
>
> - /* TODO: strip count */
> -
> SVN_ERR(resolve_target_path(new_target, patch->new_filename,
> svn_wc_adm_access_path(adm_access),
> result_pool, scratch_pool));
> @@ -1990,7 +2016,7 @@ init_patch_target(patch_target_t **targe
> if (new_target->kind == svn_node_file && ! new_target->skipped)
> {
> /* Try to open the target file */
> - SVN_ERR(svn_io_file_open(&new_target->file, new_target->path,
> + SVN_ERR(svn_io_file_open(&new_target->file, new_target->abs_path,
> APR_READ | APR_BINARY | APR_BUFFERED,
> APR_OS_DEFAULT, result_pool));
> SVN_ERR(svn_subst_detect_file_eol(&new_target->eol_str, new_target->file,
> @@ -2018,7 +2044,7 @@ init_patch_target(patch_target_t **targe
> svn_io_file_del_none,
> result_pool, scratch_pool));
>
> - SVN_ERR(check_local_mods(&new_target->local_mods, new_target->path,
> + SVN_ERR(check_local_mods(&new_target->local_mods, new_target->abs_path,
> adm_access, scratch_pool));
> }
>
> @@ -2309,7 +2335,7 @@ read_lines_from_target(svn_stream_t **li
>
> target->current_line += i;
>
> - SVN_ERR(svn_io_file_open(&file, target->path, flags, APR_OS_DEFAULT,
> + SVN_ERR(svn_io_file_open(&file, target->abs_path, flags, APR_OS_DEFAULT,
> result_pool));
> *lines = svn_stream_from_aprfile_range_readonly(file, FALSE, start, end,
> result_pool);
> @@ -2499,18 +2525,21 @@ apply_one_hunk(const svn_hunk_t *hunk, p
> return SVN_NO_ERROR;
> }
>
> -/* Use client context CTX to send a suitable notification for a patch TARGET.
> +/* Use client context CTX to send a suitable notification for a patch
> + * TARGET being applied to WC_DIR.
> * Use POOL for temporary allocations. */
> -static void
> +static svn_error_t *
> maybe_send_patch_target_notification(const patch_target_t *target,
> + const char *wc_dir,
> const svn_client_ctx_t *ctx,
> apr_pool_t *pool)
> {
> svn_wc_notify_t *notify;
> svn_wc_notify_action_t action;
> + const char *path;
>
> if (! ctx->notify_func2)
> - return;
> + return SVN_NO_ERROR;
>
> if (target->skipped)
> action = svn_wc_notify_skip;
> @@ -2519,7 +2548,17 @@ maybe_send_patch_target_notification(con
> else
> action = svn_wc_notify_update_update;
>
> - notify = svn_wc_create_notify(target->path, action, pool);
> + /* Figure out which path to report for the patch target.
> + * Try all possibilities in order of preference. */
> + if (target->wc_path)
> + path = svn_dirent_join(wc_dir, target->wc_path, pool);
> + else if (target->abs_path)
> + path = target->abs_path;
> + else
> + path = target->patch_path;
> + SVN_ERR_ASSERT(path);
> +
> + notify = svn_wc_create_notify(path, action, pool);
> notify->kind = svn_node_file;
>
> if (action == svn_wc_notify_skip)
> @@ -2544,6 +2583,8 @@ maybe_send_patch_target_notification(con
> }
>
> (*ctx->notify_func2)(ctx->notify_baton2, notify, pool);
> +
> + return SVN_NO_ERROR;
> }
>
> /* Apply a PATCH. Use client context CTX to send notifiations.
> @@ -2557,6 +2598,7 @@ apply_one_patch(svn_patch_t *patch, svn_
> apr_pool_t *pool)
> {
> patch_target_t *target;
> + const char *wc_path;
>
> SVN_ERR(init_patch_target(&target, patch, adm_access, ctx,
> tempfiles, pool, pool));
> @@ -2610,7 +2652,7 @@ apply_one_patch(svn_patch_t *patch, svn_
> {
> /* Install the patched temporary file over the working file.
> * ### Should this rather be done in a loggy fashion? */
> - SVN_ERR(svn_io_file_rename(target->result_path, patch->new_filename,
> + SVN_ERR(svn_io_file_rename(target->result_path, target->abs_path,
> pool));
>
> if (target->kind == svn_node_none)
> @@ -2619,13 +2661,7 @@ apply_one_patch(svn_patch_t *patch, svn_
> * control. Suppress the notification, we'll do it manually in
> * a minute (which is a work-around for otherwise not quite pretty
> * output of the svn CLI client...) */
> - svn_wc_adm_access_t *parent_adm_access;
> - const char *dirname = svn_dirent_dirname(patch->new_filename,
> - pool);
> - SVN_ERR(svn_wc_adm_retrieve(&parent_adm_access, adm_access,
> - dirname, pool));
> - SVN_ERR(svn_wc_add3(patch->new_filename,
> - parent_adm_access,
> + SVN_ERR(svn_wc_add3(target->abs_path, adm_access,
> svn_depth_infinity,
> NULL, SVN_INVALID_REVNUM,
> ctx->cancel_func, ctx->cancel_baton,
> @@ -2639,7 +2675,8 @@ apply_one_patch(svn_patch_t *patch, svn_
> }
> }
>
> - maybe_send_patch_target_notification(target, ctx, pool);
> + wc_path = svn_wc_adm_access_path(adm_access);
> + SVN_ERR(maybe_send_patch_target_notification(target, wc_path, ctx, pool));
>
> return SVN_NO_ERROR;
> }
>
> Modified: trunk/subversion/tests/cmdline/patch_tests.py
> URL: http://svn.collab.net/viewvc/svn/trunk/subversion/tests/cmdline/patch_tests.py?pathrev=38383&r1=38382&r2=38383
> ==============================================================================
> --- trunk/subversion/tests/cmdline/patch_tests.py Thu Jul 9 05:27:41 2009 (r38382)
> +++ trunk/subversion/tests/cmdline/patch_tests.py Thu Jul 9 07:37:30 2009 (r38383)
> @@ -284,13 +284,11 @@ def patch_unidiff(sbox):
> "winnings with the below details.\n",
> ]
>
> - os.chdir(wc_dir)
> -
> expected_output = [
> - 'U %s\n' % os.path.join('A', 'D', 'gamma'),
> - 'U iota\n',
> - 'A new\n',
> - 'U %s\n' % os.path.join('A', 'mu'),
> + 'U %s\n' % os.path.join(wc_dir, 'A', 'D', 'gamma'),
> + 'U %s\n' % os.path.join(wc_dir, 'iota'),
> + 'A %s\n' % os.path.join(wc_dir, 'new'),
> + 'U %s\n' % os.path.join(wc_dir, 'A', 'mu'),
> ]
>
> expected_disk = svntest.main.greek_state.copy()
> @@ -299,7 +297,7 @@ def patch_unidiff(sbox):
> expected_disk.add({'new' : Item(contents=new_contents)})
> expected_disk.tweak('A/mu', contents=''.join(mu_contents))
>
> - expected_status = svntest.actions.get_virginal_state('.', 1)
> + expected_status = svntest.actions.get_virginal_state(wc_dir, 1)
> expected_status.tweak('A/D/gamma', status='M ')
> expected_status.tweak('iota', status='M ')
> expected_status.add({'new' : Item(status='A ', wc_rev=0)})
> @@ -307,7 +305,7 @@ def patch_unidiff(sbox):
>
> expected_skip = wc.State('', { })
>
> - svntest.actions.run_and_verify_patch('.', os.path.abspath(patch_file_path),
> + svntest.actions.run_and_verify_patch(wc_dir, os.path.abspath(patch_file_path),
> expected_output,
> expected_disk,
> expected_status,
>
> ------------------------------------------------------
> http://subversion.tigris.org/ds/viewMessage.do?dsForumId=495&dsMessageId=2369406
------------------------------------------------------
http://subversion.tigris.org/ds/viewMessage.do?dsForumId=462&dsMessageId=2369451
Received on 2009-07-09 19:06:29 CEST