Skip to content

Commit

Permalink
[core.app] Use `equal' to compare configurations.
Browse files Browse the repository at this point in the history
  • Loading branch information
eugeneia committed Aug 26, 2014
1 parent 1a07a9e commit fd2a566
Showing 1 changed file with 18 additions and 2 deletions.
20 changes: 18 additions & 2 deletions src/core/app.lua
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,22 @@ function configure (new_config)
configuration = new_config
end

-- Returns true if x and y are structurally similar (isomorphic).
function equal (x, y)
if type(x) ~= type(y) then return false end
if type(x) == 'table' then
for k, v in pairs(x) do
if not equal(v, y[k]) then return false end
end
for k, _ in pairs(x) do
if y[k] == nil then return false end

This comment has been minimized.

Copy link
@sleinen

sleinen Aug 28, 2014

I know that Lua's table semantics are a bit special, but this code still looks fishy to me. If we survive the first loop, we know that all elements that exist in X have equal element in the same places at Y. The second loop then checks whether there are "nil" elements at some places that exist in X. But we already know that if this is the case, the corresponding elements in X are also "nil" (because they have been tested to be equal()).
What is missing is a check whether Y has any additional elements that X doesn't have, right?
[I had some proposed alternative code here but it was bullshit so I removed it, sorry.]

This comment has been minimized.

Copy link
@javierguerragiraldez

javierguerragiraldez Aug 28, 2014

Contributor

i think you're right, the second loop seems to have switched x and y variables. maybe:

for k,_ in pairs(y) do
    if x[k] == nil then return false end
end
end
return true
else
return x == y
end
end

-- Return the configuration actions needed to migrate from old config to new.
-- The return value is a table:
-- app_name -> stop | start | keep | restart | reconfig
Expand All @@ -52,8 +68,8 @@ function compute_config_actions (old, new)
local action = nil
if not old.apps[appname] then action = 'start'
elseif old.apps[appname].class ~= class then action = 'restart'
elseif (type(old.apps[appname].arg) == "string" and
old.apps[appname].arg ~= arg) then action = 'reconfig'
elseif not equal(old.apps[appname].arg, arg)
then action = 'reconfig'
else action = 'keep' end
actions[appname] = action
end
Expand Down

0 comments on commit fd2a566

Please sign in to comment.