Fix issue #2064. Merge prop changes before content changes so keyword expansion, special file handling etc. work. * subversion/libsvn_client/repos_diff.c (edit_baton): Use svn_wc_diff_callbacks2_t instead of svn_wc_diff_callbacks_t. Add empty_hash member. (delete_entry): Add properties to file_deleted call. (add_file): Set pristine_props to empty hash. (close_file): INclude properties in callback calls and call them for both file and prop changes. (close_directory): props_changed -> dir_props_changed. Update comment. (svn_client__get_diff_editor): Use new callbacks vtable. INitialize eb->empty_hash. * subversion/libsvn_client/client.h (svn_client__get_diff_editor): Use new callbacks vtable. * subversion/libsvn_client/diff.c (diff_props_changed, merge_props_changed): Move above using functions. (diff_content_changed): New function, extracted from diff_file_changed. (diff_file_changed): Implemented in terms of diff_contents_changed and diff_props_changed. Adjust to new callbacks API. (diff_file_added, diff_file_deleted_with_diff, diff_file_deleted_no_diff): Adjust to new callback API. (merge_file_changed): Adjust to new callbacks API. Merge prop changes as well as content changes. (merge_file_added): Adjust to new callbacks API. Provide new properties when adding or modifying the file. (merge_file_deleted): Adjust to new callbacks API. (merge_callbacks): Change type to svn_wc_diff_callbacks2_t. (do_merge): Take svn_wc_diff_callbacks2_t. (do_single_file_merge): Don't call merge_props_changed, but instead add propchanges to merge_file_changed call. (diff_wc_wc): Take svn_wc_diff_callbacks2_t. Use svn_wc_diff3 instead of diff2. (diff_repos_repos): Take svn_wc_diff_callbacks2_t. (diff_repos_wc): Take svn_wc_diff_callbacks2_t. Use svn_wc_get_diff_editor3. (do_diff, do_diff_peg): Take svn_wc_diff_callbacks2_t. (svn_client_diff, svn_client_diff_peg): Use svn_wc_diff_callbacks2_t. * subversion/tests/clients/cmdline/merge_tests.py (merge_keyword_expansions): New test. (test_list): Add new test. Index: subversion/libsvn_client/repos_diff.c =================================================================== --- subversion/libsvn_client/repos_diff.c (revision 12006) +++ subversion/libsvn_client/repos_diff.c (arbetskopia) @@ -46,7 +46,7 @@ /* The callback and calback argument that implement the file comparison function */ - const svn_wc_diff_callbacks_t *diff_callbacks; + const svn_wc_diff_callbacks2_t *diff_callbacks; void *diff_cmd_baton; /* RECURSE is TRUE if this is a recursive diff or merge, false otherwise */ @@ -71,6 +71,9 @@ cached here so that it can be reused, all empty files are the same. */ const char *empty_file; + /* Empty hash used for adds. */ + apr_hash_t *empty_hash; + /* If the func is non-null, send notifications of actions. */ svn_wc_notify_func_t notify_func; void *notify_baton; @@ -535,6 +538,7 @@ b->path_start_revision, b->path_end_revision, mimetype1, mimetype2, + b->pristine_props, b->edit_baton->diff_cmd_baton)); break; @@ -654,6 +658,7 @@ *file_baton = b; SVN_ERR (get_empty_file (b->edit_baton, &(b->path_start_revision))); + b->pristine_props = pb->edit_baton->empty_hash; return SVN_NO_ERROR; } @@ -776,47 +781,36 @@ else if (err) return err; - if (b->path_end_revision) + if (b->path_end_revision || b->propchanges->nelts > 0) { const char *mimetype1, *mimetype2; get_file_mime_types (&mimetype1, &mimetype2, b); if (b->added) SVN_ERR (eb->diff_callbacks->file_added - (adm_access, &content_state, + (adm_access, &content_state, &prop_state, b->wcpath, - b->path_start_revision, + b->path_end_revision ? b->path_start_revision : NULL, b->path_end_revision, 0, b->edit_baton->target_revision, mimetype1, mimetype2, + b->propchanges, b->pristine_props, b->edit_baton->diff_cmd_baton)); else SVN_ERR (eb->diff_callbacks->file_changed - (adm_access, &content_state, + (adm_access, &content_state, &prop_state, b->wcpath, - b->path_start_revision, + b->path_end_revision ? b->path_start_revision : NULL, b->path_end_revision, b->edit_baton->revision, b->edit_baton->target_revision, mimetype1, mimetype2, + b->propchanges, b->pristine_props, b->edit_baton->diff_cmd_baton)); } - /* Don't do the props_changed stuff if this is a dry_run and we don't - have an access baton, since in that case the file will already have - been recognised as added, in which case they cannot conflict. A - similar argument applies to directories in close_directory. */ - if (b->propchanges->nelts > 0 && (! (eb->dry_run && b->added))) - { - SVN_ERR (eb->diff_callbacks->props_changed - (adm_access, &prop_state, - b->wcpath, - b->propchanges, b->pristine_props, - b->edit_baton->diff_cmd_baton)); - } - /* ### Is b->path the repos path? Probably. This doesn't really matter while issue #748 (svn merge only happens in ".") is outstanding. But when we take a wc_path as an argument to @@ -877,10 +871,11 @@ else if (err) return err; - /* As for close_file, whether we do this depends on whether it's a - dry-run */ + /* Don't do the props_changed stuff if this is a dry_run and we don't + have an access baton, since in that case the directory will already + have been recognised as added, in which case they cannot conflict. */ if (! eb->dry_run || adm_access) - SVN_ERR (eb->diff_callbacks->props_changed + SVN_ERR (eb->diff_callbacks->dir_props_changed (adm_access, &prop_state, b->wcpath, b->propchanges, b->pristine_props, @@ -952,7 +947,7 @@ svn_error_t * svn_client__get_diff_editor (const char *target, svn_wc_adm_access_t *adm_access, - const svn_wc_diff_callbacks_t *diff_callbacks, + const svn_wc_diff_callbacks2_t *diff_callbacks, void *diff_cmd_baton, svn_boolean_t recurse, svn_boolean_t dry_run, @@ -981,6 +976,7 @@ eb->ra_session = ra_session; eb->revision = revision; eb->empty_file = NULL; + eb->empty_hash = apr_hash_make (subpool); eb->pool = subpool; eb->notify_func = notify_func; eb->notify_baton = notify_baton; Index: subversion/libsvn_client/client.h =================================================================== --- subversion/libsvn_client/client.h (revision 12006) +++ subversion/libsvn_client/client.h (arbetskopia) @@ -382,7 +382,7 @@ svn_error_t * svn_client__get_diff_editor (const char *target, svn_wc_adm_access_t *adm_access, - const svn_wc_diff_callbacks_t *diff_cmd, + const svn_wc_diff_callbacks2_t *diff_cmd, void *diff_cmd_baton, svn_boolean_t recurse, svn_boolean_t dry_run, Index: subversion/libsvn_client/diff.c =================================================================== --- subversion/libsvn_client/diff.c (revision 12006) +++ subversion/libsvn_client/diff.c (arbetskopia) @@ -237,7 +237,7 @@ /* These are the numeric representations of the revisions passed to svn_client_diff, either may be SVN_INVALID_REVNUM. We need these - because some of the svn_wc_diff_callbacks_t don't get revision + because some of the svn_wc_diff_callbacks2_t don't get revision arguments. ### Perhaps we should change the callback signatures and eliminate @@ -274,20 +274,43 @@ return label; } +static svn_error_t * +diff_props_changed (svn_wc_adm_access_t *adm_access, + svn_wc_notify_state_t *state, + const char *path, + const apr_array_header_t *propchanges, + apr_hash_t *original_props, + void *diff_baton) +{ + struct diff_cmd_baton *diff_cmd_baton = diff_baton; + apr_array_header_t *props; + apr_pool_t *subpool = svn_pool_create (diff_cmd_baton->pool); + + SVN_ERR (svn_categorize_props (propchanges, NULL, NULL, &props, subpool)); + + if (props->nelts > 0) + SVN_ERR (display_prop_diffs (props, original_props, path, + diff_cmd_baton->outfile, subpool)); + + if (state) + *state = svn_wc_notify_state_unknown; + + svn_pool_destroy (subpool); + return SVN_NO_ERROR; +} + /* The main workhorse, which invokes an external 'diff' program on the two temporary files. The path is the "true" label to use in the diff output. */ static svn_error_t * -diff_file_changed (svn_wc_adm_access_t *adm_access, - svn_wc_notify_state_t *state, - const char *path, - const char *tmpfile1, - const char *tmpfile2, - svn_revnum_t rev1, - svn_revnum_t rev2, - const char *mimetype1, - const char *mimetype2, - void *diff_baton) +diff_content_changed (const char *path, + const char *tmpfile1, + const char *tmpfile2, + svn_revnum_t rev1, + svn_revnum_t rev2, + const char *mimetype1, + const char *mimetype2, + void *diff_baton) { struct diff_cmd_baton *diff_cmd_baton = diff_baton; const char *diff_cmd = NULL; @@ -411,8 +434,6 @@ } /* Exit early. */ - if (state) - *state = svn_wc_notify_state_unknown; svn_pool_destroy (subpool); return SVN_NO_ERROR; } @@ -485,21 +506,47 @@ to need to write a diff plug-in mechanism that makes use of the two paths, instead of just blindly running SVN_CLIENT_DIFF. */ - if (state) - *state = svn_wc_notify_state_unknown; - /* Destroy the subpool. */ svn_pool_destroy (subpool); return SVN_NO_ERROR; } +static svn_error_t * +diff_file_changed (svn_wc_adm_access_t *adm_access, + svn_wc_notify_state_t *content_state, + svn_wc_notify_state_t *prop_state, + const char *path, + const char *tmpfile1, + const char *tmpfile2, + svn_revnum_t rev1, + svn_revnum_t rev2, + const char *mimetype1, + const char *mimetype2, + const apr_array_header_t *prop_changes, + apr_hash_t *original_props, + void *diff_baton) +{ + if (tmpfile1) + SVN_ERR (diff_content_changed (path, + tmpfile1, tmpfile2, rev1, rev2, + mimetype1, mimetype2, diff_baton)); + if (prop_changes->nelts > 0) + SVN_ERR (diff_props_changed (adm_access, prop_state, path, prop_changes, + original_props, diff_baton)); + if (content_state) + *content_state = svn_wc_notify_state_unknown; + if (prop_state) + *prop_state = svn_wc_notify_state_unknown; + return SVN_NO_ERROR; +} /* The because the repos-diff editor passes at least one empty file to each of these next two functions, they can be dumb wrappers around the main workhorse routine. */ static svn_error_t * diff_file_added (svn_wc_adm_access_t *adm_access, - svn_wc_notify_state_t *state, + svn_wc_notify_state_t *content_state, + svn_wc_notify_state_t *prop_state, const char *path, const char *tmpfile1, const char *tmpfile2, @@ -507,6 +554,8 @@ svn_revnum_t rev2, const char *mimetype1, const char *mimetype2, + const apr_array_header_t *prop_changes, + apr_hash_t *original_props, void *diff_baton) { struct diff_cmd_baton *diff_cmd_baton = diff_baton; @@ -518,9 +567,11 @@ user see that *something* happened. */ diff_cmd_baton->force_diff_output = TRUE; - SVN_ERR (diff_file_changed (adm_access, state, path, tmpfile1, tmpfile2, + SVN_ERR (diff_file_changed (adm_access, content_state, prop_state, path, + tmpfile1, tmpfile2, rev1, rev2, - mimetype1, mimetype2, diff_baton)); + mimetype1, mimetype2, + prop_changes, original_props, diff_baton)); diff_cmd_baton->force_diff_output = FALSE; @@ -535,13 +586,19 @@ const char *tmpfile2, const char *mimetype1, const char *mimetype2, + apr_hash_t *original_props, void *diff_baton) { struct diff_cmd_baton *diff_cmd_baton = diff_baton; - return diff_file_changed (adm_access, state, path, tmpfile1, tmpfile2, + /* We don't list all the deleted properties. */ + return diff_file_changed (adm_access, state, NULL, path, + tmpfile1, tmpfile2, diff_cmd_baton->revnum1, diff_cmd_baton->revnum2, - mimetype1, mimetype2, diff_baton); + mimetype1, mimetype2, + apr_array_make (diff_cmd_baton->pool, 1, + sizeof(svn_prop_t)), + apr_hash_make (diff_cmd_baton->pool), diff_baton); } static svn_error_t * @@ -552,6 +609,7 @@ const char *tmpfile2, const char *mimetype1, const char *mimetype2, + apr_hash_t *original_props, void *diff_baton) { struct diff_cmd_baton *diff_cmd_baton = diff_baton; @@ -596,32 +654,7 @@ return SVN_NO_ERROR; } -static svn_error_t * -diff_props_changed (svn_wc_adm_access_t *adm_access, - svn_wc_notify_state_t *state, - const char *path, - const apr_array_header_t *propchanges, - apr_hash_t *original_props, - void *diff_baton) -{ - struct diff_cmd_baton *diff_cmd_baton = diff_baton; - apr_array_header_t *props; - apr_pool_t *subpool = svn_pool_create (diff_cmd_baton->pool); - SVN_ERR (svn_categorize_props (propchanges, NULL, NULL, &props, subpool)); - - if (props->nelts > 0) - SVN_ERR (display_prop_diffs (props, original_props, path, - diff_cmd_baton->outfile, subpool)); - - if (state) - *state = svn_wc_notify_state_unknown; - - svn_pool_destroy (subpool); - return SVN_NO_ERROR; -} - - /*-----------------------------------------------------------------*/ /*** Callbacks for 'svn merge', invoked by the repos-diff editor. ***/ @@ -648,10 +681,49 @@ apr_pool_t *pool; }; +static svn_error_t * +merge_props_changed (svn_wc_adm_access_t *adm_access, + svn_wc_notify_state_t *state, + const char *path, + const apr_array_header_t *propchanges, + apr_hash_t *original_props, + void *baton) +{ + apr_array_header_t *props; + struct merge_cmd_baton *merge_b = baton; + apr_pool_t *subpool = svn_pool_create (merge_b->pool); + svn_error_t *err; + SVN_ERR (svn_categorize_props (propchanges, NULL, NULL, &props, subpool)); + + /* We only want to merge "regular" version properties: by + definition, 'svn merge' shouldn't touch any data within .svn/ */ + if (props->nelts) + { + err = svn_wc_merge_prop_diffs (state, path, adm_access, props, + FALSE, merge_b->dry_run, subpool); + if (err && (err->apr_err == SVN_ERR_ENTRY_NOT_FOUND + || err->apr_err == SVN_ERR_UNVERSIONED_RESOURCE)) + { + /* if the entry doesn't exist in the wc, just 'skip' over + this part of the tree-delta. */ + if (state) + *state = svn_wc_notify_state_missing; + svn_error_clear (err); + return SVN_NO_ERROR; + } + else if (err) + return err; + } + + svn_pool_destroy (subpool); + return SVN_NO_ERROR; +} + static svn_error_t * merge_file_changed (svn_wc_adm_access_t *adm_access, - svn_wc_notify_state_t *state, + svn_wc_notify_state_t *content_state, + svn_wc_notify_state_t *prop_state, const char *mine, const char *older, const char *yours, @@ -659,6 +731,8 @@ svn_revnum_t yours_rev, const char *mimetype1, const char *mimetype2, + const apr_array_header_t *prop_changes, + apr_hash_t *original_props, void *baton) { struct merge_cmd_baton *merge_b = baton; @@ -679,8 +753,10 @@ /* Easy out: no access baton means there ain't no merge target */ if (adm_access == NULL) { - if (state) - *state = svn_wc_notify_state_missing; + if (content_state) + *content_state = svn_wc_notify_state_missing; + if (prop_state) + *prop_state = svn_wc_notify_state_missing; return SVN_NO_ERROR; } @@ -700,8 +776,10 @@ if ((! entry) || (kind != svn_node_file)) { - if (state) - *state = svn_wc_notify_state_missing; + if (content_state) + *content_state = svn_wc_notify_state_missing; + if (prop_state) + *prop_state = svn_wc_notify_state_missing; return SVN_NO_ERROR; } } @@ -711,58 +789,70 @@ diff-editor-mechanisms are doing the hard work of getting the fulltexts! */ - SVN_ERR (svn_wc_text_modified_p (&has_local_mods, mine, FALSE, - adm_access, subpool)); + /* Do property merge before text merge so that keyword expansion takes + into account the new property values. */ + if (prop_changes->nelts > 0) + SVN_ERR (merge_props_changed (adm_access, prop_state, mine, prop_changes, + original_props, baton)); + else + if (prop_state) + *prop_state = svn_wc_notify_state_unchanged; - /* Special case: if a binary file isn't locally modified, and is - exactly identical to the 'left' side of the merge, then don't - allow svn_wc_merge to produce a conflict. Instead, just - overwrite the working file with the 'right' side of the merge. */ - if ((! has_local_mods) - && ((mimetype1 && svn_mime_type_is_binary (mimetype1)) - || (mimetype2 && svn_mime_type_is_binary (mimetype2)))) + if (older) { - svn_boolean_t same_contents; - /* ### someday, we should just be able to compare - identity-strings here. */ - SVN_ERR (svn_io_files_contents_same_p (&same_contents, - older, mine, subpool)); - if (same_contents) + SVN_ERR (svn_wc_text_modified_p (&has_local_mods, mine, FALSE, + adm_access, subpool)); + + /* Special case: if a binary file isn't locally modified, and is + exactly identical to the 'left' side of the merge, then don't + allow svn_wc_merge to produce a conflict. Instead, just + overwrite the working file with the 'right' side of the merge. */ + if ((! has_local_mods) + && ((mimetype1 && svn_mime_type_is_binary (mimetype1)) + || (mimetype2 && svn_mime_type_is_binary (mimetype2)))) { - if (! merge_b->dry_run) - SVN_ERR (svn_io_file_rename (yours, mine, subpool)); - merge_outcome = svn_wc_merge_merged; - merge_required = FALSE; + svn_boolean_t same_contents; + /* ### someday, we should just be able to compare + identity-strings here. */ + SVN_ERR (svn_io_files_contents_same_p (&same_contents, + older, mine, subpool)); + if (same_contents) + { + if (! merge_b->dry_run) + SVN_ERR (svn_io_file_rename (yours, mine, subpool)); + merge_outcome = svn_wc_merge_merged; + merge_required = FALSE; + } + } + + if (merge_required) + { + SVN_ERR (svn_wc_merge (older, yours, mine, adm_access, + left_label, right_label, target_label, + merge_b->dry_run, &merge_outcome, + merge_b->diff3_cmd, subpool)); } - } - if (merge_required) - { - SVN_ERR (svn_wc_merge (older, yours, mine, adm_access, - left_label, right_label, target_label, - merge_b->dry_run, &merge_outcome, - merge_b->diff3_cmd, subpool)); - } + /* Philip asks "Why?" Why does the notification depend on whether the + file had modifications before the merge? If the merge didn't change + the file because the local mods already included the change why does + that result it "merged" notification? That's information available + through the status command, while the fact that the merge didn't + change the file is lost :-( */ - /* Philip asks "Why?" Why does the notification depend on whether the - file had modifications before the merge? If the merge didn't change - the file because the local mods already included the change why does - that result it "merged" notification? That's information available - through the status command, while the fact that the merge didn't - change the file is lost :-( */ - - if (state) - { - if (merge_outcome == svn_wc_merge_conflict) - *state = svn_wc_notify_state_conflicted; - else if (has_local_mods) - *state = svn_wc_notify_state_merged; - else if (merge_outcome == svn_wc_merge_merged) - *state = svn_wc_notify_state_changed; - else if (merge_outcome == svn_wc_merge_no_merge) - *state = svn_wc_notify_state_missing; - else /* merge_outcome == svn_wc_merge_unchanged */ - *state = svn_wc_notify_state_unchanged; + if (content_state) + { + if (merge_outcome == svn_wc_merge_conflict) + *content_state = svn_wc_notify_state_conflicted; + else if (has_local_mods) + *content_state = svn_wc_notify_state_merged; + else if (merge_outcome == svn_wc_merge_merged) + *content_state = svn_wc_notify_state_changed; + else if (merge_outcome == svn_wc_merge_no_merge) + *content_state = svn_wc_notify_state_missing; + else /* merge_outcome == svn_wc_merge_unchanged */ + *content_state = svn_wc_notify_state_unchanged; + } } svn_pool_destroy (subpool); @@ -772,7 +862,8 @@ static svn_error_t * merge_file_added (svn_wc_adm_access_t *adm_access, - svn_wc_notify_state_t *state, + svn_wc_notify_state_t *content_state, + svn_wc_notify_state_t *prop_state, const char *mine, const char *older, const char *yours, @@ -780,6 +871,8 @@ svn_revnum_t rev2, const char *mimetype1, const char *mimetype2, + const apr_array_header_t *prop_changes, + apr_hash_t *original_props, void *baton) { struct merge_cmd_baton *merge_b = baton; @@ -787,21 +880,39 @@ svn_node_kind_t kind; const char *copyfrom_url; const char *child; + int i; + apr_hash_t *new_props; + /* In most cases, we just leave prop_state as unknown, and let the + content_state what happened, so we set prop_state here to avoid that + below. */ + if (prop_state) + *prop_state = svn_wc_notify_state_unknown; + + /* Apply the prop changes to a new hash table. */ + new_props = apr_hash_copy (subpool, original_props); + for (i = 0; i < prop_changes->nelts; ++i) + { + const svn_prop_t *prop = &APR_ARRAY_IDX (prop_changes, i, svn_prop_t); + apr_hash_set (new_props, prop->name, APR_HASH_KEY_STRING, prop->value); + } + /* Easy out: if we have no adm_access for the parent directory, then this portion of the tree-delta "patch" must be inapplicable. Send a 'missing' state back; the repos-diff editor should then send a 'skip' notification. */ if (! adm_access) { - if (state) + if (merge_b->dry_run && merge_b->added_path + && svn_path_is_child (merge_b->added_path, mine, subpool)) { - if (merge_b->dry_run && merge_b->added_path - && svn_path_is_child (merge_b->added_path, mine, subpool)) - *state = svn_wc_notify_state_changed; - else - *state = svn_wc_notify_state_missing; + if (content_state) + *content_state = svn_wc_notify_state_changed; + if (prop_state && apr_hash_count (new_props)) + *prop_state = svn_wc_notify_state_changed; } + else + *content_state = svn_wc_notify_state_missing; return SVN_NO_ERROR; } @@ -815,8 +926,8 @@ if (entry && entry->schedule != svn_wc_schedule_delete) { /* It's versioned but missing. */ - if (state) - *state = svn_wc_notify_state_obstructed; + if (content_state) + *content_state = svn_wc_notify_state_obstructed; return SVN_NO_ERROR; } if (! merge_b->dry_run) @@ -836,20 +947,22 @@ SVN_ERR (svn_wc_add_repos_file (mine, adm_access, yours, - apr_hash_make (merge_b->pool), + new_props, copyfrom_url, rev2, merge_b->pool)); } - if (state) - *state = svn_wc_notify_state_changed; + if (content_state) + *content_state = svn_wc_notify_state_changed; + if (prop_state && apr_hash_count (new_props)) + *prop_state = svn_wc_notify_state_changed; } break; case svn_node_dir: { /* this will make the repos_editor send a 'skipped' message */ - if (state) - *state = svn_wc_notify_state_obstructed; + if (content_state) + *content_state = svn_wc_notify_state_obstructed; } case svn_node_file: { @@ -863,22 +976,23 @@ if (!entry || entry->schedule == svn_wc_schedule_delete) { /* this will make the repos_editor send a 'skipped' message */ - if (state) - *state = svn_wc_notify_state_obstructed; + if (content_state) + *content_state = svn_wc_notify_state_obstructed; } else { - SVN_ERR (merge_file_changed (adm_access, state, + SVN_ERR (merge_file_changed (adm_access, content_state, prop_state, mine, older, yours, rev1, rev2, mimetype1, mimetype2, + prop_changes, original_props, baton)); } break; } default: - if (state) - *state = svn_wc_notify_state_unknown; + if (content_state) + *content_state = svn_wc_notify_state_unknown; break; } @@ -894,6 +1008,7 @@ const char *yours, const char *mimetype1, const char *mimetype2, + apr_hash_t *original_props, void *baton) { struct merge_cmd_baton *merge_b = baton; @@ -1127,47 +1242,8 @@ return SVN_NO_ERROR; } -static svn_error_t * -merge_props_changed (svn_wc_adm_access_t *adm_access, - svn_wc_notify_state_t *state, - const char *path, - const apr_array_header_t *propchanges, - apr_hash_t *original_props, - void *baton) -{ - apr_array_header_t *props; - struct merge_cmd_baton *merge_b = baton; - apr_pool_t *subpool = svn_pool_create (merge_b->pool); - svn_error_t *err; - - SVN_ERR (svn_categorize_props (propchanges, NULL, NULL, &props, subpool)); - - /* We only want to merge "regular" version properties: by - definition, 'svn merge' shouldn't touch any data within .svn/ */ - if (props->nelts) - { - err = svn_wc_merge_prop_diffs (state, path, adm_access, props, - FALSE, merge_b->dry_run, subpool); - if (err && (err->apr_err == SVN_ERR_ENTRY_NOT_FOUND - || err->apr_err == SVN_ERR_UNVERSIONED_RESOURCE)) - { - /* if the entry doesn't exist in the wc, just 'skip' over - this part of the tree-delta. */ - if (state) - *state = svn_wc_notify_state_missing; - svn_error_clear (err); - return SVN_NO_ERROR; - } - else if (err) - return err; - } - - svn_pool_destroy (subpool); - return SVN_NO_ERROR; -} - /* The main callback table for 'svn merge'. */ -static const svn_wc_diff_callbacks_t +static const svn_wc_diff_callbacks2_t merge_callbacks = { merge_file_changed, @@ -1269,7 +1345,7 @@ svn_boolean_t recurse, svn_boolean_t ignore_ancestry, svn_boolean_t dry_run, - const svn_wc_diff_callbacks_t *callbacks, + const svn_wc_diff_callbacks2_t *callbacks, void *callback_baton, svn_client_ctx_t *ctx, apr_pool_t *pool) @@ -1498,14 +1574,18 @@ pval = apr_hash_get (props2, SVN_PROP_MIME_TYPE, strlen(SVN_PROP_MIME_TYPE)); mimetype2 = pval ? pval->data : NULL; + /* Deduce property diffs. */ + SVN_ERR (svn_prop_diffs (&propchanges, props2, props1, pool)); + SVN_ERR (merge_file_changed (adm_access, - &text_state, + &text_state, &prop_state, merge_b->target, tmpfile1, tmpfile2, rev1, rev2, mimetype1, mimetype2, + propchanges, props1, merge_b)); /* Ignore if temporary file not found. It may have been renamed. */ @@ -1515,19 +1595,9 @@ svn_error_clear (err); err = svn_io_remove_file (tmpfile2, pool); if (err && ! APR_STATUS_IS_ENOENT (err->apr_err)) - return err; + return err; svn_error_clear (err); - /* Deduce property diffs, and merge those too. */ - SVN_ERR (svn_prop_diffs (&propchanges, props2, props1, pool)); - - SVN_ERR (merge_props_changed (adm_access, - &prop_state, - merge_b->target, - propchanges, - NULL, - merge_b)); - if (merge_b->ctx->notify_func) { (*merge_b->ctx->notify_func) (merge_b->ctx->notify_baton, @@ -1599,7 +1669,7 @@ const svn_opt_revision_t *revision2, svn_boolean_t recurse, svn_boolean_t ignore_ancestry, - const svn_wc_diff_callbacks_t *callbacks, + const svn_wc_diff_callbacks2_t *callbacks, struct diff_cmd_baton *callback_baton, svn_client_ctx_t *ctx, apr_pool_t *pool) @@ -1630,7 +1700,7 @@ (&callback_baton->revnum1, NULL, NULL, revision1, path1, pool)); callback_baton->revnum2 = SVN_INVALID_REVNUM; /* WC */ - SVN_ERR (svn_wc_diff2 (adm_access, target, callbacks, callback_baton, + SVN_ERR (svn_wc_diff3 (adm_access, target, callbacks, callback_baton, recurse, ignore_ancestry, pool)); SVN_ERR (svn_wc_adm_close (adm_access)); return SVN_NO_ERROR; @@ -1655,7 +1725,7 @@ const svn_opt_revision_t *peg_revision, svn_boolean_t recurse, svn_boolean_t ignore_ancestry, - const svn_wc_diff_callbacks_t *callbacks, + const svn_wc_diff_callbacks2_t *callbacks, struct diff_cmd_baton *callback_baton, svn_client_ctx_t *ctx, apr_pool_t *pool) @@ -1822,7 +1892,7 @@ svn_boolean_t reverse, svn_boolean_t recurse, svn_boolean_t ignore_ancestry, - const svn_wc_diff_callbacks_t *callbacks, + const svn_wc_diff_callbacks2_t *callbacks, struct diff_cmd_baton *callback_baton, svn_client_ctx_t *ctx, apr_pool_t *pool) @@ -1889,7 +1959,7 @@ NULL, NULL, NULL, FALSE, TRUE, ctx, pool)); - SVN_ERR (svn_wc_get_diff_editor2 (adm_access, target, + SVN_ERR (svn_wc_get_diff_editor3 (adm_access, target, callbacks, callback_baton, recurse, ignore_ancestry, @@ -1935,7 +2005,7 @@ const svn_opt_revision_t *revision2, svn_boolean_t recurse, svn_boolean_t ignore_ancestry, - const svn_wc_diff_callbacks_t *callbacks, + const svn_wc_diff_callbacks2_t *callbacks, struct diff_cmd_baton *callback_baton, svn_client_ctx_t *ctx, apr_pool_t *pool) @@ -2020,7 +2090,7 @@ const svn_opt_revision_t *revision2, svn_boolean_t recurse, svn_boolean_t ignore_ancestry, - const svn_wc_diff_callbacks_t *callbacks, + const svn_wc_diff_callbacks2_t *callbacks, struct diff_cmd_baton *callback_baton, svn_client_ctx_t *ctx, apr_pool_t *pool) @@ -2147,7 +2217,7 @@ apr_pool_t *pool) { struct diff_cmd_baton diff_cmd_baton; - svn_wc_diff_callbacks_t diff_callbacks; + svn_wc_diff_callbacks2_t diff_callbacks; diff_callbacks.file_changed = diff_file_changed; diff_callbacks.file_added = diff_file_added; @@ -2155,7 +2225,7 @@ diff_file_deleted_with_diff; diff_callbacks.dir_added = diff_dir_added; diff_callbacks.dir_deleted = diff_dir_deleted; - diff_callbacks.props_changed = diff_props_changed; + diff_callbacks.dir_props_changed = diff_props_changed; diff_cmd_baton.orig_path_1 = path1; diff_cmd_baton.orig_path_2 = path2; @@ -2195,7 +2265,7 @@ apr_pool_t *pool) { struct diff_cmd_baton diff_cmd_baton; - svn_wc_diff_callbacks_t diff_callbacks; + svn_wc_diff_callbacks2_t diff_callbacks; diff_callbacks.file_changed = diff_file_changed; diff_callbacks.file_added = diff_file_added; @@ -2203,7 +2273,7 @@ diff_file_deleted_with_diff; diff_callbacks.dir_added = diff_dir_added; diff_callbacks.dir_deleted = diff_dir_deleted; - diff_callbacks.props_changed = diff_props_changed; + diff_callbacks.dir_props_changed = diff_props_changed; diff_cmd_baton.orig_path_1 = path; diff_cmd_baton.orig_path_2 = path; Index: subversion/tests/clients/cmdline/merge_tests.py =================================================================== --- subversion/tests/clients/cmdline/merge_tests.py (revision 12006) +++ subversion/tests/clients/cmdline/merge_tests.py (arbetskopia) @@ -2073,7 +2073,67 @@ None, None, None, None, None, wc_dir) +#----------------------------------------------------------------------- +# Regression test for issue #2064 +def merge_keyword_expansions(sbox): + "merge changes to keyword expansion property" + + sbox.build() + + wcpath = sbox.wc_dir + tpath = os.path.join(wcpath, "t") + bpath = os.path.join(wcpath, "b") + t_fpath = os.path.join(tpath, 'f') + b_fpath = os.path.join(bpath, 'f') + + os.mkdir(tpath) + svntest.main.run_svn(None, "add", tpath) + # Commit r2. + svntest.actions.run_and_verify_svn(None, None, [], + "ci", "-m", "r2", wcpath) + + # Copy t to b. + svntest.main.run_svn(None, "cp", tpath, bpath) + # Commit r3 + svntest.actions.run_and_verify_svn(None, None, [], + "ci", "-m", "r3", wcpath) + + # Add a file to t. + svntest.main.file_append(t_fpath, "$Revision$") + svntest.actions.run_and_verify_svn(None, None, [], + 'add', t_fpath) + # Ask for keyword expansion in the file. + svntest.actions.run_and_verify_svn(None, None, [], + 'propset', 'svn:keywords', 'Revision', + t_fpath) + # Commit r4 + svntest.actions.run_and_verify_svn(None, None, [], + 'ci', '-m', 'r4', wcpath) + + # Update the wc before the merge. + svntest.actions.run_and_verify_svn(None, None, [], + 'update', wcpath) + + # Do the merge. + expected_output = wc.State(bpath, { + 'f' : Item(status='A '), + }) + expected_disk = wc.State('', { + 'f' : Item("$Revision: 4 $"), + }) + expected_status = wc.State(bpath, { + '' : Item(status=' ', wc_rev=4, repos_rev=4), + 'f' : Item(status='A ', wc_rev='-', copied='+', repos_rev=4), + }) + expected_skip = wc.State(bpath, { }) + svntest.actions.run_and_verify_merge(bpath, '2', 'HEAD', + svntest.main.current_repo_url + '/t', + expected_output, + expected_disk, + expected_status, + expected_skip) + #---------------------------------------------------------------------- def merge_prop_change_to_deleted_target(sbox): "merge prop change into deleted target" @@ -2150,6 +2210,7 @@ dry_run_adds_file_with_prop, merge_binary_with_common_ancestry, merge_funny_chars_on_path, + merge_keyword_expansions, merge_prop_change_to_deleted_target, # property_merges_galore, # Would be nice to have this. # tree_merges_galore, # Would be nice to have this.