Added --null-separator, tweaked increment error handling

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

14 files changed, 160 insertions, 73 deletions

diff --git a/rdiff-backup/CHANGELOG b/rdiff-backup/CHANGELOG
index d98f1a2..d088e9d 100644
--- a/rdiff-backup/CHANGELOG
+++ b/rdiff-backup/CHANGELOG
@@ -1,3 +1,13 @@
+New in v0.8.0 (2002/06/06)
+--------------------------
+
+Fixed 'size' bug that occurred on some non-Linux systems.  Thanks to
+Robert Weber for the report.
+
+Added --null-separator argument so filenames can safely include
+newlines in an include/exclude filelist.
+
+
 New in v0.7.6 (2002/05/31)
 --------------------------
 
diff --git a/rdiff-backup/rdiff-backup.1 b/rdiff-backup/rdiff-backup.1
index 05deb81..7f2b474 100644
--- a/rdiff-backup/rdiff-backup.1
+++ b/rdiff-backup/rdiff-backup.1
@@ -187,6 +187,13 @@ are present, this option can drastically decrease memory usage.
 Do not resume last aborted backup even if it falls within the resume
 window.
 .TP
+.B --null-separator
+Use nulls (\\0) instead of newlines (\\n) as line separators, which
+may help when dealing with filenames containing newlines.  This
+affects the expected format of the files specified by the
+--{include|exclude}-filelist[-stdin] switches as well as the format of
+the directory_statistics file.
+.TP
 .BI "-r, --restore-as-of " restore_time
 Restore the specified directory as it was as of
 .IR restore_time .
@@ -651,8 +658,10 @@ and
 .B --exclude-filelist-stdin
 options also introduce file selection conditions.  They direct
 rdiff-backup to read in a file, each line of which is a file
-specification, and to include or exclude the matching files.  The
-lines in a filelist are interpreted similarly to the way
+specification, and to include or exclude the matching files.  Lines
+are separated by newlines or nulls, depending on whether the
+--null-separator switch was given.  Each line in a filelist is
+interpreted similarly to the way
 .I extended shell patterns
 are, with a few exceptions:
 .TP
@@ -731,11 +740,6 @@ rdiff-backup uses the shell command
 to backup device files (e.g. /dev/ttyS0), so device files won't be
 handled correctly on systems with non-standard mknod syntax.
 .PP
-When an rdiff-backup session fails (for instance if a remote
-connection is lost), rdiff-backup tries to save the session so it can
-be resumed later.  Apparently sometimes, depending on how the
-rdiff-backup session fails, later sessions cannot be resumed properly.
-.PP
 Files whose names are close to the maximum length (e.g. 235 chars if
 the maximum is 255) may be skipped because the filenames of related
 increment files would be too long.
diff --git a/rdiff-backup/rdiff_backup/increment.py b/rdiff-backup/rdiff_backup/increment.py
index 7aa7009..5836f63 100644
--- a/rdiff-backup/rdiff_backup/increment.py
+++ b/rdiff-backup/rdiff_backup/increment.py
@@ -219,6 +219,7 @@ class IncrementITR(ErrorITR, StatsITR):
 			# Write updated mirror to temp file so we can compute
 			# reverse diff locally
 			mirror_tf = TempFileManager.new(dsrp)
+			old_dsrp_tf = TempFileManager.new(dsrp)
 			def init_thunk():
 				if diff_rorp.isflaglinked():
 					Hardlink.link_rp(diff_rorp, mirror_tf, dsrp)
@@ -226,8 +227,16 @@ class IncrementITR(ErrorITR, StatsITR):
 													  mirror_tf).execute()
 				self.incrp = Inc.Increment_action(mirror_tf, dsrp,
 												  incpref).execute()
-			def final(init_val): mirror_tf.rename(dsrp)
-			def error(exc, ran_init, init_val): mirror_tf.delete()
+				if dsrp.lstat(): RPathStatic.rename(dsrp, old_dsrp_tf)
+				mirror_tf.rename(dsrp)
+
+			def final(init_val): old_dsrp_tf.delete()
+			def error(exc, ran_init, init_val):
+				if ran_init: old_dsrp_tf.delete() # everything is fine
+				else: # restore to previous state
+					if old_dsrp_tf.lstat(): old_dsrp_tf.rename(dsrp)
+					if self.incrp: self.incrp.delete()
+
 			RobustAction(init_thunk, final, error).execute()
 		else: self.incrp = Robust.chain(
 			Inc.Increment_action(diff_rorp, dsrp, incpref),
diff --git a/rdiff-backup/rdiff_backup/lazy.py b/rdiff-backup/rdiff_backup/lazy.py
index 98a4027..c7d1a52 100644
--- a/rdiff-backup/rdiff_backup/lazy.py
+++ b/rdiff-backup/rdiff_backup/lazy.py
@@ -54,7 +54,7 @@ class Iter:
 				return None
 		try: i2 = iter2.next()
 		except StopIteration: return 1
-		if verbose: print "End when i2 = %s" % i2
+		if verbose: print "End when i2 = %s" % (i2,)
 		return None
 
 	def Or(iter):
diff --git a/rdiff-backup/rdiff_backup/selection.py b/rdiff-backup/rdiff_backup/selection.py
index 8b87af8..4c003c8 100644
--- a/rdiff-backup/rdiff_backup/selection.py
+++ b/rdiff-backup/rdiff_backup/selection.py
@@ -281,19 +281,25 @@ probably isn't what you meant.""" %
 
 	def filelist_read(self, filelist_fp, include, filelist_name):
 		"""Read filelist from fp, return (tuplelist, something_excluded)"""
+		prefix_warnings = [0]
+		def incr_warnings(exc):
+			"""Warn if prefix is incorrect"""
+			prefix_warnings[0] += 1
+			if prefix_warnings[0] < 6:
+				Log("Warning: file specification '%s' in filelist %s\n"
+					"doesn't start with correct prefix %s.  Ignoring." %
+					(exc, filelist_name, self.prefix), 2)
+				if prefix_warnings[0] == 5:
+					Log("Future prefix errors will not be logged.", 2)
+
 		something_excluded, tuple_list = None, []
-		prefix_warnings = 0
-		for line in filelist_fp:
-			if not line.strip(): continue # skip blanks
+		separator = Globals.null_separator and "\0" or "\n"
+		for line in filelist_fp.read().split(separator):
+			if not line: continue # skip blanks
 			try: tuple = self.filelist_parse_line(line, include)
 			except FilePrefixError, exc:
-				prefix_warnings += 1
-				if prefix_warnings < 6:
-					Log("Warning: file specification %s in filelist %s\n"
-						"doesn't start with correct prefix %s, ignoring." %
-						(exc, filelist_name, self.prefix), 2)
-					if prefix_warnings == 5:
-						Log("Future prefix errors will not be logged.", 2)
+				incr_warnings(exc)
+				continue
 			tuple_list.append(tuple)
 			if not tuple[1]: something_excluded = 1
 		if filelist_fp.close():
diff --git a/rdiff-backup/rdiff_backup/statistics.py b/rdiff-backup/rdiff_backup/statistics.py
index 7f99c31..4043f16 100644
--- a/rdiff-backup/rdiff_backup/statistics.py
+++ b/rdiff-backup/rdiff_backup/statistics.py
@@ -56,15 +56,17 @@ class StatsObj:
 		"""Add 1 to value of attribute"""
 		self.__dict__[attr] = self.get_stat(attr) + 1
 
-	def get_stats_line(self, index):
+	def get_stats_line(self, index, use_repr = 1):
 		"""Return one line abbreviated version of full stats string"""
 		file_attrs = map(lambda attr: str(self.get_stat(attr)),
 						 self.stat_file_attrs)
 		if not index: filename = "."
 		else:
-			# use repr to quote newlines in relative filename, then
-			# take of leading and trailing quote.
-			filename = repr(apply(os.path.join, index))[1:-1]
+			filename = apply(os.path.join, index)
+			if use_repr:
+				# use repr to quote newlines in relative filename, then
+				# take of leading and trailing quote.
+				filename = repr(filename)[1:-1]
 		return " ".join([filename,] + file_attrs)
 
 	def set_stats_from_line(self, line):
@@ -227,39 +229,45 @@ class StatsITR(IterTreeReducer, StatsObj):
 		"""
 		if mirror_dsrp.lstat():
 			self.mirror_base_exists = 1
-			self.mirror_base_size = mirror_dsrp.getsize()
+			self.mirror_base_size = self.stats_getsize(mirror_dsrp)
 		else: self.mirror_base_exists = None
 
+	def stats_getsize(self, rp):
+		"""Return size of rp, with error checking"""
+		try: return rp.getsize()
+		except KeyError: return 0
+
 	def end_stats(self, diff_rorp, mirror_dsrp, inc_rp = None):
 		"""Set various statistics after mirror processed"""
 		if mirror_dsrp.lstat():
+			source_size = self.stats_getsize(mirror_dsrp)
 			self.SourceFiles += 1
-			self.SourceFileSize += mirror_dsrp.getsize()
+			self.SourceFileSize += source_size
 			if self.mirror_base_exists:
 				self.MirrorFiles += 1
 				self.MirrorFileSize += self.mirror_base_size
 				if diff_rorp: # otherwise no change
 					self.ChangedFiles += 1
-					self.ChangedSourceSize += mirror_dsrp.getsize()
+					self.ChangedSourceSize += source_size
 					self.ChangedMirrorSize += self.mirror_base_size
-					if inc_rp:
-						self.IncrementFiles += 1
-						self.IncrementFileSize += inc_rp.getsize()
+					self.stats_incr_incfiles(inc_rp)
 			else: # new file was created
 				self.NewFiles += 1
-				self.NewFileSize += mirror_dsrp.getsize()
-				if inc_rp:
-					self.IncrementFiles += 1
-					self.IncrementFileSize += inc_rp.getsize()
+				self.NewFileSize += source_size
+				self.stats_incr_incfiles(inc_rp)
 		else:
 			if self.mirror_base_exists: # file was deleted from mirror
 				self.MirrorFiles += 1
 				self.MirrorFileSize += self.mirror_base_size
 				self.DeletedFiles += 1
 				self.DeletedFileSize += self.mirror_base_size
-				if inc_rp:
-					self.IncrementFiles += 1
-					self.IncrementFileSize += inc_rp.getsize()
+				self.stats_incr_incfiles(inc_rp)
+
+	def stats_incr_incfiles(self, inc_rp):
+		"""Increment IncrementFile statistics"""
+		if inc_rp:
+			self.IncrementFiles += 1
+			self.IncrementFileSize += self.stats_getsize(inc_rp)
 
 	def add_file_stats(self, subinstance):
 		"""Add all file statistics from subinstance to current totals"""
@@ -304,7 +312,9 @@ class Stats:
 
 	def write_dir_stats_line(cls, statobj, index):
 		"""Write info from statobj about rpath to statistics file"""
-		cls._dir_stats_fp.write(statobj.get_stats_line(index) +"\n")
+		if Globals.null_separator:
+			cls._dir_stats_fp.write(statobj.get_stats_line(index, None) + "\0")
+		else: cls._dir_stats_fp.write(statobj.get_stats_line(index) + "\n")
 
 	def close_dir_stats_file(cls):
 		"""Close directory statistics file if its open"""
diff --git a/rdiff-backup/src/globals.py b/rdiff-backup/src/globals.py
index 9d81ad9..0c10e48 100644
--- a/rdiff-backup/src/globals.py
+++ b/rdiff-backup/src/globals.py
@@ -147,6 +147,10 @@ class Globals:
 							"jpg|gif|png|jp2|mp3|ogg|avi|wmv|mpeg|mpg|rm|mov)$"
 	no_compression_regexp = None
 
+	# If true, filelists and directory statistics will be split on
+	# nulls instead of newlines.
+	null_separator = None
+
 	# Determines whether or not ssh will be run with the -C switch
 	ssh_compression = 1
 
diff --git a/rdiff-backup/src/increment.py b/rdiff-backup/src/increment.py
index 7aa7009..5836f63 100644
--- a/rdiff-backup/src/increment.py
+++ b/rdiff-backup/src/increment.py
@@ -219,6 +219,7 @@ class IncrementITR(ErrorITR, StatsITR):
 			# Write updated mirror to temp file so we can compute
 			# reverse diff locally
 			mirror_tf = TempFileManager.new(dsrp)
+			old_dsrp_tf = TempFileManager.new(dsrp)
 			def init_thunk():
 				if diff_rorp.isflaglinked():
 					Hardlink.link_rp(diff_rorp, mirror_tf, dsrp)
@@ -226,8 +227,16 @@ class IncrementITR(ErrorITR, StatsITR):
 													  mirror_tf).execute()
 				self.incrp = Inc.Increment_action(mirror_tf, dsrp,
 												  incpref).execute()
-			def final(init_val): mirror_tf.rename(dsrp)
-			def error(exc, ran_init, init_val): mirror_tf.delete()
+				if dsrp.lstat(): RPathStatic.rename(dsrp, old_dsrp_tf)
+				mirror_tf.rename(dsrp)
+
+			def final(init_val): old_dsrp_tf.delete()
+			def error(exc, ran_init, init_val):
+				if ran_init: old_dsrp_tf.delete() # everything is fine
+				else: # restore to previous state
+					if old_dsrp_tf.lstat(): old_dsrp_tf.rename(dsrp)
+					if self.incrp: self.incrp.delete()
+
 			RobustAction(init_thunk, final, error).execute()
 		else: self.incrp = Robust.chain(
 			Inc.Increment_action(diff_rorp, dsrp, incpref),
diff --git a/rdiff-backup/src/lazy.py b/rdiff-backup/src/lazy.py
index 98a4027..c7d1a52 100644
--- a/rdiff-backup/src/lazy.py
+++ b/rdiff-backup/src/lazy.py
@@ -54,7 +54,7 @@ class Iter:
 				return None
 		try: i2 = iter2.next()
 		except StopIteration: return 1
-		if verbose: print "End when i2 = %s" % i2
+		if verbose: print "End when i2 = %s" % (i2,)
 		return None
 
 	def Or(iter):
diff --git a/rdiff-backup/src/main.py b/rdiff-backup/src/main.py
index 6359450..f050435 100755
--- a/rdiff-backup/src/main.py
+++ b/rdiff-backup/src/main.py
@@ -33,9 +33,9 @@ class Main:
 			  "include-filelist=", "include-filelist-stdin",
 			  "include-regexp=", "list-increments", "mirror-only",
 			  "no-compression", "no-compression-regexp=",
-			  "no-hard-links", "no-resume", "parsable-output",
-			  "print-statistics", "quoting-char=", "remote-cmd=",
-			  "remote-schema=", "remove-older-than=",
+			  "no-hard-links", "no-resume", "null-separator",
+			  "parsable-output", "print-statistics", "quoting-char=",
+			  "remote-cmd=", "remote-schema=", "remove-older-than=",
 			  "restore-as-of=", "resume", "resume-window=", "server",
 			  "ssh-no-compression", "terminal-verbosity=",
 			  "test-server", "verbosity", "version", "windows-mode",
@@ -88,6 +88,7 @@ class Main:
 				Globals.set("no_compression_regexp_string", arg)
 			elif opt == "--no-hard-links": Globals.set('preserve_hardlinks', 0)
 			elif opt == '--no-resume': Globals.resume = 0
+			elif opt == "--null-separator": Globals.set("null_separator", 1)
 			elif opt == "-r" or opt == "--restore-as-of":
 				self.restore_timestr = arg
 				self.action = "restore-as-of"
diff --git a/rdiff-backup/src/selection.py b/rdiff-backup/src/selection.py
index 8b87af8..4c003c8 100644
--- a/rdiff-backup/src/selection.py
+++ b/rdiff-backup/src/selection.py
@@ -281,19 +281,25 @@ probably isn't what you meant.""" %
 
 	def filelist_read(self, filelist_fp, include, filelist_name):
 		"""Read filelist from fp, return (tuplelist, something_excluded)"""
+		prefix_warnings = [0]
+		def incr_warnings(exc):
+			"""Warn if prefix is incorrect"""
+			prefix_warnings[0] += 1
+			if prefix_warnings[0] < 6:
+				Log("Warning: file specification '%s' in filelist %s\n"
+					"doesn't start with correct prefix %s.  Ignoring." %
+					(exc, filelist_name, self.prefix), 2)
+				if prefix_warnings[0] == 5:
+					Log("Future prefix errors will not be logged.", 2)
+
 		something_excluded, tuple_list = None, []
-		prefix_warnings = 0
-		for line in filelist_fp:
-			if not line.strip(): continue # skip blanks
+		separator = Globals.null_separator and "\0" or "\n"
+		for line in filelist_fp.read().split(separator):
+			if not line: continue # skip blanks
 			try: tuple = self.filelist_parse_line(line, include)
 			except FilePrefixError, exc:
-				prefix_warnings += 1
-				if prefix_warnings < 6:
-					Log("Warning: file specification %s in filelist %s\n"
-						"doesn't start with correct prefix %s, ignoring." %
-						(exc, filelist_name, self.prefix), 2)
-					if prefix_warnings == 5:
-						Log("Future prefix errors will not be logged.", 2)
+				incr_warnings(exc)
+				continue
 			tuple_list.append(tuple)
 			if not tuple[1]: something_excluded = 1
 		if filelist_fp.close():
diff --git a/rdiff-backup/src/statistics.py b/rdiff-backup/src/statistics.py
index 7f99c31..4043f16 100644
--- a/rdiff-backup/src/statistics.py
+++ b/rdiff-backup/src/statistics.py
@@ -56,15 +56,17 @@ class StatsObj:
 		"""Add 1 to value of attribute"""
 		self.__dict__[attr] = self.get_stat(attr) + 1
 
-	def get_stats_line(self, index):
+	def get_stats_line(self, index, use_repr = 1):
 		"""Return one line abbreviated version of full stats string"""
 		file_attrs = map(lambda attr: str(self.get_stat(attr)),
 						 self.stat_file_attrs)
 		if not index: filename = "."
 		else:
-			# use repr to quote newlines in relative filename, then
-			# take of leading and trailing quote.
-			filename = repr(apply(os.path.join, index))[1:-1]
+			filename = apply(os.path.join, index)
+			if use_repr:
+				# use repr to quote newlines in relative filename, then
+				# take of leading and trailing quote.
+				filename = repr(filename)[1:-1]
 		return " ".join([filename,] + file_attrs)
 
 	def set_stats_from_line(self, line):
@@ -227,39 +229,45 @@ class StatsITR(IterTreeReducer, StatsObj):
 		"""
 		if mirror_dsrp.lstat():
 			self.mirror_base_exists = 1
-			self.mirror_base_size = mirror_dsrp.getsize()
+			self.mirror_base_size = self.stats_getsize(mirror_dsrp)
 		else: self.mirror_base_exists = None
 
+	def stats_getsize(self, rp):
+		"""Return size of rp, with error checking"""
+		try: return rp.getsize()
+		except KeyError: return 0
+
 	def end_stats(self, diff_rorp, mirror_dsrp, inc_rp = None):
 		"""Set various statistics after mirror processed"""
 		if mirror_dsrp.lstat():
+			source_size = self.stats_getsize(mirror_dsrp)
 			self.SourceFiles += 1
-			self.SourceFileSize += mirror_dsrp.getsize()
+			self.SourceFileSize += source_size
 			if self.mirror_base_exists:
 				self.MirrorFiles += 1
 				self.MirrorFileSize += self.mirror_base_size
 				if diff_rorp: # otherwise no change
 					self.ChangedFiles += 1
-					self.ChangedSourceSize += mirror_dsrp.getsize()
+					self.ChangedSourceSize += source_size
 					self.ChangedMirrorSize += self.mirror_base_size
-					if inc_rp:
-						self.IncrementFiles += 1
-						self.IncrementFileSize += inc_rp.getsize()
+					self.stats_incr_incfiles(inc_rp)
 			else: # new file was created
 				self.NewFiles += 1
-				self.NewFileSize += mirror_dsrp.getsize()
-				if inc_rp:
-					self.IncrementFiles += 1
-					self.IncrementFileSize += inc_rp.getsize()
+				self.NewFileSize += source_size
+				self.stats_incr_incfiles(inc_rp)
 		else:
 			if self.mirror_base_exists: # file was deleted from mirror
 				self.MirrorFiles += 1
 				self.MirrorFileSize += self.mirror_base_size
 				self.DeletedFiles += 1
 				self.DeletedFileSize += self.mirror_base_size
-				if inc_rp:
-					self.IncrementFiles += 1
-					self.IncrementFileSize += inc_rp.getsize()
+				self.stats_incr_incfiles(inc_rp)
+
+	def stats_incr_incfiles(self, inc_rp):
+		"""Increment IncrementFile statistics"""
+		if inc_rp:
+			self.IncrementFiles += 1
+			self.IncrementFileSize += self.stats_getsize(inc_rp)
 
 	def add_file_stats(self, subinstance):
 		"""Add all file statistics from subinstance to current totals"""
@@ -304,7 +312,9 @@ class Stats:
 
 	def write_dir_stats_line(cls, statobj, index):
 		"""Write info from statobj about rpath to statistics file"""
-		cls._dir_stats_fp.write(statobj.get_stats_line(index) +"\n")
+		if Globals.null_separator:
+			cls._dir_stats_fp.write(statobj.get_stats_line(index, None) + "\0")
+		else: cls._dir_stats_fp.write(statobj.get_stats_line(index) + "\n")
 
 	def close_dir_stats_file(cls):
 		"""Close directory statistics file if its open"""
diff --git a/rdiff-backup/testing/regressiontest.py b/rdiff-backup/testing/regressiontest.py
index 80a60d0..1a486d7 100644
--- a/rdiff-backup/testing/regressiontest.py
+++ b/rdiff-backup/testing/regressiontest.py
@@ -13,7 +13,7 @@ testfiles
 
 Globals.set('change_source_perms', 1)
 Globals.counter = 0
-Log.setverbosity(3)
+Log.setverbosity(7)
 
 class Local:
 	"""This is just a place to put increments relative to the local
diff --git a/rdiff-backup/testing/selectiontest.py b/rdiff-backup/testing/selectiontest.py
index aad2a68..a80830b 100644
--- a/rdiff-backup/testing/selectiontest.py
+++ b/rdiff-backup/testing/selectiontest.py
@@ -90,11 +90,29 @@ testfiles/select/3/3/2""")
 		assert sf(self.makeext("3/3")) == 1
 		assert sf(self.makeext("3/3/3")) == None
 
+	def testFilelistIncludeNullSep(self):
+		"""Test included filelist but with null_separator set"""
+		fp = StringIO.StringIO("""\0testfiles/select/1/2\0testfiles/select/1\0testfiles/select/1/2/3\0testfiles/select/3/3/2\0testfiles/select/hello\nthere\0""")
+		Globals.null_separator = 1
+		sf = self.Select.filelist_get_sf(fp, 1, "test")
+		assert sf(self.root) == 1
+		assert sf(self.makeext("1")) == 1
+		assert sf(self.makeext("1/1")) == None
+		assert sf(self.makeext("1/2/3")) == 1
+		assert sf(self.makeext("2/2")) == None
+		assert sf(self.makeext("3")) == 1
+		assert sf(self.makeext("3/3")) == 1
+		assert sf(self.makeext("3/3/3")) == None
+		assert sf(self.makeext("hello\nthere")) == 1
+		Globals.null_separator = 1
+
 	def testFilelistExclude(self):
 		"""Test included filelist"""
 		fp = StringIO.StringIO("""
 testfiles/select/1/2
 testfiles/select/1
+this is a badly formed line which should be ignored
+
 testfiles/select/1/2/3
 testfiles/select/3/3/2""")
 		sf = self.Select.filelist_get_sf(fp, 0, "test")