Avoid cyclic dependencies between modules

[Elv13]

gears.color->beautiful.init->beautiful.theme_assets->gears.color = BOOM

How this got past the CI...

Simplest solution, don't require theme_assets in beautiful.init

[Elv13]

nevermind, it can't happen on master. /me have too many patches

[psychon]

Should we define some hard rules on what might depend on what to make sure we have no cyclic dependencies? For example, I would propose that gears must not use beautiful...

[Elv13]

Sure. It will however require some changes as gears depends on awful.util.

gears: depends on nothing
wibox: depends on gears
beautiful: depends on gears
awful: depends on everything

awful.util deprecation support can be moved into gears.deprecation. awful.util.table should be moved to table itself. awful.util string utils should move into string. This can be done without breaking the API (with aliases).

[actionless]

why not to move awful.util to just util?

[psychon]

I don't think we should start monkey-patching standard Lua things. We already do that too much. That leads to too much confusion. Also 'util' is too much of a generic name that eventually some name collision will occur.
Edit: Besides that: 👍

[psychon]

(My plan is to implement a tool that tries to automatically check the above rules and complains if they are not followed.)
Edit: Even better: https://github.com/mpeterv/depgraph
Edit:

[psychon]

Following program run in the awesome source directory:

local depgraph = require("depgraph")

local allowed_deps = {
	gears = {
		lgi = true,
	},
	beautiful = {
		gears = true,
		lgi = true,
	},
	wibox = {
		beautiful = true,
		gears = true,
		lgi = true,
	},
	awful = {
		beautiful = true,
		gears = true,
		lgi = true,
		wibox = true,
	},
	naughty = {
		awful = true,
		beautiful = true,
		gears = true,
		lgi = true,
		wibox = true,
	},
	menubar = {
		awful = true,
		beautiful = true,
		gears = true,
		lgi = true,
		wibox = true,
	}
}

local graph = assert(depgraph.make_graph({"lib"}, {}, "lib", nil, nil))

for _, module in ipairs(graph.modules) do
	local base_name = string.match(module.name, "%a*")
	local allowed = allowed_deps[base_name] or {}
	for _, dep in ipairs(module.deps) do
		local dep_base_name = string.match(dep.name, "%a*")
		if base_name ~= dep_base_name and not allowed[dep_base_name] then
			print(string.format("Unallowed dependency by %s to %s (%s -> %s)",
					module.name, dep.name, base_name, dep_base_name))
		end
	end
end

produces the following output:

Unallowed dependency by awful.hotkeys_popup.widget to menubar (awful -> menubar)
Unallowed dependency by beautiful.xresources to awful.util (beautiful -> awful)
Unallowed dependency by gears.object to awful.util (gears -> awful)
Unallowed dependency by gears.surface to wibox.hierarchy (gears -> wibox)
Unallowed dependency by wibox.container.arcchart to awful.util (wibox -> awful)
Unallowed dependency by wibox.container.background to awful.util (wibox -> awful)
Unallowed dependency by wibox.container.constraint to awful.util (wibox -> awful)
Unallowed dependency by wibox.container.margin to awful.util (wibox -> awful)
Unallowed dependency by wibox.container.mirror to awful.util (wibox -> awful)
Unallowed dependency by wibox.container.radialprogressbar to awful.util (wibox -> awful)
Unallowed dependency by wibox.container.rotate to awful.util (wibox -> awful)
Unallowed dependency by wibox.layout.align to awful.util (wibox -> awful)
Unallowed dependency by wibox.layout.constraint to awful.util (wibox -> awful)
Unallowed dependency by wibox.layout.fixed to awful.util (wibox -> awful)
Unallowed dependency by wibox.layout.flex to awful.util (wibox -> awful)
Unallowed dependency by wibox.layout.margin to awful.util (wibox -> awful)
Unallowed dependency by wibox.layout.mirror to awful.util (wibox -> awful)
Unallowed dependency by wibox.layout.ratio to awful.util (wibox -> awful)
Unallowed dependency by wibox.layout.rotate to awful.util (wibox -> awful)
Unallowed dependency by wibox.layout.scroll to awful.util (wibox -> awful)
Unallowed dependency by wibox.layout.stack to awful.util (wibox -> awful)
Unallowed dependency by wibox.widget.background to awful.util (wibox -> awful)
Unallowed dependency by wibox.widget.base to awful.util (wibox -> awful)
Unallowed dependency by wibox.widget.checkbox to awful.util (wibox -> awful)
Unallowed dependency by wibox.widget.imagebox to awful.util (wibox -> awful)
Unallowed dependency by wibox.widget.piechart to awful.util (wibox -> awful)
Unallowed dependency by wibox.widget.progressbar to awful.util (wibox -> awful)
Unallowed dependency by wibox.widget.slider to awful.util (wibox -> awful)
Unallowed dependency by wibox.widget.systray to awful.util (wibox -> awful)
Unallowed dependency by wibox.widget.textbox to awful.util (wibox -> awful)

So... how do we get from here to a situation that does not violate the proposed rules?

[psychon]

Also: We might want to run luadepgraph -m lib -p lib --cycles --strict during a normal build to check for cyclic strong dependencies.