Commit 0a42064b authored by Michal 'vorner' Vaner's avatar Michal 'vorner' Vaner

Integrate modification config check to removal

Check if config file has been modified when considering to remove it. If
it has, leave it in place.
parent d65755d0
...@@ -108,4 +108,11 @@ function clone(data) ...@@ -108,4 +108,11 @@ function clone(data)
end end
end end
-- Add all elements of src to dest
function table_merge(dest, src)
for k, v in pairs(src) do
dest[k] = v
end
end
return _M return _M
...@@ -668,41 +668,46 @@ end ...@@ -668,41 +668,46 @@ end
Remove files provided as a set and any directories which became Remove files provided as a set and any directories which became
empty by doing so (recursively). empty by doing so (recursively).
]] ]]
function pkg_cleanup_files(files) function pkg_cleanup_files(files, rm_configs)
for f in pairs(files) do for f in pairs(files) do
-- Make sure there are no // in there, which would confuse the directory cleaning code -- Make sure there are no // in there, which would confuse the directory cleaning code
f = f:gsub("/+", "/") f = f:gsub("/+", "/")
path = root_dir .. f local path = root_dir .. f
DBG("Removing file " .. path) local hash = rm_configs[f]
local ok, err = pcall(function () os.remove(path) end) if hash and config_modified(path, hash) then
-- If it failed because the file didn't exist, that's OK. Mostly. DBG("Not removing config " .. f .. ", as it has been modified")
if not ok then else
local tp = stat(path) DBG("Removing file " .. path)
if tp then local ok, err = pcall(function () os.remove(path) end)
error(err) -- If it failed because the file didn't exist, that's OK. Mostly.
else if not ok then
WARN("Not removing " .. path .. " since it is not there") local tp = stat(path)
if tp then
error(err)
else
WARN("Not removing " .. path .. " since it is not there")
end
end end
end -- Now, go through the levels of f, looking if they may be removed
-- Now, go through the levels of f, looking if they may be removed -- Iterator for the chunking of the path
-- Iterator for the chunking of the path function get_parent()
function get_parent() local parent = f:match("^(.+)/[^/]+")
local parent = f:match("^(.+)/[^/]+") f = parent
f = parent return f
return f end
end for parent in get_parent do
for parent in get_parent do if next(ls(root_dir .. parent)) then
if next(ls(root_dir .. parent)) then DBG("Directory " .. root_dir .. parent .. " not empty, keeping in place")
DBG("Directory " .. root_dir .. parent .. " not empty, keeping in place") -- It is not empty
-- It is not empty
break
else
DBG("Removing empty directory " .. root_dir .. parent)
local ok, err = pcall(function () os.remove(root_dir .. parent) end)
if not ok then
-- It is an error, but we don't want to give up on the rest of the operation because of that
ERROR("Failed to removed empty " .. parent .. ", ignoring")
break break
else
DBG("Removing empty directory " .. root_dir .. parent)
local ok, err = pcall(function () os.remove(root_dir .. parent) end)
if not ok then
-- It is an error, but we don't want to give up on the rest of the operation because of that
ERROR("Failed to removed empty " .. parent .. ", ignoring")
break
end
end end
end end
end end
......
...@@ -29,6 +29,7 @@ local ipairs = ipairs ...@@ -29,6 +29,7 @@ local ipairs = ipairs
local next = next local next = next
local error = error local error = error
local pcall = pcall local pcall = pcall
local pairs = pairs
local table = table local table = table
local backend = require "backend" local backend = require "backend"
local utils = require "utils" local utils = require "utils"
...@@ -105,10 +106,13 @@ function perform(operations) ...@@ -105,10 +106,13 @@ function perform(operations)
-- TODO: Format the error message about collisions -- TODO: Format the error message about collisions
error("Collisions happened") error("Collisions happened")
end end
local all_configs = {}
-- TODO: Journal note, we're going to proceed now. -- TODO: Journal note, we're going to proceed now.
-- Go through the list once more and perform the prepared operations -- Go through the list once more and perform the prepared operations
for _, op in ipairs(plan) do for _, op in ipairs(plan) do
if op.op == "install" then if op.op == "install" then
-- We may want to remove one of the old configs on upgrade. Store the hash to check modification
utils.table_merge(all_configs, op.old_configs)
-- TODO: pre-install scripts (who would use such thing anyway?) -- TODO: pre-install scripts (who would use such thing anyway?)
backend.pkg_merge_files(op.dir .. "/data", op.dirs, op.files, op.old_configs) backend.pkg_merge_files(op.dir .. "/data", op.dirs, op.files, op.old_configs)
end end
...@@ -121,12 +125,13 @@ function perform(operations) ...@@ -121,12 +125,13 @@ function perform(operations)
status[op.control.Package] = op.control status[op.control.Package] = op.control
-- TODO: Postinst script -- TODO: Postinst script
elseif op.op == "remove" then elseif op.op == "remove" then
utils.table_merge(all_configs, status[op.name].Conffiles or {})
status[op.name] = nil status[op.name] = nil
-- TODO: Pre-rm script, but only if not re-installed -- TODO: Pre-rm script, but only if not re-installed
end end
end end
-- Clean up the files from removed or upgraded packages -- Clean up the files from removed or upgraded packages
backend.pkg_cleanup_files(removes) backend.pkg_cleanup_files(removes, all_configs)
-- TODO: post-rm scripts, for the removed (not re-installed) packages -- TODO: post-rm scripts, for the removed (not re-installed) packages
-- TODO: Think about when to clean up any leftover files if something goes wrong? On success? On transaction rollback as well? -- TODO: Think about when to clean up any leftover files if something goes wrong? On success? On transaction rollback as well?
end) end)
......
...@@ -394,10 +394,27 @@ function test_pkg_unpack() ...@@ -394,10 +394,27 @@ function test_pkg_unpack()
-- Delete the backed-up file, it is not tracked -- Delete the backed-up file, it is not tracked
os.remove(test_root .. "/etc/config/updater-opkg") os.remove(test_root .. "/etc/config/updater-opkg")
-- Now try clearing the package. When we list all the files, it should remove the directories as well. -- Now try clearing the package. When we list all the files, it should remove the directories as well.
B.pkg_cleanup_files(files) B.pkg_cleanup_files(files, {})
assert_table_equal({}, ls(test_root)) assert_table_equal({}, ls(test_root))
end end
function test_cleanup_files_config()
local test_root = mkdtemp()
table.insert(tmp_dirs, test_root)
-- Create an empty testing file
local fname = test_root .. "/config"
io.open(fname, "w"):close()
B.root_dir = test_root
-- First try with a non-matching hash ‒ the file has been modified
B.pkg_cleanup_files({["/config"] = true}, {["/config"] = "12345678901234567890123456789012"})
-- It is left there
assert_equal("r", stat(fname))
-- But when it matches, it is removed
B.pkg_cleanup_files({["/config"] = true}, {["/config"] = "d41d8cd98f00b204e9800998ecf8427e"})
-- The file disappeared
assert_nil(stat(fname))
end
-- Test the collision_check function -- Test the collision_check function
function test_collisions() function test_collisions()
local status = B.status_parse() local status = B.status_parse()
......
...@@ -26,7 +26,10 @@ module("transaction-tests", package.seeall, lunit.testcase) ...@@ -26,7 +26,10 @@ module("transaction-tests", package.seeall, lunit.testcase)
local test_status = { local test_status = {
["pkg-rem"] = { ["pkg-rem"] = {
Package = "pkg-rem" Package = "pkg-rem",
Conffiles = {
remconf = "12345678901234567890123456789012"
}
}, },
["pkg-name"] = { ["pkg-name"] = {
Package = "pkg-name", Package = "pkg-name",
...@@ -118,7 +121,7 @@ function test_perform_empty() ...@@ -118,7 +121,7 @@ function test_perform_empty()
}, },
{ {
f = "backend.pkg_cleanup_files", f = "backend.pkg_cleanup_files",
p = {{}} p = {{}, {}}
} }
}, outro({}, test_status)) }, outro({}, test_status))
assert_table_equal(expected, mocks_called) assert_table_equal(expected, mocks_called)
...@@ -174,7 +177,7 @@ function test_perform_ok() ...@@ -174,7 +177,7 @@ function test_perform_ok()
}, },
{ {
f = "backend.pkg_cleanup_files", f = "backend.pkg_cleanup_files",
p = {{d2 = true}} p = {{d2 = true}, {c = "12345678901234567890123456789012", remconf = "12345678901234567890123456789012"}}
} }
}, outro({"pkg_dir"}, status_mod)) }, outro({"pkg_dir"}, status_mod))
assert_table_equal(expected, mocks_called) assert_table_equal(expected, mocks_called)
......
...@@ -67,3 +67,14 @@ function test_clone() ...@@ -67,3 +67,14 @@ function test_clone()
assert_not_equal(input.z, output.z) assert_not_equal(input.z, output.z)
assert_equal("xyz", U.clone("xyz")) assert_equal("xyz", U.clone("xyz"))
end end
function test_table_merge()
local t1 = {}
local t2 = {a = 1, b = 2}
U.table_merge(t1, t2)
assert_table_equal(t2, t1)
U.table_merge(t1, {})
assert_table_equal(t2, t1)
U.table_merge(t1, {b = 3, c = 4})
assert_table_equal({a = 1, b = 3, c = 4}, t1)
end
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment