Added --null-separator, tweaked increment error handling

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

trunk/rdiff-backup/CHANGELOG

+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)
 --------------------------
 

trunk/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.

trunk/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),

trunk/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):

trunk/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():

trunk/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"""

trunk/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
 

trunk/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),

trunk/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):

trunk/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"

trunk/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():

trunk/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"""

trunk/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

trunk/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")