Neels Hofmeyr wrote:
>
> Julian Foad wrote:
>> On Wed, 2008-08-20 at 10:58 +0100, Julian Foad wrote:
>>> Here is a log message for the changes currently sitting on the
>>> tree-conflicts branch.
>>>
> ...
>>> * subversion/libsvn_wc/update_editor.c
> ...
>>> (do_entry_deletion): Have the parent's admin access baton passed in
>>> by the
>>> caller. Check for tree conflicts.
>>> ### Broken when parent_adm_access is NULL.
>
> taking this one.
>
This is quite intricate.
We've got these functions:
do_entry_deletion() "Helper for delete_entry()."
delete_entry() Yes, it calls do_entry_deletion().
close_edit() Why, this one *also* calls do_entry_deletion().
On trunk, there was no adm_access passed down into do_entry_deletion(). It
used to figure that out itself from the parent_path and the edit baton:
SVN_ERR(svn_wc_adm_retrieve(&adm_access, eb->adm_access,
parent_path, pool));
On the tree-conflicts branch, this line was moved out to delete_entry(). The
result is passed down into do_entry_deletion() as a new parameter called
parent_adm_access.
Now, in close_edit(), this new parameter is simply set to "NULL", which is
the problem.
The comment above do_entry_deletion has this to say:
/* PARENT_ADM_ACCESS is the admin access baton for the parent directory,
* or NULL if this is the target of the "update" being deleted.
It tries to say that, e.g., when the user provided a target path on the
command line (say `svn up A/D/G') and G *itself* is deleted by the update,
PARENT_ADM_ACCESS is NULL.
Basically, this amounts to a mere flag which says whether
do_entry_deletion() has been called by delete_entry() or close_edit(). This
"flag" is used to skip tree-conflicts detection for the latter case. (Why?)
What bothers us is that do_entry_deletion() *uses* the new parent_adm_access
parameter instead of the "figured out" adm_access. close_edit() sets it to
NULL and thus causes a segmentation fault.
So far, it seemingly makes sense to revert to "figuring out" adm_access
within do_entry_deletion(), and introduce a flag parameter
do_tree_conflicts_detection (instead of parent_adm_access).
Which leads me back to above question: Why ever is tree-conflicts detection
disabled for the case that the user provided explicit update targets which
happen to be removed by the update?
The log history has nothing on this. Addition of parameter parent_adm_access
is not mentioned anywhere.
So, I investigated and came up with below patch. It does this:
- do *not* skip tree-conflicts testing for that special case
(parent_adm_access == NULL), retrieve adm_access as done on trunk.
That itself causes an error message "Directory '%s' is missing", in
check_tree_conflict() calling entry_has_local_mods().
So, I gather, this is actually use case 3: update tries to delete, but the
user has locally removed the directory (without marking it as removed, though).
The subtle difference being that the target of deletion has explicitly been
issued as target, e.g. on the commandline, and not picked up recursively.
- in check_tree_conflict(), I thus add some code to catch this case, where a
directory is missing. The current check node->kind == svn_node_none is not
sufficient, because a missing dir is apparently still svn_node_dir if its
parent is still there.
- If this case is encountered, I mark it svn_wc_conflict_reason_missing.
This causes update_tests.py 15 to fail, because the output contains this
tree-conflict all of a sudden.
I could choose to merely catch above case and not mark it as a conflict,
then update_tests.py 15 succeeds. However, this *is* a tree-conflict and
should be marked.
- I fix update_tests.py 15 by calling svn resolved on the conflict marked
directory in the appropriate place.
I hope I've made this somewhat clear without babbling too much... ;)
Neels
[[[
Fix segmentation fault during tree-conflict detection, in effect catching
a tree-conflicts case where an update wants to delete a directory that is
missing in the local working copy, when the victim is an explicit target,
e.g. explicitly issued on the command line.
* subversion/libsvn_wc/update_editor.c
(check_tree_conflict): Catch case of update trying to remove a directory
that is missing in the working copy, marking a tree-conflict with
'svn_wc_conflict_reason_missing'.
(do_entry_deletion): Revert to behaviour as on trunk, getting adm_access
via svn_wc_adm_retrieve(). Remove parameter PARENT_ADM_ACCESS, it was
added on the branch. Remove error comment (error is hereby fixed).
Adjust function's main comment, adding that this is also a helper for
close_edit(), removing the comment about PARENT_ADM_ACCESS.
(delete_entry): Call do_entry_deletion() without adm_access parameter,
remove retrieval of adm_access. Revert to spacing as on trunk.
(close_edit): Remove the PARENT_ADM_ACCESS parameter, passed as NULL
to do_entry_deletion().
* subversion/tests/cmdline/update_tests.py (another_hudson_problem):
Above change marks a tree-conflict in this test, which confuses
expected output. Adding a svn call that resolves the conflict.
]]]
--
Neels Hofmeyr -- elego Software Solutions GmbH
Gustav-Meyer-Allee 25 / Gebäude 12, 13355 Berlin, Germany
phone: +49 30 23458696 mobile: +49 177 2345869 fax: +49 30 23458695
http://www.elegosoft.com | Geschäftsführer: Olaf Wagner | Sitz: Berlin
Handelsreg: Amtsgericht Charlottenburg HRB 77719 | USt-IdNr: DE163214194
Received on 2008-08-23 22:47:50 CEST