Re: svn commit: r1096264 - in /subversion/trunk/subversion: include/svn_wc.h libsvn_client/diff.c libsvn_wc/deprecated.c libsvn_wc/diff.c

[Greg Stein <gs...@gmail.com>]

This broke a ton of diff tests. Did you even run them before committing
this?

Seriously, I'm really tired of all the buildbot breakage.
On Apr 23, 2011 9:45 PM, <rh...@apache.org> wrote:
> Author: rhuijben
> Date: Sun Apr 24 01:44:57 2011
> New Revision: 1096264
>
> URL: http://svn.apache.org/viewvc?rev=1096264&view=rev
> Log:
> Make the diff editor report changes relative from the anchor, instead of
> relative from whatever is passed in to work more like the other editors.
> (And reverse this behavior in the deprecated wrappers)
>
> * subversion/include/svn_wc.h
> (svn_wc_get_diff_editor6): Update arguments. Takes an anchor_abspath now.
> (svn_wc_diff6): Update arguments. Takes a target_abspath now.
>
> * subversion/libsvn_client/diff.c
> (diff_cmd_baton): Store anchor.
> (diff_props_changed): Update documentation.
> (diff_dir_props_changed): New function, wrapping diff_dir_props_changed.
> Calculate old style path.
> (diff_file_changed,
> diff_file_added,
> diff_file_deleted_with_diff,
> diff_file_deleted_no_diff,
> diff_dir_added,
> diff_dir_deleted,
> diff_dir_opened,
> diff_dir_closed): Calculate old style path.
> (find_wc_root): Remove function.
> (diff_wc_wc): Calculate anchor and store in baton.
> (diff_repos_wc): Fetch wcroot directly and store anchor in baton.
>
> * subversion/libsvn_wc/deprecated.c
> (diff_callbacks3_wrapper_baton): Add anchor.
> (wrap_4to3_file_changed,
> wrap_4to3_file_added,
> wrap_4to3_file_deleted,
> wrap_4to3_dir_added,
> wrap_4to3_dir_deleted,
> wrap_4to3_dir_props_changed,
> wrap_4to3_dir_opened,
> wrap_4to3_dir_closed): Join anchor before path.
> (svn_wc_get_diff_editor5): Pass anchor_abspath and store anchor.
> svn_wc_diff5): Pass target_abspath and store anchor.
>
> * subversion/libsvn_wc/diff.c
> (edit_baton): Remove anchor.
> (dir_baton): Update documentation of compared.
> (file_baton): Remove wc_path.
> (make_edit_baton): Update arguments.
> (make_file_baton): Don't calculate wc_path.
> (walk_local_nodes_diff): Simplify and path anchor calculations.
> (report_wc_directory_as_added): Use relpath join.
> (set_target_revision): Root path is "" now.
> (delete_entry,
> add_directory,
> open_directory): Simplify path calculations.
> (close_directory): Store path as key.
> (add_file,
> open_file): Simplify path calculations.
> (close_edit): Use "" for anchor path.
> (svn_wc_get_diff_editor6): Ensure anchor_abspath is absolute.
> (svn_wc_diff6): Expect abspath
>
> Modified:
> subversion/trunk/subversion/include/svn_wc.h
> subversion/trunk/subversion/libsvn_client/diff.c
> subversion/trunk/subversion/libsvn_wc/deprecated.c
> subversion/trunk/subversion/libsvn_wc/diff.c
>
> Modified: subversion/trunk/subversion/include/svn_wc.h
> URL:
http://svn.apache.org/viewvc/subversion/trunk/subversion/include/svn_wc.h?rev=1096264&r1=1096263&r2=1096264&view=diff
>
==============================================================================
> --- subversion/trunk/subversion/include/svn_wc.h (original)
> +++ subversion/trunk/subversion/include/svn_wc.h Sun Apr 24 01:44:57 2011
> @@ -5915,7 +5915,8 @@ svn_wc_canonicalize_svn_prop(const svn_s
> * working copy (when @a use_text_base is #FALSE), or the current pristine
> * information (when @a use_text_base is #TRUE) against the editor driver.
> *
> - * @a anchor_path/@a target represent the base of the hierarchy to be
compared.
> + * @a anchor_abspath/@a target represent the base of the hierarchy to be
> + * compared. The diff callback paths will be relative to this path.
> *
> * @a callbacks/@a callback_baton is the callback table to use when two
> * files are to be compared.
> @@ -5958,7 +5959,7 @@ svn_error_t *
> svn_wc_get_diff_editor6(const svn_delta_editor_t **editor,
> void **edit_baton,
> svn_wc_context_t *wc_ctx,
> - const char *anchor_path,
> + const char *anchor_abspath,
> const char *target,
> const svn_wc_diff_callbacks4_t *callbacks,
> void *callback_baton,
> @@ -6097,7 +6098,7 @@ svn_wc_get_diff_editor(svn_wc_adm_access
> /**
> * Compare working copy against the text-base.
> *
> - * @a target_path represents the base of the hierarchy to be compared.
> + * @a target_abspath represents the base of the hierarchy to be compared.
> *
> * @a callbacks/@a callback_baton is the callback table to use when two
> * files are to be compared.
> @@ -6136,7 +6137,7 @@ svn_wc_get_diff_editor(svn_wc_adm_access
> */
> svn_error_t *
> svn_wc_diff6(svn_wc_context_t *wc_ctx,
> - const char *target_path,
> + const char *target_abspath,
> const svn_wc_diff_callbacks4_t *callbacks,
> void *callback_baton,
> svn_depth_t depth,
>
> Modified: subversion/trunk/subversion/libsvn_client/diff.c
> URL:
http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_client/diff.c?rev=1096264&r1=1096263&r2=1096264&view=diff
>
==============================================================================
> --- subversion/trunk/subversion/libsvn_client/diff.c (original)
> +++ subversion/trunk/subversion/libsvn_client/diff.c Sun Apr 24 01:44:57
2011
> @@ -776,6 +776,9 @@ struct diff_cmd_baton {
> * directory of the working copy involved in the diff. */
> const char *wc_root_abspath;
>
> + /* The anchor to prefix before wc paths */
> + const char *anchor;
> +
> /* A hashtable using the visited paths as keys.
> * ### This is needed for us to know if we need to print a diff header for
> * ### a path that has property changes. */
> @@ -783,8 +786,8 @@ struct diff_cmd_baton {
> };
>
>
> -/* An svn_wc_diff_callbacks4_t function. Used for both file and directory
> - property diffs. */
> +/* An helper for diff_dir_props_changed, diff_file_changed and
diff_file_added
> + */
> static svn_error_t *
> diff_props_changed(const char *local_dir_abspath,
> svn_wc_notify_state_t *state,
> @@ -843,6 +846,31 @@ diff_props_changed(const char *local_dir
> return SVN_NO_ERROR;
> }
>
> +/* An svn_wc_diff_callbacks4_t function. */
> +static svn_error_t *
> +diff_dir_props_changed(const char *local_dir_abspath,
> + svn_wc_notify_state_t *state,
> + svn_boolean_t *tree_conflicted,
> + const char *path,
> + const apr_array_header_t *propchanges,
> + apr_hash_t *original_props,
> + void *diff_baton,
> + apr_pool_t *scratch_pool)
> +{
> + struct diff_cmd_baton *diff_cmd_baton = diff_baton;
> +
> + if (diff_cmd_baton->anchor)
> + path = svn_dirent_join(diff_cmd_baton->anchor, path, scratch_pool);
> +
> + return svn_error_return(diff_props_changed(local_dir_abspath, state,
> + tree_conflicted, path,
> + propchanges,
> + original_props,
> + diff_baton,
> + scratch_pool));
> +}
> +
> +
> /* Show differences between TMPFILE1 and TMPFILE2. PATH, REV1, and REV2
are
> used in the headers to indicate the file and revisions. If either
> MIMETYPE1 or MIMETYPE2 indicate binary content, don't show a diff,
> @@ -1039,6 +1067,9 @@ diff_file_changed(const char *local_dir_
> void *diff_baton,
> apr_pool_t *scratch_pool)
> {
> + struct diff_cmd_baton *diff_cmd_baton = diff_baton;
> + if (diff_cmd_baton->anchor)
> + path = svn_dirent_join(diff_cmd_baton->anchor, path, scratch_pool);
> if (tmpfile1)
> SVN_ERR(diff_content_changed(path,
> tmpfile1, tmpfile2, rev1, rev2,
> @@ -1083,6 +1114,9 @@ diff_file_added(const char *local_dir_ab
> {
> struct diff_cmd_baton *diff_cmd_baton = diff_baton;
>
> + if (diff_cmd_baton->anchor)
> + path = svn_dirent_join(diff_cmd_baton->anchor, path, scratch_pool);
> +
> /* We want diff_file_changed to unconditionally show diffs, even if
> the diff is empty (as would be the case if an empty file were
> added.) It's important, because 'patch' would still see an empty
> @@ -1133,6 +1167,9 @@ diff_file_deleted_with_diff(const char *
> {
> struct diff_cmd_baton *diff_cmd_baton = diff_baton;
>
> + if (diff_cmd_baton->anchor)
> + path = svn_dirent_join(diff_cmd_baton->anchor, path, scratch_pool);
> +
> if (tmpfile1)
> SVN_ERR(diff_content_changed(path,
> tmpfile1, tmpfile2, diff_cmd_baton->revnum1,
> @@ -1166,6 +1203,9 @@ diff_file_deleted_no_diff(const char *lo
> {
> struct diff_cmd_baton *diff_cmd_baton = diff_baton;
>
> + if (diff_cmd_baton->anchor)
> + path = svn_dirent_join(diff_cmd_baton->anchor, path, scratch_pool);
> +
> if (state)
> *state = svn_wc_notify_state_unknown;
> if (tree_conflicted)
> @@ -1190,6 +1230,9 @@ diff_dir_added(const char *local_dir_abs
> void *diff_baton,
> apr_pool_t *scratch_pool)
> {
> + struct diff_cmd_baton *diff_cmd_baton = diff_baton;
> + if (diff_cmd_baton->anchor)
> + path = svn_dirent_join(diff_cmd_baton->anchor, path, scratch_pool);
> if (state)
> *state = svn_wc_notify_state_unknown;
> if (tree_conflicted)
> @@ -1209,6 +1252,9 @@ diff_dir_deleted(const char *local_dir_a
> void *diff_baton,
> apr_pool_t *scratch_pool)
> {
> + struct diff_cmd_baton *diff_cmd_baton = diff_baton;
> + if (diff_cmd_baton->anchor)
> + path = svn_dirent_join(diff_cmd_baton->anchor, path, scratch_pool);
> if (state)
> *state = svn_wc_notify_state_unknown;
> if (tree_conflicted)
> @@ -1229,6 +1275,9 @@ diff_dir_opened(const char *local_dir_ab
> void *diff_baton,
> apr_pool_t *scratch_pool)
> {
> + struct diff_cmd_baton *diff_cmd_baton = diff_baton;
> + if (diff_cmd_baton->anchor)
> + path = svn_dirent_join(diff_cmd_baton->anchor, path, scratch_pool);
> if (tree_conflicted)
> *tree_conflicted = FALSE;
> if (skip_children)
> @@ -1249,6 +1298,9 @@ diff_dir_closed(const char *local_dir_ab
> void *diff_baton,
> apr_pool_t *scratch_pool)
> {
> + struct diff_cmd_baton *diff_cmd_baton = diff_baton;
> + if (diff_cmd_baton->anchor)
> + path = svn_dirent_join(diff_cmd_baton->anchor, path, scratch_pool);
> if (contentstate)
> *contentstate = svn_wc_notify_state_unknown;
> if (propstate)
> @@ -1322,49 +1374,6 @@ convert_to_url(const char **url,
> return SVN_NO_ERROR;
> }
>
> -/* Return the absolute path to the root of the working copy which
> - * LOCAL_ABSPATH is located in, or NULL if LOCAL_ABSPATH is not within
> - * a working copy. Use working copy context WC_CTX.
> - * Allocate the result in RESULT_POOL.
> - * Use SCRATCH_POOL for temporary allocations. */
> -static const char *
> -find_wc_root(const char *local_abspath, svn_wc_context_t *wc_ctx,
> - apr_pool_t *result_pool, apr_pool_t *scratch_pool)
> -{
> - svn_boolean_t wc_root_found;
> - apr_pool_t *iterpool;
> -
> - wc_root_found = FALSE;
> - iterpool = svn_pool_create(scratch_pool);
> - while (! wc_root_found && *local_abspath)
> - {
> - svn_error_t *err;
> -
> - svn_pool_clear(iterpool);
> -
> - err = svn_wc_is_wc_root2(&wc_root_found, wc_ctx, local_abspath,
> - iterpool);
> - if (err)
> - {
> - /* Ignore all errors. We don't care, because if all we get is
> - * errors, the path is not in a working copy. */
> - svn_error_clear(err);
> - }
> -
> - if (svn_dirent_is_root(local_abspath, strlen(local_abspath)))
> - break;
> -
> - if (! wc_root_found)
> - local_abspath = svn_dirent_dirname(local_abspath, scratch_pool);
> - }
> - svn_pool_destroy(iterpool);
> -
> - if (wc_root_found)
> - return apr_pstrdup(result_pool, local_abspath);
> -
> - return NULL;
> -}
> -
> /** Check if paths PATH1 and PATH2 are urls and if the revisions REVISION1
> * and REVISION2 are local. If PEG_REVISION is not unspecified, ensure that
> * at least one of the two revisions is non-local.
> @@ -1604,6 +1613,7 @@ diff_wc_wc(const char *path1,
> {
> const char *abspath1;
> svn_error_t *err;
> + svn_node_kind_t kind;
>
> SVN_ERR_ASSERT(! svn_path_is_url(path1));
> SVN_ERR_ASSERT(! svn_path_is_url(path2));
> @@ -1640,8 +1650,15 @@ diff_wc_wc(const char *path1,
>
> callback_baton->revnum2 = SVN_INVALID_REVNUM; /* WC */
>
> + SVN_ERR(svn_wc_read_kind(&kind, ctx->wc_ctx, abspath1, FALSE, pool));
> +
> + if (kind != svn_node_dir)
> + callback_baton->anchor = svn_dirent_dirname(path1, pool);
> + else
> + callback_baton->anchor = path1;
> +
> SVN_ERR(svn_wc_diff6(ctx->wc_ctx,
> - path1,
> + abspath1,
> callbacks, callback_baton,
> depth,
> ignore_ancestry, show_copies_as_adds,
> @@ -1843,12 +1860,16 @@ diff_repos_wc(const char *path1,
> ctx, pool));
> callback_baton->ra_session = ra_session;
> if (use_git_diff_format)
> - callback_baton->wc_root_abspath = find_wc_root(anchor_abspath,
ctx->wc_ctx,
> - pool, pool);
> + {
> + SVN_ERR(svn_wc_get_wc_root(&callback_baton->wc_root_abspath,
> + ctx->wc_ctx, anchor_abspath,
> + pool, pool));
> + }
> + callback_baton->anchor = anchor;
>
> SVN_ERR(svn_wc_get_diff_editor6(&diff_editor, &diff_edit_baton,
> ctx->wc_ctx,
> - anchor,
> + anchor_abspath,
> target,
> callbacks, callback_baton,
> depth,
> @@ -2181,7 +2202,7 @@ svn_client_diff5(const apr_array_header_
> svn_client_ctx_t *ctx,
> apr_pool_t *pool)
> {
> - struct diff_cmd_baton diff_cmd_baton;
> + struct diff_cmd_baton diff_cmd_baton = { 0 };
> svn_wc_diff_callbacks4_t diff_callbacks;
>
> /* We will never do a pegged diff from here. */
> @@ -2195,7 +2216,7 @@ svn_client_diff5(const apr_array_header_
> diff_file_deleted_with_diff;
> diff_callbacks.dir_added = diff_dir_added;
> diff_callbacks.dir_deleted = diff_dir_deleted;
> - diff_callbacks.dir_props_changed = diff_props_changed;
> + diff_callbacks.dir_props_changed = diff_dir_props_changed;
> diff_callbacks.dir_opened = diff_dir_opened;
> diff_callbacks.dir_closed = diff_dir_closed;
>
> @@ -2219,6 +2240,7 @@ svn_client_diff5(const apr_array_header_
> diff_cmd_baton.visited_paths = apr_hash_make(pool);
> diff_cmd_baton.ra_session = NULL;
> diff_cmd_baton.wc_root_abspath = NULL;
> + diff_cmd_baton.anchor = NULL;
>
> return do_diff(&diff_callbacks, &diff_cmd_baton, ctx,
> path1, path2, revision1, revision2, &peg_revision,
> @@ -2246,7 +2268,7 @@ svn_client_diff_peg5(const apr_array_hea
> svn_client_ctx_t *ctx,
> apr_pool_t *pool)
> {
> - struct diff_cmd_baton diff_cmd_baton;
> + struct diff_cmd_baton diff_cmd_baton = { 0 };
> svn_wc_diff_callbacks4_t diff_callbacks;
>
> /* setup callback and baton */
> @@ -2256,7 +2278,7 @@ svn_client_diff_peg5(const apr_array_hea
> diff_file_deleted_with_diff;
> diff_callbacks.dir_added = diff_dir_added;
> diff_callbacks.dir_deleted = diff_dir_deleted;
> - diff_callbacks.dir_props_changed = diff_props_changed;
> + diff_callbacks.dir_props_changed = diff_dir_props_changed;
> diff_callbacks.dir_opened = diff_dir_opened;
> diff_callbacks.dir_closed = diff_dir_closed;
>
> @@ -2280,6 +2302,7 @@ svn_client_diff_peg5(const apr_array_hea
> diff_cmd_baton.visited_paths = apr_hash_make(pool);
> diff_cmd_baton.ra_session = NULL;
> diff_cmd_baton.wc_root_abspath = NULL;
> + diff_cmd_baton.anchor = NULL;
>
> return do_diff(&diff_callbacks, &diff_cmd_baton, ctx,
> path, path, start_revision, end_revision, peg_revision,
>
> Modified: subversion/trunk/subversion/libsvn_wc/deprecated.c
> URL:
http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_wc/deprecated.c?rev=1096264&r1=1096263&r2=1096264&view=diff
>
==============================================================================
> --- subversion/trunk/subversion/libsvn_wc/deprecated.c (original)
> +++ subversion/trunk/subversion/libsvn_wc/deprecated.c Sun Apr 24 01:44:57
2011
> @@ -1575,6 +1575,7 @@ struct diff_callbacks3_wrapper_baton {
> const svn_wc_diff_callbacks3_t *callbacks3;
> svn_wc__db_t *db;
> void *baton;
> + const char *anchor;
> };
>
> /* An svn_wc_diff_callbacks4_t function for wrapping
> @@ -1604,7 +1605,10 @@ wrap_4to3_file_changed(const char *local
> scratch_pool);
>
> return b->callbacks3->file_changed(adm_access, contentstate, propstate,
> - tree_conflicted, path, tmpfile1, tmpfile2,
> + tree_conflicted,
> + svn_dirent_join(b->anchor, path,
> + scratch_pool),
> + tmpfile1, tmpfile2,
> rev1, rev2, mimetype1, mimetype2,
> propchanges, originalprops, b->baton);
> }
> @@ -1638,7 +1642,10 @@ wrap_4to3_file_added(const char *local_d
> scratch_pool);
>
> return b->callbacks3->file_added(adm_access, contentstate, propstate,
> - tree_conflicted, path, tmpfile1, tmpfile2,
> + tree_conflicted,
> + svn_dirent_join(b->anchor, path,
> + scratch_pool),
> + tmpfile1, tmpfile2,
> rev1, rev2, mimetype1, mimetype2,
> propchanges, originalprops, b->baton);
> }
> @@ -1666,7 +1673,9 @@ wrap_4to3_file_deleted(const char *local
> scratch_pool);
>
> return b->callbacks3->file_deleted(adm_access, state, tree_conflicted,
> - path, tmpfile1, tmpfile2,
> + svn_dirent_join(b->anchor, path,
> + scratch_pool),
> + tmpfile1, tmpfile2,
> mimetype1, mimetype2, originalprops,
> b->baton);
> }
> @@ -1691,7 +1700,10 @@ wrap_4to3_dir_added(const char *local_di
> adm_access = svn_wc__adm_retrieve_internal2(b->db, local_dir_abspath,
> scratch_pool);
>
> - return b->callbacks3->dir_added(adm_access, state, tree_conflicted,
path, rev, b->baton);
> + return b->callbacks3->dir_added(adm_access, state, tree_conflicted,
> + svn_dirent_join(b->anchor, path,
> + scratch_pool),
> + rev, b->baton);
> }
>
> /* An svn_wc_diff_callbacks4_t function for wrapping
> @@ -1712,7 +1724,9 @@ wrap_4to3_dir_deleted(const char *local_
> scratch_pool);
>
> return b->callbacks3->dir_deleted(adm_access, state, tree_conflicted,
> - path, b->baton);
> + svn_dirent_join(b->anchor, path,
> + scratch_pool),
> + b->baton);
> }
>
> /* An svn_wc_diff_callbacks4_t function for wrapping
> @@ -1735,7 +1749,9 @@ wrap_4to3_dir_props_changed(const char *
> scratch_pool);
>
> return b->callbacks3->dir_props_changed(adm_access, propstate,
> - tree_conflicted, path,
> + tree_conflicted,
> + svn_dirent_join(b->anchor, path,
> + scratch_pool),
> propchanges, original_props,
> b->baton);
> }
> @@ -1760,8 +1776,10 @@ wrap_4to3_dir_opened(const char *local_d
> if (skip_children)
> *skip_children = FALSE;
>
> - return b->callbacks3->dir_opened(adm_access, tree_conflicted, path, rev,
> - b->baton);
> + return b->callbacks3->dir_opened(adm_access, tree_conflicted,
> + svn_dirent_join(b->anchor, path,
> + scratch_pool),
> + rev, b->baton);
> }
>
> /* An svn_wc_diff_callbacks4_t function for wrapping
> @@ -1783,7 +1801,10 @@ wrap_4to3_dir_closed(const char *local_d
> scratch_pool);
>
> return b->callbacks3->dir_closed(adm_access, contentstate, propstate,
> - tree_conflicted, path, b->baton);
> + tree_conflicted,
> + svn_dirent_join(b->anchor, path,
> + scratch_pool),
> + b->baton);
> }
>
>
> @@ -1824,11 +1845,12 @@ svn_wc_get_diff_editor5(svn_wc_adm_acces
> b->callbacks3 = callbacks;
> b->baton = callback_baton;
> b->db = db;
> + b->anchor = svn_wc_adm_access_path(anchor);
>
> SVN_ERR(svn_wc_get_diff_editor6(editor,
> edit_baton,
> wc_ctx,
> - svn_wc_adm_access_path(anchor),
> + svn_wc__adm_access_abspath(anchor),
> target,
> &diff_callbacks3_wrapper,
> b,
> @@ -1978,9 +2000,10 @@ svn_wc_diff5(svn_wc_adm_access_t *anchor
>
> b->callbacks3 = callbacks;
> b->baton = callback_baton;
> + b->anchor = svn_wc_adm_access_path(anchor);
>
> SVN_ERR(svn_wc_diff6(wc_ctx,
> - svn_dirent_join(svn_wc_adm_access_path(anchor),
> + svn_dirent_join(svn_wc__adm_access_abspath(anchor),
> target, pool),
> &diff_callbacks3_wrapper,
> b,
>
> Modified: subversion/trunk/subversion/libsvn_wc/diff.c
> URL:
http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_wc/diff.c?rev=1096264&r1=1096263&r2=1096264&view=diff
>
==============================================================================
> --- subversion/trunk/subversion/libsvn_wc/diff.c (original)
> +++ subversion/trunk/subversion/libsvn_wc/diff.c Sun Apr 24 01:44:57 2011
> @@ -199,7 +199,6 @@ struct edit_baton {
> svn_wc__db_t *db;
>
> /* ANCHOR/TARGET represent the base of the hierarchy to be compared. */
> - const char *anchor_path;
> const char *target;
>
> /* The absolute path of the anchor directory */
> @@ -274,7 +273,7 @@ struct dir_baton {
> itself).
>
> If the directory's properties have been compared, an item with hash
> - key of "" (an empty string) will be present in the hash. */
> + key of path will be present in the hash. */
> apr_hash_t *compared;
>
> /* The baton for the parent directory, or null if this is the root of the
> @@ -302,11 +301,8 @@ struct file_baton {
> const char *local_abspath;
>
> /* PATH is the "correct" path of the file, but it may not exist in the
> - working copy. WC_PATH is a path we can use to make temporary files
> - or open empty files; it doesn't necessarily exist either, but the
> - directory part of it does. */
> + working copy */
> const char *path;
> - const char *wc_path;
>
> /* When constructing the requested repository version of the file, we
> drop the result into a file at TEMP_FILE_PATH. */
> @@ -346,7 +342,7 @@ struct file_baton {
> static svn_error_t *
> make_edit_baton(struct edit_baton **edit_baton,
> svn_wc__db_t *db,
> - const char *anchor_path,
> + const char *anchor_abspath,
> const char *target,
> const svn_wc_diff_callbacks4_t *callbacks,
> void *callback_baton,
> @@ -364,13 +360,14 @@ make_edit_baton(struct edit_baton **edit
> apr_hash_t *changelist_hash = NULL;
> struct edit_baton *eb;
>
> + SVN_ERR_ASSERT(svn_dirent_is_absolute(anchor_abspath));
> +
> if (changelists && changelists->nelts)
> SVN_ERR(svn_hash_from_cstring_keys(&changelist_hash, changelists, pool));
>
> eb = apr_pcalloc(pool, sizeof(*eb));
> eb->db = db;
> - eb->anchor_path = anchor_path;
> - SVN_ERR(svn_dirent_get_absolute(&eb->anchor_abspath, eb->anchor_path,
pool));
> + eb->anchor_abspath = apr_pstrdup(pool, anchor_abspath);
> eb->target = apr_pstrdup(pool, target);
> eb->callbacks = callbacks;
> eb->callback_baton = callback_baton;
> @@ -452,29 +449,6 @@ make_file_baton(const char *path,
> fb->local_abspath = svn_dirent_join(parent_baton->local_abspath, fb->name,
> pool);
>
> - /* If the parent directory is added rather than replaced it does not
> - exist in the working copy. Determine a working copy path whose
> - directory part does exist; we can use that to create temporary
> - files. It doesn't matter whether the file part exists in the
> - directory. */
> - if (parent_baton->added)
> - {
> - struct dir_baton *wc_dir_baton = parent_baton;
> -
> - /* Ascend until a directory is not being added, this will be a
> - directory that does exist. This must terminate since the root of
> - the comparison cannot be added. */
> - while (wc_dir_baton->added)
> - wc_dir_baton = wc_dir_baton->parent_baton;
> -
> - fb->wc_path = svn_dirent_join(wc_dir_baton->path, "unimportant",
> - fb->pool);
> - }
> - else
> - {
> - fb->wc_path = fb->path;
> - }
> -
> return fb;
> }
>
> @@ -825,9 +799,7 @@ walk_local_nodes_diff(struct edit_baton
> to the target. When the target is a file, the anchor is the parent
> directory and if this is that directory the non-target entries must be
> skipped. */
> - in_anchor_not_target =
> - (*eb->target
> - && (! svn_path_compare_paths(path, eb->anchor_path)));
> + in_anchor_not_target = ((*path == '\0') && (*eb->target != '\0'));
>
> /* Check for local property mods on this directory, if we haven't
> already reported them and we aren't changelist-filted.
> @@ -837,7 +809,7 @@ walk_local_nodes_diff(struct edit_baton
> if (svn_wc__internal_changelist_match(db, local_abspath,
> eb->changelist_hash, scratch_pool)
> && (! in_anchor_not_target)
> - && (!compared || ! apr_hash_get(compared, "", 0)))
> + && (!compared || ! apr_hash_get(compared, path, 0)))
> {
> svn_boolean_t modified;
>
> @@ -888,7 +860,6 @@ walk_local_nodes_diff(struct edit_baton
> if (in_anchor_not_target && strcmp(eb->target, name))
> continue;
>
> -
> child_abspath = svn_dirent_join(local_abspath, name, iterpool);
>
> SVN_ERR(svn_wc__db_read_info(&status, &kind, NULL, NULL, NULL, NULL,
> @@ -903,7 +874,7 @@ walk_local_nodes_diff(struct edit_baton
> || status == svn_wc__db_status_absent)
> continue;
>
> - child_path = svn_dirent_join(path, name, iterpool);
> + child_path = svn_relpath_join(path, name, iterpool);
>
> /* Skip this node if it is in the list of nodes already diff'd. */
> if (compared && apr_hash_get(compared, child_path, APR_HASH_KEY_STRING))
> @@ -1145,7 +1116,7 @@ report_wc_directory_as_added(struct edit
> if (!eb->use_text_base && status == svn_wc__db_status_deleted)
> continue;
>
> - child_path = svn_dirent_join(path, name, iterpool);
> + child_path = svn_relpath_join(path, name, iterpool);
>
> switch (kind)
> {
> @@ -1205,7 +1176,7 @@ open_root(void *edit_baton,
> struct dir_baton *db;
>
> eb->root_opened = TRUE;
> - db = make_dir_baton(eb->anchor_path, NULL, eb, FALSE, eb->depth,
dir_pool);
> + db = make_dir_baton("", NULL, eb, FALSE, eb->depth, dir_pool);
> *root_baton = db;
>
> return SVN_NO_ERROR;
> @@ -1222,15 +1193,14 @@ delete_entry(const char *path,
> struct edit_baton *eb = pb->eb;
> svn_wc__db_t *db = eb->db;
> const char *empty_file;
> - const char *full_path = svn_dirent_join(eb->anchor_path, path,
> - pb->pool);
> - const char *name = svn_dirent_basename(path, pool);
> + const char *name = svn_dirent_basename(path, NULL);
> const char *local_abspath = svn_dirent_join(pb->local_abspath, name,
pool);
> svn_wc__db_status_t status;
> svn_wc__db_kind_t kind;
>
> /* Mark this node as compared in the parent directory's baton. */
> - apr_hash_set(pb->compared, full_path, APR_HASH_KEY_STRING, "");
> + apr_hash_set(pb->compared, apr_pstrdup(pb->pool, path),
> + APR_HASH_KEY_STRING, "");
>
> SVN_ERR(svn_wc__db_read_info(&status, &kind, NULL, NULL, NULL, NULL,
> NULL, NULL, NULL, NULL, NULL, NULL, NULL,
> @@ -1269,7 +1239,7 @@ delete_entry(const char *path,
> pool, pool));
> base_mimetype = get_prop_mimetype(baseprops);
>
> - SVN_ERR(eb->callbacks->file_deleted(NULL, NULL, NULL, full_path,
> + SVN_ERR(eb->callbacks->file_deleted(NULL, NULL, NULL, path,
> textbase,
> empty_file,
> base_mimetype,
> @@ -1281,7 +1251,7 @@ delete_entry(const char *path,
> else
> {
> /* Or normally, show the working file being added. */
> - SVN_ERR(report_wc_file_as_added(eb, local_abspath, full_path, pool));
> + SVN_ERR(report_wc_file_as_added(eb, local_abspath, path, pool));
> }
> break;
> case svn_wc__db_kind_dir:
> @@ -1289,7 +1259,7 @@ delete_entry(const char *path,
> revision, so diff should show this as an add. */
> SVN_ERR(report_wc_directory_as_added(eb,
> local_abspath,
> - full_path,
> + path,
> svn_depth_infinity,
> pool));
>
> @@ -1311,14 +1281,12 @@ add_directory(const char *path,
> {
> struct dir_baton *pb = parent_baton;
> struct dir_baton *db;
> - const char *full_path;
> svn_depth_t subdir_depth = (pb->depth == svn_depth_immediates)
> ? svn_depth_empty : pb->depth;
>
> /* ### TODO: support copyfrom? */
>
> - full_path = svn_dirent_join(pb->eb->anchor_path, path, dir_pool);
> - db = make_dir_baton(full_path, pb, pb->eb, TRUE, subdir_depth,
> + db = make_dir_baton(path, pb, pb->eb, TRUE, subdir_depth,
> dir_pool);
> *child_baton = db;
>
> @@ -1335,14 +1303,12 @@ open_directory(const char *path,
> {
> struct dir_baton *pb = parent_baton;
> struct dir_baton *db;
> - const char *full_path;
> svn_depth_t subdir_depth = (pb->depth == svn_depth_immediates)
> ? svn_depth_empty : pb->depth;
>
> /* Allocate path from the parent pool since the memory is used in the
> parent's compared hash */
> - full_path = svn_dirent_join(pb->eb->anchor_path, path, pb->pool);
> - db = make_dir_baton(full_path, pb, pb->eb, FALSE, subdir_depth,
dir_pool);
> + db = make_dir_baton(path, pb, pb->eb, FALSE, subdir_depth, dir_pool);
> *child_baton = db;
>
> return SVN_NO_ERROR;
> @@ -1415,7 +1381,7 @@ close_directory(void *dir_baton,
> /* Mark the properties of this directory as having already been
> compared so that we know not to show any local modifications
> later on. */
> - apr_hash_set(db->compared, "", 0, "");
> + apr_hash_set(db->compared, db->path, 0, "");
> }
>
> /* Report local modifications for this directory. Skip added
> @@ -1432,7 +1398,8 @@ close_directory(void *dir_baton,
> /* Mark this directory as compared in the parent directory's baton,
> unless this is the root of the comparison. */
> if (pb)
> - apr_hash_set(pb->compared, db->path, APR_HASH_KEY_STRING, "");
> + apr_hash_set(pb->compared, apr_pstrdup(pb->pool, db->path),
> + APR_HASH_KEY_STRING, "");
>
> return SVN_NO_ERROR;
> }
> @@ -1448,17 +1415,15 @@ add_file(const char *path,
> {
> struct dir_baton *pb = parent_baton;
> struct file_baton *fb;
> - const char *full_path;
>
> /* ### TODO: support copyfrom? */
>
> - full_path = svn_dirent_join(pb->eb->anchor_path, path, file_pool);
> - fb = make_file_baton(full_path, TRUE, pb, file_pool);
> + fb = make_file_baton(path, TRUE, pb, file_pool);
> *file_baton = fb;
>
> /* Add this filename to the parent directory's list of elements that
> have been compared. */
> - apr_hash_set(pb->compared, apr_pstrdup(pb->pool, full_path),
> + apr_hash_set(pb->compared, apr_pstrdup(pb->pool, path),
> APR_HASH_KEY_STRING, "");
>
> return SVN_NO_ERROR;
> @@ -1475,15 +1440,13 @@ open_file(const char *path,
> struct dir_baton *pb = parent_baton;
> struct edit_baton *eb = pb->eb;
> struct file_baton *fb;
> - const char *full_path;
>
> - full_path = svn_dirent_join(pb->eb->anchor_path, path, file_pool);
> - fb = make_file_baton(full_path, FALSE, pb, file_pool);
> + fb = make_file_baton(path, FALSE, pb, file_pool);
> *file_baton = fb;
>
> /* Add this filename to the parent directory's list of elements that
> have been compared. */
> - apr_hash_set(pb->compared, apr_pstrdup(pb->pool, full_path),
> + apr_hash_set(pb->compared, apr_pstrdup(pb->pool, path),
> APR_HASH_KEY_STRING, "");
>
> SVN_ERR(svn_wc__db_base_get_info(NULL, NULL, NULL, NULL, NULL, NULL, NULL,
> @@ -1871,7 +1834,7 @@ close_edit(void *edit_baton,
> {
> SVN_ERR(walk_local_nodes_diff(eb,
> eb->anchor_abspath,
> - eb->anchor_path,
> + "",
> eb->depth,
> NULL,
> eb->pool));
> @@ -1888,7 +1851,7 @@ svn_error_t *
> svn_wc_get_diff_editor6(const svn_delta_editor_t **editor,
> void **edit_baton,
> svn_wc_context_t *wc_ctx,
> - const char *anchor_path,
> + const char *anchor_abspath,
> const char *target,
> const svn_wc_diff_callbacks4_t *callbacks,
> void *callback_baton,
> @@ -1908,11 +1871,12 @@ svn_wc_get_diff_editor6(const svn_delta_
> void *inner_baton;
> svn_delta_editor_t *tree_editor;
> const svn_delta_editor_t *inner_editor;
> - const char *anchor_abspath;
> +
> + SVN_ERR_ASSERT(svn_dirent_is_absolute(anchor_abspath));
>
> SVN_ERR(make_edit_baton(&eb,
> wc_ctx->db,
> - anchor_path, target,
> + anchor_abspath, target,
> callbacks, callback_baton,
> depth, ignore_ancestry, show_copies_as_adds,
> use_git_diff_format,
> @@ -1939,8 +1903,6 @@ svn_wc_get_diff_editor6(const svn_delta_
> inner_editor = tree_editor;
> inner_baton = eb;
>
> - SVN_ERR(svn_dirent_get_absolute(&anchor_abspath, anchor_path,
result_pool));
> -
> if (depth == svn_depth_unknown)
> SVN_ERR(svn_wc__ambient_depth_filter_editor(&inner_editor,
> &inner_baton,
> @@ -1965,7 +1927,7 @@ svn_wc_get_diff_editor6(const svn_delta_
> /* Compare working copy against the text-base. */
> svn_error_t *
> svn_wc_diff6(svn_wc_context_t *wc_ctx,
> - const char *target_path,
> + const char *target_abspath,
> const svn_wc_diff_callbacks4_t *callbacks,
> void *callback_baton,
> svn_depth_t depth,
> @@ -1979,25 +1941,24 @@ svn_wc_diff6(svn_wc_context_t *wc_ctx,
> {
> struct edit_baton *eb;
> const char *target;
> - const char *target_abspath;
> - const char *anchor_path;
> + const char *anchor_abspath;
> svn_wc__db_kind_t kind;
>
> - SVN_ERR(svn_dirent_get_absolute(&target_abspath, target_path, pool));
> + SVN_ERR_ASSERT(svn_dirent_is_absolute(target_abspath));
> SVN_ERR(svn_wc__db_read_kind(&kind, wc_ctx->db, target_abspath, FALSE,
> pool));
>
> if (kind == svn_wc__db_kind_dir)
> {
> - anchor_path = target_path;
> + anchor_abspath = target_abspath;
> target = "";
> }
> else
> - svn_dirent_split(&anchor_path, &target, target_path, pool);
> + svn_dirent_split(&anchor_abspath, &target, target_abspath, pool);
>
> SVN_ERR(make_edit_baton(&eb,
> wc_ctx->db,
> - anchor_path,
> + anchor_abspath,
> target,
> callbacks, callback_baton,
> depth, ignore_ancestry, show_copies_as_adds,
> @@ -2008,7 +1969,7 @@ svn_wc_diff6(svn_wc_context_t *wc_ctx,
>
> SVN_ERR(walk_local_nodes_diff(eb,
> eb->anchor_abspath,
> - eb->anchor_path,
> + "",
> depth,
> NULL,
> eb->pool));
>
>

Re: svn commit: r1096264 - in /subversion/trunk/subversion: include/svn_wc.h libsvn_client/diff.c libsvn_wc/deprecated.c libsvn_wc/diff.c

[Greg Stein <gs...@gmail.com>]

Oh, I fully understand when things break on alternate setups or
platforms. If it passes on "ra_local" vs "ra_*" ... then yeah. Totally
normal.

It is just that the bots are showing breakage so, so often lately. I
just got a bit frustrated. Sorry.

On Sun, Apr 24, 2011 at 16:02, Bert Huijben <be...@qqmail.nl> wrote:
>
>
>> -----Original Message-----
>> From: Greg Stein [mailto:gstein@gmail.com]
>> Sent: zondag 24 april 2011 14:27
>> To: dev@subversion.apache.org
>> Subject: Re: svn commit: r1096264 - in /subversion/trunk/subversion:
>> include/svn_wc.h libsvn_client/diff.c libsvn_wc/deprecated.c
>> libsvn_wc/diff.c
>>
>> This broke a ton of diff tests. Did you even run them before committing
>> this?
>>
>
>> Seriously, I'm really tired of all the buildbot breakage.
>
> Well... Buy me a dozen machines and some rackspace  and maybe I can test
> every commit on every configuration before I commit. (And maybe we can
> release Subversion 1.7 in 2016 if everybody needs its own rack of
> buildservers)
>
> I ran the test suite 3 times over ra local before committing this patch and
> without a real problem.
>
> Just now I found the memory corruption that was specific to how ra_neon and
> ra_serf don't keep their paths valid while the baton is.
>
>
> The buildbots are there to help us find these problems. If we should never
> break builds we really don't need them.
>
>        Bert
>
>

RE: svn commit: r1096264 - in /subversion/trunk/subversion: include/svn_wc.h libsvn_client/diff.c libsvn_wc/deprecated.c libsvn_wc/diff.c

[Bert Huijben <be...@qqmail.nl>]

> -----Original Message-----
> From: Greg Stein [mailto:gstein@gmail.com]
> Sent: zondag 24 april 2011 14:27
> To: dev@subversion.apache.org
> Subject: Re: svn commit: r1096264 - in /subversion/trunk/subversion:
> include/svn_wc.h libsvn_client/diff.c libsvn_wc/deprecated.c
> libsvn_wc/diff.c
> 
> This broke a ton of diff tests. Did you even run them before committing
> this?
> 

> Seriously, I'm really tired of all the buildbot breakage.

Well... Buy me a dozen machines and some rackspace  and maybe I can test
every commit on every configuration before I commit. (And maybe we can
release Subversion 1.7 in 2016 if everybody needs its own rack of
buildservers)

I ran the test suite 3 times over ra local before committing this patch and
without a real problem.

Just now I found the memory corruption that was specific to how ra_neon and
ra_serf don't keep their paths valid while the baton is.


The buildbots are there to help us find these problems. If we should never
break builds we really don't need them.

	Bert