From ad39f8ebdc5d1e31f9c01ae6973ef521a9679fbe Mon Sep 17 00:00:00 2001 From: Tobias Ulmer Date: Fri, 12 Jul 2013 15:51:54 +0200 Subject: [PATCH] Add error handling to tools module and all call sites Signed-off-by: Tobias Ulmer --- generic/e2lib.lua | 161 ++++++++++++++++++++++++---------------- generic/e2option.lua | 3 +- generic/generic_git.lua | 5 +- generic/tools.lua | 125 ++++++++++++++++--------------- generic/transport.lua | 21 +++++- local/e2-help.lua | 7 +- local/e2build.lua | 41 ++++++---- local/e2tool.lua | 5 +- plugins/cvs.lua | 44 +++++++++-- plugins/files.lua | 5 +- plugins/git.lua | 16 ++-- 11 files changed, 274 insertions(+), 159 deletions(-) diff --git a/generic/e2lib.lua b/generic/e2lib.lua index a2a0848..e2decd0 100644 --- a/generic/e2lib.lua +++ b/generic/e2lib.lua @@ -233,11 +233,12 @@ end -- @return True on success, false on error. -- @return Error object on failure. function e2lib.init2() - local rc, re - local e = err.new("initializing globals (step2)") + local rc, re, e, config, ssh, host_system_arch + + e = err.new("initializing globals (step2)") -- get the global configuration - local config, re = e2lib.get_global_config() + config, re = e2lib.get_global_config() if not config then return false, re end @@ -245,26 +246,31 @@ function e2lib.init2() -- honour tool customizations from the config file if config.tools then for k,v in pairs(config.tools) do - tools.set_tool(k, v.name, v.flags) + rc, re = tools.set_tool(k, v.name, v.flags) + if not rc then + return false, e:cat(re) + end end end -- handle E2_SSH environment setting - local ssh = nil - ssh = e2lib.globals.osenv["E2_SSH"] + ssh = e2lib.globals.osenv["E2_SSH"] if ssh then e2lib.logf(3, "using E2_SSH environment variable: %s", ssh) - tools.set_tool("ssh", ssh) + rc, re = tools.set_tool("ssh", ssh) + if not rc then + return false, e:cat(re) + end end -- initialize the tools library after resetting tools - local rc, re = tools.init() + rc, re = tools.init() if not rc then return false, e:cat(re) end -- get host system architecture - local host_system_arch, re = e2lib.get_sys_arch() + host_system_arch, re = e2lib.get_sys_arch() if not host_system_arch then return false, e:cat(re) end @@ -1555,19 +1561,24 @@ end -- @return bool -- @return string: the last line ouf captured output function e2lib.call_tool(tool, args) - local cmd = tools.get_tool(tool) + local rc, re, cmd, flags, call + + cmd, re = tools.get_tool(tool) if not cmd then - e2lib.bomb("trying to call invalid tool: " .. tostring(tool)) + return false, re end - local flags = tools.get_tool_flags(tool) + + flags, re = tools.get_tool_flags(tool) if not flags then - e2lib.bomb("invalid tool flags for tool: " .. tostring(tool)) + return false, re end - local call = string.format("%s %s %s", cmd, flags, args) - local rc, e = e2lib.callcmd_log(call) + + call = string.format("%s %s %s", cmd, flags, args) + rc, re = e2lib.callcmd_log(call) if rc ~= 0 then - return false, e + return false, re end + return true end @@ -1577,28 +1588,31 @@ end -- @return bool -- @return string: the last line ouf captured output function e2lib.call_tool_argv(tool, argv) - local cmd = tools.get_tool(tool) + local rc, re, cmd, flags, call + + cmd, re = tools.get_tool(tool) if not cmd then - e2lib.bomb("trying to call invalid tool: " .. tostring(tool)) + return false, re end - local flags = tools.get_tool_flags(tool) + + flags, re = tools.get_tool_flags(tool) if not flags then - e2lib.bomb("invalid tool flags for tool: " .. tostring(tool)) + return false, re end - -- TODO: flags should be quoted as well, requires config changes - local call = string.format("%s %s", e2lib.shquote(cmd), flags) + call = string.format("%s %s", e2lib.shquote(cmd), flags) for _,arg in ipairs(argv) do assert(type(arg) == "string") call = call .. " " .. e2lib.shquote(arg) end - local rc, e = e2lib.callcmd_log(call) + rc, re = e2lib.callcmd_log(call) if rc ~= 0 then - return false, e + return false, re end - return true, e + + return true end --- call a tool with argv and capture output @@ -1608,27 +1622,30 @@ end -- @return bool -- @return string: the last line ouf captured output function e2lib.call_tool_argv_capture(tool, argv, capturefn) - local cmd = tools.get_tool(tool) + local rc, re, cmd, flags, call + + cmd, re = tools.get_tool(tool) if not cmd then - e2lib.bomb("trying to call invalid tool: " .. tostring(tool)) + return false, re end - local flags = tools.get_tool_flags(tool) + + flags, re = tools.get_tool_flags(tool) if not flags then - e2lib.bomb("invalid tool flags for tool: " .. tostring(tool)) + return false, re end - -- TODO: flags should be quoted as well, requires config changes - local call = string.format("%s %s", e2lib.shquote(cmd), flags) + call = string.format("%s %s", e2lib.shquote(cmd), flags) for _,arg in ipairs(argv) do assert(type(arg) == "string") call = call .. " " .. e2lib.shquote(arg) end - local rc, e = e2lib.callcmd_capture(call, capturefn) + rc, re = e2lib.callcmd_capture(call, capturefn) if rc ~= 0 then - return false, e + return false, re end + return true end @@ -1639,26 +1656,31 @@ end -- @return bool -- @return an error object on failure function e2lib.git(gitdir, subtool, args) - local rc, re - local e = err.new("calling git failed") + local rc, re, e, git, call + + e = err.new("calling git failed") + if not gitdir then gitdir = ".git" end + if not args then args = "" end - local git, re = tools.get_tool("git") + + git, re = tools.get_tool("git") if not git then return false, e:cat(re) end - -- TODO: args should be quoted as well - local call = string.format("GIT_DIR=%s %s %s %s", - e2lib.shquote(gitdir), e2lib.shquote(git), e2lib.shquote(subtool), args) + + call = string.format("GIT_DIR=%s %s %s %s", + e2lib.shquote(gitdir), e2lib.shquote(git), e2lib.shquote(subtool), args) rc, re = e2lib.callcmd_log(call) if rc ~= 0 then return false, e:cat(re) end - return true, e + + return true end function e2lib.git_argv(argv) @@ -1823,36 +1845,38 @@ end -- @return string: sha1 sum of file -- @return an error object on failure function e2lib.sha1sum(path) + local rc, re, e, sha1sum, sha1sum_flags, cmd, p, msg, out, sha1, file + assert(type(path) == "string") - local e = err.new("calculating SHA1 checksum failed") + e = err.new("calculating SHA1 checksum failed") - local sha1sum, re = tools.get_tool("sha1sum") + sha1sum, re = tools.get_tool("sha1sum") if not sha1sum then - return nil, e:cat(re) + return false , e:cat(re) end - local sha1sum_flags, re = tools.get_tool_flags("sha1sum") + sha1sum_flags, re = tools.get_tool_flags("sha1sum") if not sha1sum_flags then - return nil, e:cat(re) + return false, e:cat(re) end - -- TODO: sha1sum_flags should be quoted as well - local cmd = string.format("%s %s %s", e2lib.shquote(sha1sum), sha1sum_flags, - e2lib.shquote(path)) + cmd = string.format("%s %s %s", e2lib.shquote(sha1sum), sha1sum_flags, + e2lib.shquote(path)) - local p, msg = io.popen(cmd, "r") + p, msg = io.popen(cmd, "r") if not p then - return nil, e:cat(msg) + return false, e:cat(msg) end - local out, msg = p:read("*l") + out, msg = p:read("*l") p:close() - local sha1, file = out:match("(%S+) (%S+)") + sha1, file = out:match("(%S+) (%S+)") if type(sha1) ~= "string" then - return nil, e:cat("parsing sha1sum output failed") + return false, e:cat("parsing sha1sum output failed") end + return sha1 end @@ -1878,23 +1902,32 @@ end -- @return string: machine hardware name -- @return an error object on failure function e2lib.get_sys_arch() - local rc, re - local e = err.new("getting host system architecture failed") - local uname = tools.get_tool("uname") - local cmd = string.format("%s -m", e2lib.shquote(uname)) - local p, msg = io.popen(cmd, "r") + local rc, re, e, uname, cmd, p, msg, l, arch + + e = err.new("getting host system architecture failed") + + uname, re = tools.get_tool("uname") + if not uname then + return false, e:cat(re) + end + + cmd = string.format("%s -m", e2lib.shquote(uname)) + p, msg = io.popen(cmd, "r") if not p then - return nil, e:cat(msg) + return false, e:cat(msg) end - local l, msg = p:read() + + l, msg = p:read() if not l then - return nil, e:cat(msg) + return false, e:cat(msg) end - local arch = l:match("(%S+)") + + arch = l:match("(%S+)") if not arch then - return nil, e:append("%s: %s: cannot parse", cmd, l) + return false, e:append("%s: %s: cannot parse", cmd, l) end - return arch, nil + + return arch end --- return a table of parent directories diff --git a/generic/e2option.lua b/generic/e2option.lua index 9155ce9..29a5819 100644 --- a/generic/e2option.lua +++ b/generic/e2option.lua @@ -460,11 +460,12 @@ function e2option.showtoolmanpage() local cmd = {} for _,s in ipairs({"man"}) do local viewer, viewerflags + viewer = tools.get_tool(s) viewerflags = tools.get_tool_flags(s) if viewer then table.insert(cmd, e2lib.shquote(viewer)) - if viewerflags ~= "" then + if viewerflags and viewerflags ~= "" then table.insert(cmd, viewerflags) end diff --git a/generic/generic_git.lua b/generic/generic_git.lua index d8fa22e..1540b9f 100644 --- a/generic/generic_git.lua +++ b/generic/generic_git.lua @@ -195,7 +195,10 @@ function generic_git.git_init_db1(rurl) if u.transport == "ssh" or u.transport == "scp" or u.transport == "rsync+ssh" then - local ssh = tools.get_tool("ssh") + local ssh, re = tools.get_tool("ssh") + if not ssh then + return false, re + end cmd = string.format("%s %s %s", e2lib.shquote(ssh), e2lib.shquote(u.server), e2lib.shquote(gitcmd)) elseif u.transport == "file" then diff --git a/generic/tools.lua b/generic/tools.lua index dca59df..bb01da4 100644 --- a/generic/tools.lua +++ b/generic/tools.lua @@ -68,47 +68,48 @@ local toollist = { flags = "", optional = false }, } ---- get a tool command --- @param name string: the tool name --- @return string: the tool command, nil on error +--- Get a absolute tool command. +-- @param name Tool name (string). +-- @return Tool command or false on error. +-- @return Error object on failure. function tools.get_tool(name) if not toollist[name] then - e2lib.bomb("looking up invalid tool: " .. tostring(name)) + return false, err.new("tool '%s' is not registered in tool list", name) end return toollist[name].path end ---- get tool flags --- @param name string: the tool name --- @return string: the tool flags +--- Get tool flags. +-- @param name Tool name (string). +-- @return Tool flags as a string, potentially empty - or false on error. +-- @return Error object on failure. function tools.get_tool_flags(name) if not toollist[name] then - e2lib.bomb("looking up flags for invalid tool: " .. - tostring(name)) + return false, err.new("tool '%s' is not registered in tool list", name) end return toollist[name].flags or "" end --- Get tool name. --- @param name tool name (string) --- @return Tool name field (string) used to find tool in PATH. +-- @param name Tool name (string). +-- @return Tool name field (string) used to find tool in PATH or false on error. +-- @return Error object on failure. function tools.get_tool_name(name) if not toollist[name] then - e2lib.bomb("looking up flags for invalid tool: " .. - tostring(name)) + return false, err.new("tool '%s' is not registered in tool list", name) end return toollist[name].name end ---- set a tool command and flags --- @param name string: the tool name --- @param value string: the new tool command --- @param flags string: the new tool flags. Optional. --- @return bool --- @return nil, an error string on error +--- Set a tool command and flags. +-- @param name Tool name (string). +-- @param value Tool command (string). May also be an absolute command. +-- @param flags Tool flags (string). Optional. +-- @return True on success, false on error. +-- @return Error object on failure. function tools.set_tool(name, value, flags) if not toollist[name] then - return false, "invalid tool setting" + return false, err.new("tool '%s' is not registered in tool list", name) end if type(value) == "string" then toollist[name].name = value @@ -118,27 +119,26 @@ function tools.set_tool(name, value, flags) end e2lib.logf(3, "setting tool: %s=%s flags=%s", name, toollist[name].name, toollist[name].flags) - return true, nil + return true end ---- add a new tool --- @param name string: the tool name --- @param value string: the new tool command --- @param flags string: the new tool flags. --- @param optional bool: wheter the tool is optional or not --- @return bool --- @return nil, an error string on error +--- Add a new tool. +-- @param name Tool name (string). +-- @param value Tool command, may contain absolute path (string). +-- @param flags Tool flags (string). May be empty. +-- @param optional Whether the tool is required (true) or optional (false). +-- @return True on success, false on error. +-- @return Error object on failure. function tools.add_tool(name, value, flags, optional) if toollist[name] then - e2lib.bomb("trying to add a tool that already exists: " .. - tostring(name)) + return false, err.new("tool '%s' already registered in tool list", name) end if type(name) ~= "string" or type(value) ~= "string" or type(flags) ~= "string" or type(optional) ~= "boolean" then - print("error in add_tool") - e2lib.bomb("one or more parameters wrong while adding tool " .. - tostring(name)) + return false, + err.new("one or more parameters wrong while adding tool %s", + tostring(name)) end toollist[name] = { @@ -151,53 +151,60 @@ function tools.add_tool(name, value, flags, optional) e2lib.logf(3, "adding tool: %s=%s flags=%s optional=%s", name, t.name, t.flags, tostring(t.optional)) - return true, nil + return true end ---- check if a tool is available +--- Check if a tool is available. -- @param name string a valid tool name --- @return bool --- @return nil, an error string on error +-- @return True if tool exists, otherwise false. False may also indicate an +-- error, if the second return value is not nil. +-- @return Error object on failure. function tools.check_tool(name) - local tool = toollist[name] + local tool, which, p + if not toollist[name] then + return false, err.new("tool '%s' is not registered in tool list", name) + end + + tool = toollist[name] if not tool.path then - local which = string.format("which \"%s\"", tool.name) - local p = io.popen(which, "r") + which = string.format("which \"%s\"", tool.name) + p = io.popen(which, "r") tool.path = p:read() p:close() if not tool.path then - e2lib.logf(3, "tool not available: %s", tool.name) - return false, "tool not available" + return false end end - e2lib.logf(4, "tool available: %s (%s)", tool.name, tool.path) - return true end ---- initialize the library --- @return bool +--- Initialize the tools library. Must be called before the tools library can +-- be used. Logs a warning about missing optional tools. +-- @return True on success (all required tools have been found), false on error. +-- @return Error object on failure. function tools.init() - local error = false - for tool,t in pairs(toollist) do - local rc = tools.check_tool(tool) + local rc, re + + for tool, t in pairs(toollist) do + rc, re = tools.check_tool(tool) + if not rc and re then + return false, re + end if not rc then - local warn = "Warning" - if not t.optional then - error = true - warn = "Error" + if t.optional then + e2lib.warnf("optional tool is not available: %s", tool) + else + return false, err.new("required tool is missing: %s", tool) end - e2lib.logf(1, "%s: tool is not available: %s", warn, tool) end end - if error then - return false, "missing mandatory tools" - end + initialized = true - return true, nil + + return true end ---- Check whether the tools library is initialized. +--- Check whether the tools library is initialized. There is no error condition. -- @return True or false. function tools.isinitialized() return initialized diff --git a/generic/transport.lua b/generic/transport.lua index ed2ea64..be7dc07 100644 --- a/generic/transport.lua +++ b/generic/transport.lua @@ -56,9 +56,26 @@ local function rsync_ssh(opts, src, dest) table.insert(argv, "-L") -- copy symlinks as real files table.insert(argv, "-k") -- copy dirlinks as directories - local rsh = tools.get_tool("ssh") .. " " .. tools.get_tool_flags("ssh") - table.insert(argv, "--rsh=" .. rsh) + local rsh, rshflags, re + rsh, re = tools.get_tool("ssh") + if not rsh then + return false, re + end + + rsh = string.format("--rsh=%s", rsh) + + rshflags, re = tools.get_tool_flags("ssh") + if not rshflags then + return false, re + end + + if rshflags ~= "" then + rsh = string.format("%s %s", rsh, rshflags) + end + + + table.insert(argv, rsh) table.insert(argv, src) table.insert(argv, dest) diff --git a/local/e2-help.lua b/local/e2-help.lua index cf9c25a..ea5e55b 100644 --- a/local/e2-help.lua +++ b/local/e2-help.lua @@ -174,12 +174,12 @@ local function display_man_page(doc) local viewer = tools.get_tool("man") if not viewer then - return false, err.new("Manual page viewer is not available") + return false, err.new("no man page viewer is available") end table.insert(cmd, e2lib.shquote(viewer)) local viewerflags = tools.get_tool_flags("man") - if viewerflags ~= "" then + if viewerflags and viewerflags ~= "" then table.insert(cmd, viewerflags) end @@ -188,7 +188,8 @@ local function display_man_page(doc) rc = os.execute(table.concat(cmd, ' ')) rc = rc / 256 if rc ~= 0 then - return false, err.new("Manual viewer terminated with exit code %d", rc) + return false, + err.new("man page viewer terminated with exit code %d", rc) end return true diff --git a/local/e2build.lua b/local/e2build.lua index 18a774c..211b331 100644 --- a/local/e2build.lua +++ b/local/e2build.lua @@ -335,26 +335,36 @@ local function setup_chroot(info, r, return_flags) return true, nil end +--- Enter playground. +-- @param info +-- @param r +-- @param chroot_command (optional) +-- @return True on success, false on error. +-- @return Error object on failure. function e2build.enter_playground(info, r, chroot_command) + local rc, re, e, res, e2_su, cmd + if not chroot_command then chroot_command = "/bin/bash" end - local res = info.results[r] - local rc, re - local e = err.new("entering playground") - e2lib.logf(4, "entering playground for %s ...", r) - local term = e2lib.globals.terminal - local e2_su = tools.get_tool("e2-su-2.2") - local cmd = string.format("%s %s chroot_2_3 %s %s", - res.build_config.chroot_call_prefix, - e2lib.shquote(e2_su), - e2lib.shquote(res.build_config.base), - chroot_command) + + res = info.results[r] + e = err.new("entering playground") + + e2_su = tools.get_tool("e2-su-2.2") + if not e2_su then + return false, e:cat(re) + end + + cmd = string.format("%s %s chroot_2_3 '%s' %s", + res.build_config.chroot_call_prefix, e2_su, + res.build_config.base, chroot_command) e2tool.set_umask(info) + -- return code depends on user commands. Ignore. os.execute(cmd) e2tool.reset_umask(info) - -- return code depends on user commands. Ignore. - return true, nil + + return true end local function fix_permissions(info, r, return_flags) @@ -401,7 +411,10 @@ local function runbuild(info, r, return_flags) e2lib.shquote(res.build_config.Tc), e2lib.shquote(res.build_config.scriptdir), e2lib.shquote(res.build_config.build_driver_file)) - local e2_su = tools.get_tool("e2-su-2.2") + local e2_su, re = tools.get_tool("e2-su-2.2") + if not e2_su then + return false, e:cat(re) + end local cmd = string.format("%s %s chroot_2_3 %s %s", res.build_config.chroot_call_prefix, e2lib.shquote(e2_su), diff --git a/local/e2tool.lua b/local/e2tool.lua index 5ac452c..0975efa 100644 --- a/local/e2tool.lua +++ b/local/e2tool.lua @@ -2048,7 +2048,10 @@ local function verify_remote_fileid(info, file, fileid) if u.transport == "ssh" or u.transport == "scp" or u.transport == "rsync+ssh" then local cmd = "sha1sum" - local ssh = tools.get_tool("ssh") + local ssh, re = tools.get_tool("ssh") + if not ssh then + return false, e:cat(re) + end local retcmd = string.format("%s %s ", e2lib.shquote(ssh), e2lib.shquote(u.server)) diff --git a/plugins/cvs.lua b/plugins/cvs.lua index ba3357a..669fff1 100644 --- a/plugins/cvs.lua +++ b/plugins/cvs.lua @@ -168,9 +168,18 @@ function cvs.fetch_source(info, sourcename) end -- always fetch the configured branch, as we don't know the build mode here. local rev = src.branch - local rsh = tools.get_tool("ssh") - local cvstool = tools.get_tool("cvs") - local cvsflags = tools.get_tool_flags("cvs") + local rsh, re = tools.get_tool("ssh") + if not rsh then + return false, e:cat(re) + end + local cvstool, re = tools.get_tool("cvs") + if not cvstool then + return false, e:cat(re) + end + local cvsflags, re = tools.get_tool_flags("cvs") + if not cvsflags then + return false, e:cat(re) + end -- split the working directory into dirname and basename as some cvs clients -- don't like slashes (e.g. in/foo) in their checkout -d argument local dir = e2lib.dirname(src.working) @@ -218,9 +227,19 @@ function cvs.prepare_source(info, sourcename, source_set, buildpath) local cmd = nil if source_set == "tag" or source_set == "branch" then local rev = mkrev(src, source_set) - local rsh = tools.get_tool("ssh") - local cvstool = tools.get_tool("cvs") - local cvsflags = tools.get_tool_flags("cvs") + local rsh, re = tools.get_tool("ssh") + if not rsh then + return false, e:cat(re) + end + local cvstool, re = tools.get_tool("cvs") + if not cvstool then + return false, e:cat(re) + end + local cvsflags, re = tools.get_tool_flags("cvs") + if not cvsflags then + return false, e:cat(re) + end + -- cd buildpath && cvs -d cvsroot export -R -r rev module cmd = string.format("cd %s && CVS_RSH=%s " .. "%s %s -d %s export -R -r %s -d %s %s", @@ -250,9 +269,18 @@ function cvs.update(info, sourcename) local e = err.new("updating source '%s' failed", sourcename) local src = info.sources[ sourcename ] local working = string.format("%s/%s", info.root, src.working) - local rsh = tools.get_tool("ssh") - local cvstool = tools.get_tool("cvs") + local rsh, re = tools.get_tool("ssh") + if not rsh then + return false, e:cat(re) + end + local cvstool, re = tools.get_tool("cvs") + if not cvstool then + return false, e:cat(re) + end local cvsflags = tools.get_tool_flags("cvs") + if cvsflags then + return false, e:cat(re) + end local cmd = string.format("cd %s && CVS_RSH=%s %s %s update -R", e2lib.shquote(working), e2lib.shquote(rsh), e2lib.shquote(cvstool), cvsflags) diff --git a/plugins/files.lua b/plugins/files.lua index 0a7f022..5b08374 100644 --- a/plugins/files.lua +++ b/plugins/files.lua @@ -561,7 +561,10 @@ function files.toresult(info, sourcename, sourceset, directory) end local tool, toolargv = rc, re - local toolname = tools.get_tool_name(tool) + local toolname, re = tools.get_tool_name(tool) + if not toolname then + return false, e:cat(re) + end f:write(string.format("\t%s", toolname)) for _,v in ipairs(toolargv) do diff --git a/plugins/git.lua b/plugins/git.lua index ba66120..9053b59 100644 --- a/plugins/git.lua +++ b/plugins/git.lua @@ -339,13 +339,19 @@ function git.prepare_source(info, sourcename, sourceset, buildpath) if not rc then return false, re end - local tar = tools.get_tool("tar") - local tarflags = tools.get_tool_flags("tar") + local tar, re = tools.get_tool("tar") + if not tar then + return false, e:cat(re) + end + local tarflags, re = tools.get_tool_flags("tar") + if not tarflags then + return false, e:cat(re) + end local cmd1 = string.format("%s %s -c -C %s/%s --exclude '.git' .", - e2lib.shquote(tar), tarflags, e2lib.shquote(info.root), - e2lib.shquote(src.working)) + e2lib.shquote(tar), tarflags, e2lib.shquote(info.root), + e2lib.shquote(src.working)) local cmd2 = string.format("%s %s -x -C %s/%s", e2lib.shquote(tar), - tarflags, e2lib.shquote(buildpath), e2lib.shquote(sourcename)) + tarflags, e2lib.shquote(buildpath), e2lib.shquote(sourcename)) rc, re = e2lib.callcmd_pipe({ cmd1, cmd2 }) if not rc then return false, e:cat(re) -- 2.39.5