Invalid change ordering?

[Dirk]

Hi Stephen,

what do you think of this patch?

Dirk

Dirk schrieb:
>
>> I think this discussion established that the middle two changes were 
>> unnecessary. Indeed the comment #1 just above it says "we can use 
>> this item name, since it was valid in that time" suggesting that when 
>> that comment was written someone was aware the item could have been 
>> deleted.
>
> Ok, I have thought about this a little longer, esp. in respect to the 
> 2 patched lines that regard projects, where the item is deleted, as 
> non valid. As you state above, and also the comment in the code, this 
> item pathes are valid. I would like to extend this: "they are valid, 
> but not the best choice, if there are other non deleted pathes". So I 
> would propose to first check for contexts, where the item is not 
> deleted and if we can't find one, we will fall back to a deleted path.
>
> Dirk
>
> _______________________________________________
> vss2svn-users mailing list
> Project homepage:
> http://www.pumacode.org/projects/vss2svn/
> Subscribe/Unsubscribe/Admin:
> http://lists.pumacode.org/mailman/listinfo/vss2svn-users-lists.pumacode.org 
>
> Mailing list web interface (with searchable archives):
> http://dir.gmane.org/gmane.comp.version-control.subversion.vss2svn.user
>
>
>

-------------- next part --------------
Index: ActionHandler.pm
===================================================================
--- ActionHandler.pm	(revision 291)
+++ ActionHandler.pm	(working copy)
@@ -575,7 +575,7 @@
 
     # recover this item within the current parent
     my $parentinfo = $physinfo->{parents}->{$row->{parentphys}};
-    if (undef $parentinfo->{deleted}) {
+    if (!defined $parentinfo->{deleted}) {
         $self->{errmsg} .= "Attempt to recover an active item '$physname':\n"
             . "$self->{physname_seen}\n";
 
@@ -1111,15 +1111,32 @@
 ###############################################################################
 #  _get_valid_path
 # This function returns an itempath for the physical file, that was valid in
-# the previous version. Since all activities that create a new version of a file
-# must be done on at least one active path, there should be at least one valid
-# item path for the version.
+# the previous version. Since all activities that create new versions of a file
+# must be done on an active path, there should be at least one valid item path
+# for the version.
 # If we can't find any valid itempath, we can not perform a "copy from" revision
 # In this case, we need to recheckin the current content of the item
 ###############################################################################
 sub _get_valid_path {
     my($self, $physname, $parentphys, $version) = @_;
+    
+    # 1.) check for non deleted contexts
+    my $path = $self->_get_valid_path2 ($physname, $parentphys, $version, 0);
+    return $path if defined $path;
 
+    # 2.) now check also for deleted contexts
+    $path = $self->_get_valid_path2 ($physname, $parentphys, $version, 1);
+    return $path;
+} # End _get_valid_path
+
+###############################################################################
+#  _get_valid_path2
+# This function is an internal helper: It will check for active and inactive,
+# but valid item pathes, depending on the $deleted flag, see also _get_valid_path
+###############################################################################
+sub _get_valid_path2 {
+    my($self, $physname, $parentphys, $version, $deleted) = @_;
+
     my $physinfo = $gPhysInfo{$physname};
     if (!defined $physinfo) {
         return undef;
@@ -1130,10 +1147,10 @@
     }
     
     # 1. check the parent requested, if there was an item name for this version
-    # we can use this item name, since it was valid in that time
+    #    we can use this item name, since it was valid in that time
     my $parent = $physinfo->{parents}->{$parentphys};
     if (defined $parent &&
-#        $parentphys ne '99999999' &&
+        (!defined $parent->{deleted} || $deleted == 1) &&
         $parent->{versions}->[$version]) {
         return $parent->{versions}->[$version];
     }
@@ -1146,14 +1163,14 @@
 
         $parent = $physinfo->{parents}->{$parentphys};
         if (defined $parent &&
-#            $parentphys ne '99999999' &&
+            (!defined $parent->{deleted} || $deleted == 1) &&
             $parent->{versions}->[$version]) {
             return $parent->{versions}->[$version];
         }
     }
     
     return undef;
-}  #  End _get_valid_path
+}  #  End _get_valid_path2
 
 ###############################################################################
 #  _add_parent
@@ -1167,8 +1184,15 @@
 
     my $physinfo = $gPhysInfo{$physname};
     if (defined $physinfo) {
-        $physinfo->{parents}->{$parentphys} = {};
-        push @{ $physinfo->{order} }, $parentphys;
+        # check wether this parent was previously deleted
+        if (defined $physinfo->{parents}->{$parentphys} &&
+            defined $physinfo->{parents}->{$parentphys}->{deleted}) {
+            undef $physinfo->{parents}->{$parentphys}->{deleted};
+        }
+        else {
+            $physinfo->{parents}->{$parentphys} = {};
+            push @{ $physinfo->{order} }, $parentphys;
+        }
     }
 }  #  End _add_parent