You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@subversion.apache.org by st...@apache.org on 2012/06/19 22:12:34 UTC
svn commit: r1351834 -
/subversion/trunk/subversion/bindings/javahl/src/org/tigris/subversion/javahl/ConflictDescriptor.java
Author: stsp
Date: Tue Jun 19 20:12:34 2012
New Revision: 1351834
URL: http://svn.apache.org/viewvc?rev=1351834&view=rev
Log:
Fix a crash in JavaHL due to my recent changes for delayed invocation of
the interactive conflict resolution callback.
Conflict descriptions read back from conflict storage lack some information
previously passed to the callback during a running update/merge process.
JavaHL's tigris.org backwards compat code unconditionally dereferenced
pointers to the left/mine versions contained in conflict descriptions,
which are no longer provided because of my changes :(
Let's hope we can fix this properly in the short term by persisting more
information about conflicts in the conflict store (hi Bert!) and then reading
it back from there. For now, just avoid dereferencing null pointers.
* subversion/bindings/javahl/src/org/tigris/subversion/javahl/ConflictDescriptor.java
(ConflictDescriptor): Don't dereference null ConflictVersions in the conflict
description provided by the org.apache Java code.
Modified:
subversion/trunk/subversion/bindings/javahl/src/org/tigris/subversion/javahl/ConflictDescriptor.java
Modified: subversion/trunk/subversion/bindings/javahl/src/org/tigris/subversion/javahl/ConflictDescriptor.java
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/bindings/javahl/src/org/tigris/subversion/javahl/ConflictDescriptor.java?rev=1351834&r1=1351833&r2=1351834&view=diff
==============================================================================
--- subversion/trunk/subversion/bindings/javahl/src/org/tigris/subversion/javahl/ConflictDescriptor.java (original)
+++ subversion/trunk/subversion/bindings/javahl/src/org/tigris/subversion/javahl/ConflictDescriptor.java Tue Jun 19 20:12:34 2012
@@ -125,8 +125,12 @@ public class ConflictDescriptor
aDesc.getAction().ordinal(), aDesc.getReason().ordinal(),
aDesc.getOperation().ordinal(), aDesc.getBasePath(),
aDesc.getTheirPath(), aDesc.getMyPath(), aDesc.getMergedPath(),
- new ConflictVersion(aDesc.getSrcLeftVersion()),
- new ConflictVersion(aDesc.getSrcRightVersion()));
+ aDesc.getSrcLeftVersion() != null
+ ? new ConflictVersion(aDesc.getSrcLeftVersion())
+ : null,
+ aDesc.getSrcRightVersion() != null
+ ? new ConflictVersion(aDesc.getSrcRightVersion())
+ : null);
}
public String getPath()
Re: svn commit: r1351834 - /subversion/trunk/subversion/bindings/javahl/src/org/tigris/subversion/javahl/ConflictDescriptor.java
Posted by Stephen Butler <sb...@elego.de>.
On Jun 19, 2012, at 22:12 , stsp@apache.org wrote:
> Author: stsp
> Date: Tue Jun 19 20:12:34 2012
> New Revision: 1351834
>
> URL: http://svn.apache.org/viewvc?rev=1351834&view=rev
> Log:
> Fix a crash in JavaHL due to my recent changes for delayed invocation of
> the interactive conflict resolution callback.
>
> Conflict descriptions read back from conflict storage lack some information
> previously passed to the callback during a running update/merge process.
> JavaHL's tigris.org backwards compat code unconditionally dereferenced
> pointers to the left/mine versions contained in conflict descriptions,
> which are no longer provided because of my changes :(
> Let's hope we can fix this properly in the short term by persisting more
> information about conflicts in the conflict store (hi Bert!) and then reading
> it back from there.
I think the problem isn't persisting the information per se, it's whacking all
of the moles^W edge cases. In the function
subversion/libsvn_wc/update_editor.c:create_tree_conflict()
there's a conflict for which a version field is NULL:
[[[
if (left_repos_relpath == NULL)
/* A locally added or unversioned path in conflict with an incoming add.
* Send an 'empty' left revision. */
src_left_version = NULL;
else
]]]
This function was written a couple of years ago and hasn't changed much
since. If we want a left_revision in all cases, we should revisit it and its
counterpart in the merge code.
For reference, here's the original commit of most of the code in
create_tree_conflict():
http://svn.apache.org/viewvc?view=revision&revision=911661
and here's the todo-comment mentioned there:
[[[
/* ###TODO: It would be nice to tell the user at which URL and
* ### revision source-left was empty, which could be quite difficult
* ### to code, and is a slight theoretical leap of the svn mind.
* ### Update should show
* ### URL: svn_wc__db_scan_addition( &repos_relpath )
* ### REV: The base revision of the parent of before this update
* ### started
* ### ### BUT what if parent was updated/switched away with
* ### ### depth=empty after this node was added?
* ### Switch should show
* ### URL: scan_addition URL of before this switch started
* ### REV: same as above */
]]]
See notes/tree-conflicts/all-add-vs-add-tree-conflicts.txt for more details.
Steve
--
Stephen Butler
elego Software Solutions GmbH
http://www.elego.de
Re: svn commit: r1351834 - /subversion/trunk/subversion/bindings/javahl/src/org/tigris/subversion/javahl/ConflictDescriptor.java
Posted by Stephen Butler <sb...@elego.de>.
On Jun 19, 2012, at 22:12 , stsp@apache.org wrote:
> Author: stsp
> Date: Tue Jun 19 20:12:34 2012
> New Revision: 1351834
>
> URL: http://svn.apache.org/viewvc?rev=1351834&view=rev
> Log:
> Fix a crash in JavaHL due to my recent changes for delayed invocation of
> the interactive conflict resolution callback.
>
> Conflict descriptions read back from conflict storage lack some information
> previously passed to the callback during a running update/merge process.
> JavaHL's tigris.org backwards compat code unconditionally dereferenced
> pointers to the left/mine versions contained in conflict descriptions,
> which are no longer provided because of my changes :(
> Let's hope we can fix this properly in the short term by persisting more
> information about conflicts in the conflict store (hi Bert!) and then reading
> it back from there.
I think the problem isn't persisting the information per se, it's whacking all
of the moles^W edge cases. In the function
subversion/libsvn_wc/update_editor.c:create_tree_conflict()
there's a conflict for which a version field is NULL:
[[[
if (left_repos_relpath == NULL)
/* A locally added or unversioned path in conflict with an incoming add.
* Send an 'empty' left revision. */
src_left_version = NULL;
else
]]]
This function was written a couple of years ago and hasn't changed much
since. If we want a left_revision in all cases, we should revisit it and its
counterpart in the merge code.
For reference, here's the original commit of most of the code in
create_tree_conflict():
http://svn.apache.org/viewvc?view=revision&revision=911661
and here's the todo-comment mentioned there:
[[[
/* ###TODO: It would be nice to tell the user at which URL and
* ### revision source-left was empty, which could be quite difficult
* ### to code, and is a slight theoretical leap of the svn mind.
* ### Update should show
* ### URL: svn_wc__db_scan_addition( &repos_relpath )
* ### REV: The base revision of the parent of before this update
* ### started
* ### ### BUT what if parent was updated/switched away with
* ### ### depth=empty after this node was added?
* ### Switch should show
* ### URL: scan_addition URL of before this switch started
* ### REV: same as above */
]]]
See notes/tree-conflicts/all-add-vs-add-tree-conflicts.txt for more details.
Steve
--
Stephen Butler
elego Software Solutions GmbH
http://www.elego.de