wc_db_update_move.c

Checkout Tools
  • last updated 3 hours ago
Constraints
Constraints: committers
 
Constraints: files
Constraints: dates
Fix memory lifetime problem in a libsvn_wc error code path.

* subversion/libsvn_wc/wc_db_update_move.c

(suitable_for_move): Calling svn_sqlite__column_text() with a NULL result

pool twice means the result of the first call becomes invalid. Store the

child_relpath variable in a pool. It is passed to path_for_error_message()

later, after another call to svn_sqlite__column_text() with a NULL result

pool has already occurred.

Crash observed on OpenBSD:

#0 strlen () at /usr/src/lib/libc/arch/amd64/string/strlen.S:125

#1 0x00000c38d5de6db7 in svn_dirent_join (

base=0xc38dfe1ef00 "/home/stsp/svn/svn-1.12.0/subversion/tests/libsvn_wc/svn-test-work/working-copies/move_update_subtree",

component=0xc390ca94fc8 '\337' <repeats 55 times>, <incomplete sequence \337><error: Cannot access memory at address 0xc390ca95000>, pool=0xc38eeceff00)

at subversion/libsvn_subr/dirent_uri.c:1007

#2 0x00000c38f686a815 in path_for_error_message (wcroot=0xc387ee3d300,

local_relpath=0xc390ca94fc8 '\337' <repeats 55 times>, <incomplete sequence \337><error: Cannot access memory at address 0xc390ca95000>,

result_pool=0xc38eeceff00) at subversion/libsvn_wc/wc_db_update_move.c:167

#3 0x00000c38f686ad1f in suitable_for_move (wcroot=0xc387ee3d300,

local_relpath=0xc387efe4ce0 "A/B", scratch_pool=0xc38eeceff00)

at subversion/libsvn_wc/wc_db_update_move.c:2192

Record the move target path in a moved-away tree conflict skel.

Updates can modify the NODES table in ways which discard local move

information about tree conflict victims. One example is:

echo foo >> epsilon/foo.txt

svn mv epsilon alpha

svn ci # create r2

svn up -r1

svn mv epsilon beta

svn up -r2

When 'epsilon' is updated to r2, a tree conflict is raised and the move

'epsilon' -> 'beta' is deleted from the NODES table. 'beta' remains a copy.

$ svn status

A + beta

! C epsilon

> local dir moved away, incoming dir delete or move upon update

$

The conflict skel for the tree conflict on 'epsilon' now contains:

(tree () moved-away deleted epsilon)

This leaves the resolver with insufficient information about the local

move 'epsilon' -> 'beta' which existed before the update. To resolve this

conflict we must be able to identify the copy 'beta' as one potential move

target. The fact that copyfrom on 'beta' points to 'epsilon' is insufficient

because, in the general case, this copy could have occurred independently

of the tree conflict.

As of this commit, the tree conflict skel also records a move destination

path, 'beta' in our example:

(tree () moved-away deleted epsilon beta)

Apart from recording the path in the skel, this commit introduces no other

visible change in behaviour. In the future, the conflict resolver will be

able to make use of this new information to correlate the copy 'beta' with

the conflict victim 'epsilon' in the above example.

Note that old clients will simply ignore the new extra element at the end

of the conflict skel.

* subversion/libsvn_wc/conflicts.c

(svn_wc__conflict_skel_add_tree_conflict): Add move_dst_op_root_abspath

parameter and append it to the skel if it is non-NULL.

(svn_wc__conflict_read_tree_conflict): Return the move_dst_op_root_abspath

if present in the conflict skel.

(read_tree_conflict_desc, resolve_tree_conflict_on_node,

svn_wc__conflict_tree_update_break_moved_away,

svn_wc__conflict_tree_update_incoming_move,

svn_wc__conflict_tree_update_local_add): Update callers.

* subversion/libsvn_wc/conflicts.h

(svn_wc__conflict_skel_add_tree_conflict,

svn_wc__conflict_read_tree_conflict): Update declaration and docstring.

* subversion/libsvn_wc/questions.c

(internal_conflicted_p): Update caller.

* subversion/libsvn_wc/tree_conflicts.c

(svn_wc__add_tree_conflict): Update caller.

* subversion/libsvn_wc/update_editor.c

(open_root, check_tree_conflict, add_directory, open_directory,

add_file, open_file, change_file_prop): Update callers.

* subversion/libsvn_wc/upgrade.c

(svn_wc__upgrade_conflict_skel_from_raw): Update caller.

* subversion/libsvn_wc/wc_db.c

(revert_maybe_raise_moved_away): Update caller.

* subversion/libsvn_wc/wc_db_update_move.c

(create_tree_conflict, fetch_conflict_details): Update callers.

* subversion/tests/libsvn_wc/conflict-data-test.c

(test_serialize_tree_conflict): Update test expectations.

* subversion/tests/libsvn_wc/op-depth-test.c

(check_db_conflicts): Update caller.

  1. … 9 more files in changeset.
* subversion/libsvn_wc/wc_db_update_move.c

(update_locally_added_node): Remove local variable 'local_relpath' which was

a copy of 'nb->local_relpath', and just use 'nb->local_relpath' throughout.

* subversion/libsvn_wc/wc_db_update_move.c

(update_locally_added_node): Pass an absolute path to the

svn_io_file_checksum2() function instead of passing a relative path.

Add an implementation of a new resolver option which resolves "incoming add

vs local add upon update" by merging the local and incoming directory trees.

It mirrors the same resolution option we already support for 'svn merge'.

The implementation for update is entirely different, though. It is merging

the new BASE tree with a locally added tree, both stored in wc.db, whereas

the same resolution option for merge is driven by a repository diff.

This is still work in progress. It mostly works, but there are no tests yet

to confirm that it is working well. But it is functionally complete and good

enough to be committed now.

* subversion/include/private/svn_wc_private.h

(svn_wc__conflict_tree_update_local_add): Declare.

* subversion/libsvn_client/conflicts.c

(resolve_update_incoming_added_dir_merge): New resolver function.

(configure_option_incoming_added_dir_merge): Configure new resolver function.

* subversion/libsvn_wc/conflicts.c

(svn_wc__conflict_tree_update_local_add): New private libsvn_wc API function

which backs the new resolution option.

* subversion/libsvn_wc/wc_db.h

(svn_wc__db_update_local_add): Declare.

* subversion/libsvn_wc/wc_db_update_move.c

(): Update notes on top of this file to document our procedure for updating

locally added directories.

(create_tree_conflict): Handle 'old_version' being NULL, as it is when

a conflict is due to a local addition.

(update_local_add_baton_t, added_node_baton_t): New types for a small

editor-style collection of functions which do the actual ground work.

(update_local_add_mark_node_edited, update_local_add_mark_parent_edited,

mark_update_add_add_tree_conflict, update_incoming_add_merge_props,

update_local_add_notify_obstructed_or_missing): New helper functions.

(tc_editor_update_add_new_file, tc_editor_update_add_new_directory,

tc_editor_update_add_merge_files, tc_editor_update_add_merge_dirprops):

New editor-style callbacks which modify working copy state.

(update_locally_added_node): Drives the former editor functions according

to differences between the post-update BASE layer and the directory tree

which was locally added at the same path before the update was executed.

(update_local_add): The body of svn_wc__db_update_local_add().

This runs within a wc.db transaction.

(svn_wc__db_update_local_add): New wc_db API function for updating locally

added directory trees.

  1. … 4 more files in changeset.
Second attempt to preserve local changes for files which become victims of

a nested incoming move during an update (same problem as in r1771924).

This time, we make sure the file is copied to the move destination before

the move source is deleted. The file becomes the victim of a new tree conflict

at the move destination, which the resolver could resolve later on.

Information about the file's original location is preserved in conflict data.

* subversion/libsvn_wc/wc_db_update_move.c

(tc_editor_update_incoming_moved_file): If the file cannot be found at the

move destination, copy it there and raise a new tree conflict.

* subversion/tests/libsvn_client/conflicts-test.c

(test_update_incoming_dir_move_with_nested_file_move): Update test

expectations accordingly. And verify that expected file content is present.

  1. … 1 more file in changeset.
Revert r1771924.

Leaving nested conflicts within the tree conflict victim of an incoming

move after update does not seem to be the best approach. It is unclear how

we can make sure the victim, which is now left behind as a non-conflicted copy,

will eventually be removed to complete the incoming move.

I will try another idea next, which is to move any conflicted children into

the incoming move destination and let their conflicts be resolved there.

  1. … 5 more files in changeset.
[ Note from the future: Reverted in r1772051 ].

Make sure to preserve local changes for nodes which become victims of a

nested incoming move during an update.

With a nested move of a file, previous code flagged a tree conflict at a path

where this file was expected to sit relative to its moved parent.

This was wrong, since the file was not moved along with the parent, but was

also moved. So the real location of the missing file is unknown at that point.

Worse, conflict test 25 has shown that local changes to this file ended up

being deleted after resolving the parent's tree conflict.

Instead, we must keep the parent's copy around if any new conflicts appear

within its children. This preserves local modifications and allows the

resolver to resolve these conflicted children later on.

This change has some loose ends. The nested conflict cannot be resolved yet,

and I'm not sure how we can eventually decide to delete the original conflict

victim (the parent) in order to complete the incoming move operation after

resolving all the nested conflicts. Perhaps we need a new flag in conflict

storage for this?

* subversion/libsvn_client/conflicts.c

(resolve_incoming_move_dir_merge): If new conflicts remain after resolving

the update of an incoming move, do not blow the conflict victim away.

Delete its tree conflict marker instead but otherwise leave it unchanged.

This allows nested conflicts to be dealt with later on.

* subversion/include/private/svn_wc_private.h,

subversion/libsvn_wc/conflicts.c

(svn_wc__conflict_tree_update_incoming_move): New output parameter

'new_conflicts_remain'. Pass it on.

* subversion/libsvn_wc/wc_db.h,

subversion/libsvn_wc/wc_db_update_move.c

(svn_wc__db_update_incoming_move); New output parameter

'new_conflicts_remain'.

(update_move_baton_t): New field 'raised_new_conflicts'.

(mark_tc_on_op_root, tc_editor_add_directory,

tc_editor_incoming_add_directory, tc_editor_add_file,

tc_editor_incoming_add_file, tc_editor_alter_directory,

tc_editor_alter_file, tc_editor_update_incoming_moved_file,

tc_editor_delete, tc_incoming_editor_delete): Set the 'raised_new_conflicts'

flag in the update_move_baton if a new conflict is raised.

(tc_editor_update_incoming_moved_file): Likewise, and also change the

way conflicts are recorded: If the move destination is missing, then

flag a conflict at the move source, not at the move destination (which

is not really known at this point).

(update_incoming_move): New output parameter 'new_conflicts_remain'.

* subversion/tests/libsvn_client/conflicts-test.c

(test_update_incoming_dir_move_with_nested_file_move): Adjust test

expectations accordingly.

  1. … 5 more files in changeset.
* subversion/libsvn_client/conflicts.c

(tc_editor_update_incoming_moved_file): Rename local variable to better name.

* subversion/libsvn_client/conflicts.c

(tc_editor_merge_local_file_change): Rename to ...

(tc_editor_update_incoming_moved_file): ... this.

(update_incoming_moved_node): Update caller.

Fix paths recorded in tree conflicts while updating incoming moves.

* subversion/libsvn_wc/wc_db_update_move.c

(update_incoming_move): Destination op-depth must be set to the relpath

depth of the incoming move instead of zero. This allows paths recorded

in new tree conflicts to be constructed correctly.

* subversion/libsvn_wc/wc_db_update_move.c

(tc_editor_merge_local_file_change): Fix incorrect use of node kind variable.

Found by: svn-windows-ra buildbot

Fix handling of nested file moves in the while updating incoming moves.

Also fix some bugs in the associated regression test.

* subversion/svn/conflict-callbacks.c

(tc_editor_merge_local_file_change): Fix tree conflict recording in case

a file is missing or obstructed. The existing code was inherited from the

local-move update code and not yet adjusted. Only merge props and text if

no tree conflict was raised.

* subversion/tests/libsvn_client/conflicts-test.c

(modified_file_content): New global constant.

(test_update_incoming_dir_move_with_nested_file_move): Fix a typo in a path

which caused a new unversioned file to be created, instead of a local

modification to a versioned file. And this test did not check the

resolution result at all! Expect a tree conflict on the nested moved file.

  1. … 1 more file in changeset.
* subversion/libsvn_wc/wc_db_update_move.c

(create_conflict_markers): Allow setting 'merge' as an operation.

Make the conflict resolver use reasonable diff labels for text conflicts which

arise during an update of an incoming move.

* subversion/libsvn_wc/wc_db_update_move.c

(tc_editor_merge_local_file_change): Override the diff labels used by

svn_wc__internal_merge() so the resulting conflict diff3 makes more sense.

* subversion/libsvn_wc/wc_db_update_move.c

(update_incoming_moved_node): Cosmetic whitespace tweak.

In the conflict resolver, ensure we don't leave temp files behind upon error.

* subversion/libsvn_wc/wc_db_update_move.c

(tc_editor_incoming_add_file): Remove the content copy in cases where this

removal won't be handled by the work queue.

(update_incoming_moved_node): Allow cancellation during file content copy and

ensure we won't leave the temporary file on disk if the copy is cancelled.

Fix the recursive tree walk logic for updating incoming moves.

* subversion/libsvn_wc/wc_db_update_move.c

(update_incoming_moved_node): Continue walking based on whether the WORKING

layer node is a directory, not whether the old victim node is a directory.

* subversion/libsvn_wc/wc_db_update_move.c

(update_incoming_move): Fix comment.

* subversion/libsvn_wc/wc_db_update_move.c

(tc_editor_incoming_add_file): No need to check for shadowed nodes here.

Merge a duplicated bit of code in the resolver into a helper function.

* subversion/libsvn_wc/wc_db_update_move.c

(copy_working_node): New helper function.

(tc_editor_incoming_add_directory, tc_editor_incoming_add_file): Use it.

Add a custom 'add directory' handler for updates of incoming moves.

* subversion/libsvn_wc/wc_db_update_move.c

(tc_editor_incoming_add_directory): New editor function.

(update_incoming_moved_node): Call new function.

* subversion/libsvn_wc/wc_db_update_move.c

(suitable_for_move): Check for switched subtrees before checking for

mixed-revness. Otherwise subtrees switched to a PATH@REV where REV is

not equal to the parent's revision trigger the mixed-rev error message

instead of the switched-subtree error message.

Improve two conflict resolver error messages.

* subversion/libsvn_wc/wc_db_update_move.c

(suitable_for_move): Provide hints as to what the user can do to fix

switched nodes and mixed-rev WCs. Show the proper path for switched

nodes instead of the path of the move target.

Follow-up to r1771469 and r1771471:

* subversion/libsvn_wc/wc_db_update_move.c

(update_incoming_move): Use helper function suitable_for_move() instead

of reinventing it.

* subversion/libsvn_wc/wc_db_update_move.c

(update_incoming_move): Error out if incoming move is a mixed-rev WC.

* subversion/libsvn_wc/wc_db_update_move.c

(update_incoming_move): Error out if the incoming move destination contains

switched subtrees.

* subversion/libsvn_wc/wc_db_update_move.c

(create_conflict_markers): Assert that we're operating on valid input paths.

* subversion/libsvn_wc/wc_db_update_move.c

(update_incoming_move): Error out if local modifications exist at

the incoming move target location.

Fix a NULL-deref in the conflict resolver.

* subversion/libsvn_wc/wc_db_update_move.c

(create_conflict_markers): If the repository relpath is not a child

of the old conflict version, it must be a child of the new version.