Fix for perm-changer bug

git-svn-id: http://svn.savannah.nongnu.org/svn/rdiff-backup/trunk@533 2b77aa54-bcbc-44c9-a7ec-4f6cf2b41109

3 files changed, 38 insertions, 20 deletions

diff --git a/rdiff-backup/CHANGELOG b/rdiff-backup/CHANGELOG
index 4b9158d..0936943 100644
--- a/rdiff-backup/CHANGELOG
+++ b/rdiff-backup/CHANGELOG
@@ -10,6 +10,8 @@ Fix by Dean Gaudet for --calculate-average mode (it broke somewhere in
 Fix for regress warning code:  rdiff-backup should warn you if you are
 trying to back up a directory into itself.
 
+Fix for restoring certain directories when not run as root.
+
 Now when determining group permissions check supplementary groups as
 well as main group.  (Bug report by Ryan Castle.)
 
diff --git a/rdiff-backup/rdiff_backup/restore.py b/rdiff-backup/rdiff_backup/restore.py
index 1c1d1cc..1492cfd 100644
--- a/rdiff-backup/rdiff_backup/restore.py
+++ b/rdiff-backup/rdiff_backup/restore.py
@@ -348,7 +348,7 @@ class CachedRF:
 				if not self.add_rfs(index): return None
 			rf = self.rf_list[0]
 			if rf.index == index:
-				if Globals.process_uid != 0: self.perm_changer(rf.mirror_rp)
+				if Globals.process_uid != 0: self.perm_changer(index)
 				return rf
 			elif rf.index > index:
 				# Try to add earlier indicies.  But if first is
@@ -377,9 +377,9 @@ The cause is probably data loss from the destination directory.""" %
 		"""
 		if not index: return self.root_rf
 		parent_index = index[:-1]
+		if Globals.process_uid != 0: self.perm_changer(parent_index)
 		temp_rf = RestoreFile(self.root_rf.mirror_rp.new_index(parent_index),
 							  self.root_rf.inc_rp.new_index(parent_index), [])
-		if Globals.process_uid != 0: self.perm_changer(temp_rf.mirror_rp)
 		new_rfs = list(temp_rf.yield_sub_rfs())
 		if not new_rfs:
 			log.Log("Warning: No RFs added for index %s" % (index,), 2)
@@ -690,16 +690,15 @@ class PermissionChanger:
 		# order that need clearing
 		self.open_index_list = []
 
-	def __call__(self, rp):
-		"""Given rpath, change permissions up and including rp"""
-		index, old_index = rp.index, self.current_index
+	def __call__(self, index):
+		"""Given rpath, change permissions up to and including index"""
+		old_index = self.current_index
 		self.current_index = index
-		if not index or index == old_index: return
-		assert index > old_index, (index, old_index)
-		self.restore_old(rp, index)
-		self.add_new(rp, old_index, index)
+		if not index or index <= old_index: return
+		self.restore_old(index)
+		self.add_new(old_index, index)
 
-	def restore_old(self, rp, index):
+	def restore_old(self, index):
 		"""Restore permissions for indicies we are done with"""
 		while self.open_index_list:
 			old_index, old_rp, old_perms = self.open_index_list[0]
@@ -707,29 +706,31 @@ class PermissionChanger:
 			else: break
 			del self.open_index_list[0]
 
-	def add_new(self, rp, old_index, index):
+	def add_new(self, old_index, index):
 		"""Change permissions of directories between old_index and index"""
-		for rp in self.get_new_rp_list(rp, old_index, index):
+		for rp in self.get_new_rp_list(old_index, index):
 			if ((rp.isreg() and not rp.readable()) or
 				(rp.isdir() and not rp.hasfullperms())):
 				old_perms = rp.getperms()
-				self.open_index_list.insert(0, (index, rp, old_perms))
+				self.open_index_list.insert(0, (rp.index, rp, old_perms))
 				if rp.isreg(): rp.chmod(0400 | old_perms)
 				else: rp.chmod(0700 | old_perms)
 
-	def get_new_rp_list(self, rp, old_index, index):
-		"""Return list of new rp's between old_index and index"""
+	def get_new_rp_list(self, old_index, index):
+		"""Return list of new rp's between old_index and index
+
+		Do this lazily so that the permissions on the outer
+		directories are fixed before we need the inner dirs.
+
+		"""
 		for i in range(len(index)-1, -1, -1):
 			if old_index[:i] == index[:i]:
 				common_prefix_len = i
 				break
 		else: assert 0
 
-		new_rps = []
-		for total_len in range(common_prefix_len+1, len(index)):
-			new_rps.append(self.root_rp.new_index(index[:total_len]))
-		new_rps.append(rp)
-		return new_rps
+		for total_len in range(common_prefix_len+1, len(index)+1):
+			yield self.root_rp.new_index(index[:total_len])
 
 	def finish(self):
 		"""Restore any remaining rps"""
diff --git a/rdiff-backup/testing/roottest.py b/rdiff-backup/testing/roottest.py
index 87cad2a..171321e 100644
--- a/rdiff-backup/testing/roottest.py
+++ b/rdiff-backup/testing/roottest.py
@@ -124,6 +124,21 @@ class HalfRoot(unittest.TestCase):
 		rp2_3_3 = rp2_3.append('file2')
 		rp2_3_3.touch()
 		rp2_3.chmod(0)
+		# The rp_2_4 below test for a perm error, also tested in
+		# regressiontest.py testConfig1
+		rp2_4 = rp2.append('test2')
+		rp2_4.mkdir()
+		rp2_4_1 = rp2_4.append('1-dir')
+		rp2_4_1.mkdir()
+		reg2_4_1_1 = rp2_4_1.append('reg')
+		reg2_4_1_1.touch()
+		reg2_4_1_1.chmod(0)
+		rp2_4_1.chmod(0)
+		reg2_4_2 = rp2_4.append('2-reg')
+		reg2_4_2.touch()
+		reg2_4_2.chmod(0)
+		rp2_4.chmod(0)
+
 		return rp1, rp2
 
 	def cause_regress(self, rp):