Working on the 2.1 rsync configure API. Beautifing code

default-rsync.lua

@@ -15,8 +15,13 @@
 --
 --~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
 
-if not default   then error('default not loaded'); end
+if not default then
-if default.rsync then error('default-rsync already loaded'); end
+	error('default not loaded')
+end
+
+if default.rsync then
+	error('default-rsync already loaded')
+end
 
 default.rsync = {
 	-----
@@ -52,7 +57,7 @@ default.rsync = {
 				end
 			end)
 		-- stores all filters with integer index
-		-- local filterI = inlet.getExcludes();
+		-- local filterI = inlet.getExcludes()
 		local filterI = {}
 		-- stores all filters with path index
 		local filterP = {}
@@ -89,10 +94,15 @@ default.rsync = {
 		log('Normal', 'Calling rsync with filter-list of new/modified files/dirs\n', filterS)
 		local config = inlet.getConfig()
 		local delete = nil
-		if config.delete then delete = { '--delete', '--ignore-errors' }; end
+
-		spawn(elist, config.rsyncBinary,
+		if config.delete then
+			delete = { '--delete', '--ignore-errors' }
+		end
+
+
+		spawn(elist, config.rsync.binary,
 			'<', filter0,
-			config.rsyncOpts,
+			config.rsync._computed,
 			'-r',
 			delete,
 			'--force',
@@ -111,13 +121,16 @@ default.rsync = {
 		local inlet = event.inlet
 		local excludes = inlet.getExcludes()
 		local delete = nil
-		if config.delete then delete = { '--delete', '--ignore-errors' }; end
+
+		if config.delete then
+			delete = { '--delete', '--ignore-errors' }
+		end
 
 		if #excludes == 0 then
 			log('Normal', 'recursive startup rsync: ', config.source, ' -> ', config.target)
-			spawn(event, config.rsyncBinary,
+			spawn(event, config.rsync.binary,
 				delete,
-				config.rsyncOpts,
+				config.rsync._computed,
 				'-r',
 				config.source,
 				config.target)
@@ -125,11 +138,11 @@ default.rsync = {
 			local exS = table.concat(excludes, '\n')
 			log('Normal', 'recursive startup rsync: ',config.source,
 				' -> ',config.target,' excluding\n',exS)
-			spawn(event, config.rsyncBinary,
+			spawn(event, config.rsync.binary,
 				'<', exS,
 				'--exclude-from=-',
 				delete,
-				config.rsyncOpts,
+				config.rsync._computed,
 				'-r',
 				config.source,
 				config.target)
@@ -145,9 +158,77 @@ default.rsync = {
 		end
 
 		if config.rsyncOps then
-			error('did you mean rsyncOpts with "t"?', 4)
+			error('"rsyncOps" is outdated please use the new rsync = { ... } syntax.', 4)
 		end
 
+		if config.rsyncOpts and config.rsync._extra then
+			error(
+				'"rsyncOpts" is outdated in favor of the new rsync = { ... } syntax\n"' +
+				'for which you provided the _extra attribute as well.\n"' +
+				'Please remove rsyncOpts from your config.',
+				4
+			)
+		end
+
+		if config.rsyncOpts then
+			log(
+				'Warn',
+				'"rsyncOpts" is outdated. Please use the new rsync = { ... } syntax."',
+				event.etype, '"'
+			)
+
+			config.rsync._extra = config.rsyncOpts
+
+			config.rsyncOpts = nil
+		end
+
+		if config.rsyncBinary and config.rsync.binary then
+			error(
+				'"rsyncBinary is outdated in favor of the new rsync = { ... } syntax\n"'+
+				'for which you provided the binary attribute as well.\n"' +
+				"Please remove rsyncBinary from your config.'",
+				4
+			)
+		end
+
+		if config.rsyncBinary then
+			log(
+				'Warn',
+				'"rsyncBinary" is outdated. Please use the new rsync = { ... } syntax."',
+				event.etype, '"'
+			)
+
+			config.rsync.binary = config.rsyncBinary
+
+			config.rsyncOpts = nil
+		end
+
+		-- checks if the _computed argument does not exist already
+		if config.rsync._computed then
+			error(
+				'please do not use the internal rsync._computed parameter',
+				4
+			)
+		end
+
+		-- computes the rsync arguments into one list
+		local rsync = config.rsync;
+		rsync._computed = { true }
+		local computed = rsync._computed
+		local shorts = { '-' }
+
+		if config.rsync._extra then
+			for k, v in ipairs( config.rsync._extra ) do
+				computed[k + 1] = v
+			end
+		end
+
+		if rsync.links then shorts[ #shorts ] = 'l'; end
+		if rsync.times then shorts[ #shorts ] = 't'; end
+		if rsync.protectArgs then  shorts[ #shorts ] = 's'; end
+
+		computed[1] = table.concat(shorts, '')
+
 		-- appends a / to target if not present
 		if string.sub(config.target, -1) ~= '/' then
 			config.target = config.target..'/'
@@ -163,15 +244,16 @@ default.rsync = {
 	--
 	delete = true,
 
-	-----
-	-- The rsync binary to be called.
-	--
-	rsyncBinary = '/usr/bin/rsync',
-
 	-----
 	-- Calls rsync with this default short opts.
 	--
-	rsyncOpts = '-lts',
+	rsync = {
+		-- The rsync binary to be called.
+		binary = '/usr/bin/rsync',
+		links = true,
+		times = true,
+		protectArgs = true
+	},
 
 	-----
 	-- Exit codes for rsync.

lsyncd.lua

@@ -13,9 +13,9 @@
 -- require('profiler')
 -- profiler.start()
 
------
+--
 -- A security measurement.
--- Core will exit if version ids mismatch.
+-- The core will exit if version ids mismatch.
 --
 if lsyncd_version then
 	-- checks if the runner is being loaded twice
@@ -24,43 +24,48 @@ if lsyncd_version then
 end
 lsyncd_version = '2.0.7'
 
------
+--
--- Hides the core interface from user scripts
+-- Hides the core interface from user scripts.
 --
 local _l = lsyncd
 lsyncd = nil
 local lsyncd = _l
 _l = nil
 
------
+--
 -- Shortcuts (which user is supposed to be able to use them as well)
 --
 log       = lsyncd.log
 terminate = lsyncd.terminate
 now       = lsyncd.now
 readdir   = lsyncd.readdir
--- just to safe from userscripts changing this.
+
+--
+-- Coping globals to ensure userscripts don't change this.
+--
 local log       = log
 local terminate = terminate
 local now       = now
 
-------
+--
 -- Predeclarations
 --
 local Monitors
 
------
+--
 -- Global: total number of processess running
+--
 local processCount = 0
 
 --============================================================================
 -- Lsyncd Prototypes
 --============================================================================
 
------
+--
--- The array objects are tables that error if accessed with a non-number.
+-- Array tables error if accessed with a non-number.
 --
 local Array = ( function( )
+
 	-- Metatable
 	local mt = { }
 
@@ -89,25 +94,32 @@ local Array = (function()
 
 	-- objects public interface
 	return { new = new }
+
 end )( )
 
 
------
+--
--- The count array objects are tables that error if accessed with a non-number.
+-- Count array tables error if accessed with a non-number.
--- Additionally they maintain their length as 'size' attribute.
+--
--- Lua's # operator does not work on tables which key values are not
+-- Additionally they maintain their length as 'size' attribute,
+-- since Lua's # operator does not work on tables whose key values are not
 -- strictly linear.
 --
 local CountArray = ( function( )
+
+	--
 	-- Metatable
+	--
 	local mt = { }
 
-	-----
+	--
-	-- key to native table
+	-- Key to native table
+	--
 	local k_nt = { }
 
-	-----
+	--
-	-- on accessing a nil index.
+	-- On accessing a nil index.
+	--
 	mt.__index = function( t, k )
 		if type( k ) ~= 'number' then
 			error( 'Key "'..k..'" invalid for CountArray', 2 )
@@ -115,12 +127,15 @@ local CountArray = (function()
 		return t[ k_nt ][ k ]
 	end
 
-	-----
+	--
-	-- on assigning a new index.
+	-- On assigning a new index.
+	--
 	mt.__newindex = function( t, k, v )
+
 		if type(k) ~= 'number' then
 			error( 'Key "'..k..'" invalid for CountArray', 2 )
 		end
+
 		-- value before
 		local vb = t[ k_nt ][ k ]
 		if v and not vb then
@@ -131,57 +146,69 @@ local CountArray = (function()
 		t[ k_nt ][ k ] = v
 	end
 
-	-----
+	--
 	-- Walks through all entries in any order.
 	--
 	local function walk( self )
 		return pairs( self[ k_nt ] )
 	end
 
-	-----
+	--
-	-- returns the count
+	-- Returns the count
 	--
 	local function size( self )
 		return self._size
 	end
 
-	-----
+	--
-	-- creates a new count array
+	-- Creates a new count array
 	--
 	local function new( )
+
 		-- k_nt is native table, private for this object.
-		local o = {_size = 0, walk = walk, size = size, [k_nt] = {} }
+		local o = {
+			_size = 0,
+			walk = walk,
+			size = size,
+			[k_nt] = { }
+		}
+
 		setmetatable(o, mt)
 		return o
 	end
 
-	-----
+	--
-	-- public interface
+	-- Public interface
 	--
 	return { new = new }
 end )( )
 
------
--- Queue
---   optimized for pushing on the right and poping on the left.
 --
+-- A queue is optimized for pushing on the right and poping on the left.
 --
 Queue = ( function( )
-	-----
+
+	--
 	-- Creates a new queue.
 	--
 	local function new( )
-		return { first = 1, last = 0, size = 0};
+		return {
+			first = 1,
+			last  = 0,
+			size  = 0
+		};
 	end
 
-	-----
+	--
 	-- Pushes a value on the queue.
 	-- Returns the last value
 	--
 	local function push( list, value )
+
 		if not value then
 			error('Queue pushing nil value', 2)
 		end
+
 		local last = list.last + 1
 		list.last = last
 		list[ last ] = value
@@ -189,212 +216,295 @@ Queue = (function()
 		return last
 	end
 
-	-----
+	--
-	-- Removes item at pos from Queue.
+	-- Removes an item at pos from the Queue.
 	--
 	local function remove( list, pos )
+
 		if list[ pos ] == nil then
 			error('Removing nonexisting item in Queue', 2)
 		end
+
 		list[ pos ] = nil
 
-		-- if removing first element, move list on.
+		-- if removing first or last element,
+		-- the queue limits are adjusted.
 		if pos == list.first then
+
 			local last = list.last
+
 			while list[ pos ] == nil and pos <= list.last do
 				pos = pos + 1
 			end
+
 			list.first = pos
+
 		elseif pos == list.last then
+
 			while list[ pos ] == nil and pos >= list.first do
 				pos = pos - 1
 			end
+
 			list.last = pos
+
 		end
 
-		-- reset indizies if list is empty
+		-- reset the indizies if the queue is empty
 		if list.last < list.first then
 			list.first = 1
 			list.last  = 0
 		end
+
 		list.size = list.size - 1
 	end
 
-	-----
+	--
 	-- Queue iterator (stateless)
 	--
 	local function iter( list, pos )
+
 		pos = pos + 1
+
 		while list[ pos ] == nil and pos <= list.last do
 			pos = pos + 1
 		end
+
 		if pos > list.last then
 			return nil
 		end
+
 		return pos, list[ pos ]
 	end
 
-	-----
+	--
-	-- Reverse queue iterator. (stateless)
+	-- Reverse queue iterator (stateless)
 	--
 	local function iterReverse( list, pos )
+
 		pos = pos - 1
+
 		while list[pos] == nil and pos >= list.first do
 			pos = pos - 1
 		end
+
 		if pos < list.first then
 			return nil
 		end
+
 		return pos, list[ pos ]
 	end
 
-	-----
+	--
 	-- Iteraters through the queue
-	-- returning all non-nil pos-value entries
+	-- returning all non-nil pos-value entries.
 	--
 	local function qpairs( list )
 		return iter, list, list.first - 1
 	end
 
-	-----
+	--
 	-- Iteraters backwards through the queue
-	-- returning all non-nil pos-value entries
+	-- returning all non-nil pos-value entries.
 	--
 	local function qpairsReverse( list )
 		return iterReverse, list, list.last + 1
 	end
 
-	return {new = new,
+	return {
+		new = new,
 		push = push,
 		remove = remove,
 		qpairs = qpairs,
-			qpairsReverse = qpairsReverse}
+		qpairsReverse = qpairsReverse
+	}
 end )( )
 
------
+--
 -- Locks globals,
--- no more globals can be created
+-- No more globals can be created after this
 --
 local function lockGlobals( )
+
 	local t = _G
 	local mt = getmetatable( t ) or { }
+
+	-- TODO try to remove the underscore exceptions
 	mt.__index = function( t, k )
-		if (k~='_' and string.sub(k, 1, 2) ~= '__') then
+		if k ~= '_' and string.sub(k, 1, 2) ~= '__' then
 			error( 'Access of non-existing global "'..k..'"', 2 )
 		else
 			rawget( t, k )
 		end
 	end
+
 	mt.__newindex = function( t, k, v )
-		if (k~='_' and string.sub(k, 1, 2) ~= '__') then
+		if k ~= '_' and string.sub( k, 1, 2 ) ~= '__' then
 			error('Lsyncd does not allow GLOBALS to be created on the fly. '..
 			      'Declare "'..k..'" local or declare global on load.', 2)
 		else
 			rawset( t, k, v )
 		end
 	end
+
 	setmetatable( t, mt )
 end
 
------
+--
--- Holds information about a delayed event of one Sync.
+-- Holds the information about a delayed event for one Sync.
 --
 local Delay = ( function( )
-	-----
+
+	--
 	-- Creates a new delay.
 	--
-	-- @params see below
+	-- Params see below.
 	--
 	local function new( etype, sync, alarm, path, path2 )
+
 		local o = {
-			-----
+			--
 			-- Type of event.
 			-- Can be 'Create', 'Modify', 'Attrib', 'Delete' and 'Move'
+			--
 			etype = etype,
 
-			------
+			--
-			-- Sync this delay belongs to
+			-- the Sync this delay belongs to
+			--
 			sync = sync,
 
-			-----
+			--
 			-- Latest point in time this should be catered for.
 			-- This value is in kernel ticks, return of the C's
 			-- times(NULL) call.
 			alarm = alarm,
 
-			-----
+			--
-			-- path and filename or dirname of the delay relative
+			-- Path and filename or dirname of the delay relative
 			-- to the syncs root.
+			--
 			-- for the directories it contains a trailing slash
 			--
 			path = path,
 
-			------
+			--
-			-- only not nil for 'Move's.
+			-- Used only for Moves.
-			-- path and file/dirname of a move destination.
+			-- Path and file/dirname of a move destination.
+			--
 			path2  = path2,
 
-			------
+			--
 			-- Status of the event. Valid stati are:
+			--
 			-- 'wait'    ... the event is ready to be handled.
+			--
 			-- 'active'  ... there is process running catering for this event.
+			--
 			-- 'blocked' ... this event waits for another to be handled first.
+			--
 			-- 'done'    ... event has been collected. This should never be
 			--               visible as all references should be droped on
-			--               collection, nevertheless seperat status for
+			--               collection, nevertheless the seperate status is
-			--               insurrance.
+			--               used as insurrance everything is running correctly.
 			status = 'wait',
 
-			-----
+			--
 			-- Position in the queue
+			--
 			dpos = -1,
 		}
+
 		return o
 	end
 
-	-- public interface
+	--
+	-- Public interface
+	--
 	return { new = new }
+
 end )( )
 
------
+--
--- combines delays
+-- Combines delays
 --
 local Combiner = ( function( )
 
-	----
+	--
-	-- new delay absorbed by old
+	-- The new delay is absorbed by an older one.
 	--
 	local function abso( d1, d2 )
-		log('Delay',d2.etype,':',d2.path,' absorbed by ',d1.etype,':',d1.path)
+
+		log(
+			'Delay',
+			d2.etype, ':',d2.path,
+			' absorbed by ',
+			d1.etype,':',d1.path
+		)
+
 		return 'absorb'
+
 	end
 
-	----
+	--
-	-- new delay replaces the old one if it is a file
+	-- The new delay replaces the old one if it's a file
 	--
 	local function refi( d1, d2 )
+
+		-- but a directory blocks
 		if d2.path:byte( -1 ) == 47 then
-			log('Delay',d2.etype,':',d2.path,' blocked by ',d1.etype,':',d1.path)
+
+			log(
+				'Delay',
+				d2.etype,':',d2.path,
+				' blocked by ',
+				d1.etype,':',d1.path
+			)
+
 			return 'stack'
-		end
+
-		log('Delay',d2.etype,':',d2.path,' replaces ',d1.etype,':',d1.path)
-		return 'replace'
 		end
 
-	----
+		log(
-	-- new delay replaces the old one
+			'Delay',
+			d2.etype,':',d2.path,
+			' replaces ',
+			d1.etype,':',d1.path
+		)
+
+		return 'replace'
+
+	end
+
+	--
+	-- The new delay replaces an older one.
 	--
 	local function repl( d1, d2 )
-		log('Delay',d2.etype,':',d2.path,' replaces ',d1.etype,':',d1.path)
+
+		log(
+			'Delay',
+			d2.etype,':',d2.path,
+			' replaces ',
+			d1.etype,':',d1.path
+		)
+
 		return 'replace'
+
 	end
 
-	----
+	--
-	-- delays nullificate each other
+	-- Two delays nullificate each other.
 	--
 	local function null( d1, d2 )
-		log('Delay',d2.etype,':',d2.path,' nullifies ',d1.etype,':',d1.path)
+
+		log(
+			'Delay',
+			d2.etype,':',d2.path,
+			' nullifies ',
+			d1.etype,':',d1.path
+		)
+
 		return 'remove'
+
 	end
 
 	-----
@@ -407,8 +517,8 @@ local Combiner = (function()
 		Delete = { Attrib = abso, Modify = abso, Create = refi, Delete = abso },
 	}
 
-	------
+	--
-	-- combines two delays
+	-- Combines two delays
 	--
 	local function combine(d1, d2)
 		if d1.etype == 'Init' or d1.etype == 'Blanket' then