Rule source dependencies are not honored

[jcrd]

Output of awesome --version:

awesome v4.3-150-gf69b72ab (Too long)
 • Compiled against Lua 5.3.5 (running with Lua 5.3)
 • D-Bus support: ✔
 • xcb-errors support: ✔
 • execinfo support: ✔
 • xcb-randr version: 1.6
 • LGI version: 0.9.2

How to reproduce the issue:

local awful = require("awful")

awful.rules.add_rule_source("source1", function () print("source1") end)
awful.rules.add_rule_source("source2", function () print("source2") end, {"source1"})

Actual result:

"source2" is printed before "source1".

Expected result:

source2 should be run after its dependency, source1.

[psychon]

I took a quick look:

awesome/lib/awful/rules.lua

Lines 301 to 308 in 0642d92

	for _, v in ipairs(res) do
	if callbacks[v] then
	table.insert(rule_sources, 1, {
	callback = callbacks[v],
	name = v
	})
	end
	end

add_rule_source reverses the result of the topological sort. It repeatedly prepends (notice the argument 1) the next item to the list of rule sources. Thus, the effect of depends_on and precede are exactly reversed, I guess..?
(What does this mean for the AwesomeWM-internal calls to add_rule_source? Are the dependencies the wrong way around in here, too?)

[psychon]

Wait a second. I think everything is working as intended. Rule sources are not meant to actually apply anything, but to only collect the things that should be done. For example, the rule source for awful.rules just collects the matching rules and saves the properties and callbacks in the provided arguments:

awesome/lib/awful/rules.lua

Lines 330 to 338 in 0642d92

	local function apply_awful_rules(c, props, callbacks)
	for _, entry in ipairs(rules.matching_rules(c, rules.rules)) do
	gtable.crush(props,entry.properties or {})
	if entry.callback then
	table.insert(callbacks, entry.callback)
	end
	end
	end

Thus, code that runs later overwrites settings done by earlier rule sources. Thus, the later a rule source runs, the higher is its priority.

@jcrd Does this make sense to you?

@Elv13 My only question now is: Should the order of callbacks be reversed as well? So that callbacks which are appended to the callbacks arguments last are run first?

Code:

awful.rules.add_rule_source("source1", function (c, props, callback) table.insert(callback, function() print("in cb of source1") end) end)
awful.rules.add_rule_source("source2", function (c, props, callback) table.insert(callback, function() print("in cb of source2") end) end, {"source1"})

Output when a new client appears:

in cb of source2
in cb of source1

Expected output: I'm not sure. Is it the other way around or should rule source callbacks insert at the beginning? (which would lead to worse runtime due to having to shift all other callbacks back by one position; do we care?)

[jcrd]

Thus, the later a rule source runs, the higher is its priority.

This makes sense, but depends_on and precedes being reversed is confusing.

[warptozero]

It is confusing though (apart from the callback order). As rules.lua states that precedes means has priority over

--- The rule source for clients spawned by `awful.spawn.once` and `single_instance`.
--
-- **Has priority over:**
--
-- * `awful.rules`
--
-- **Depends on:**
--
-- * `awful.spawn`
--
-- @rulesources awful.spawn_once

rules.add_rule_source("awful.spawn_once", apply_singleton_rules, {"awful.spawn"}, {"awful.rules"})

but shouldn't that be depends_on instead? And don't the default rules have the wrong priority then?

rules.add_rule_source("awful.rules", apply_awful_rules, {"awful.spawn"}, {})
rules.add_rule_source("awful.spawn", apply_spawn_rules, {}, {"awful.rules"})
rules.add_rule_source("awful.spawn_once", apply_singleton_rules, {"awful.spawn"}, {"awful.rules"})

As awful.rules is overriding the spawn and spawn_once rules now (if depends_on and precedes have the right meaning).

[warptozero]

I think the meaning of the terms is actually reversed, because the awful.spawn rules do run after awful.rules as they should. And when I reversed the arguments for my custom rule sources, the rules and callbacks started running in the right order. This explains why they never worked right.

[jcrd]

Based on this description of rule_sources, new sources should be appended so that all callbacks run in the correct order without reversing the list as is done in apply_awful_rules.

awesome/lib/awful/rules.lua

Lines 213 to 218 in 0642d92

	-- Contains the sources.
	-- The elements are ordered "first in, first executed". Thus, the higher the
	-- index, the higher the priority. Each entry is a table with a `name` and a
	-- `callback` field. This table is exposed for debugging purpose. The API
	-- is private and should be modified using the public accessors.
	local rule_sources = {}

Making add_rule_source append does appear to fix custom rule dependency ordering:

awful.rules.add_rule_source("source1", function () print("source1") end)
awful.rules.add_rule_source("source2", function () print("source2") end, {"source1"})
awful.rules.add_rule_source("source3", function () print("source3") end, nil, {"source2"})
awful.rules.add_rule_source("source4", function () print("source4") end, {"source3"})

produces:

source3
source4
source1
source2

[warptozero]

@jcrd Maybe you already know and I'm misunderstanding you, but just to clarify #2725 (comment): those functions given to add_rule_source are not the actual callbacks that are run when a rule matches, but should be functions like apply_awful_rules that in turn add the actual callback to the callbacks table (given as argument callbacks).

Having the argument in add_rule_source also named plain callback is rather confusing and maybe it should be renamed to adding_callback or application_callback or something like that, including the table that they are collected in.

After adding this to a default config and running in Xephyr

awful.rules.add_rule_source("source1", function(c, props, callback)
    print("applying source1")
    gears.table.crush(props, { tag = "1" })
    table.insert(callback, function() print("in cb of source1") end)
end)

awful.rules.add_rule_source("source3", function(c, props, callback)
    print("applying source3")
    gears.table.crush(props, { tag = "3" })
    table.insert(callback, function() print("in cb of source3") end)
end, nil, {"source1"})

awful.rules.add_rule_source("source2", function(c, props, callback)
    print("applying source2")
    gears.table.crush(props, { tag = "2" })
    table.insert(callback, function() print("in cb of source2") end)
end, {"source3"}, {"source1"})

awful.spawn(terminal)

the output is

applying source1
applying source2
applying source3
in cb of source1
in cb of source2
in cb of source3

and the terminal ends up on tag 3.

This seems to be the desired behavior, so I think the terms really are reversed. And maybe they should just be renamed before and after as this is more intuitive. Although this assumes thinking in order (and the rule source itself as subject) instead of priority, but order is probably the better one here, no?

[warptozero]

Also, shouldn't it be easier to just add a table of rules as a rule source? As it stands now one has to copy the local function apply_awful_rules to their config and change one variable to make it read a new rules table.

[warptozero]

Testing the startup notifications rules

awful.rules.rules = {
    { rule = { },
      properties = { ...
                     tag = "4",
      }
      callback = function() print("in cb of awful.rules *") end,
      ...

awful.rules.add_rule_source("source1", function(c, props, callback)
    print("applying source1")
    gears.table.crush(props, { tag = "1" })
    table.insert(callback, function() print("in cb of source1") end)
end, nil, {"awful.rules"})

awful.rules.add_rule_source("source3", function(c, props, callback)
    print("applying source3")
    gears.table.crush(props, { tag = "3" })
    table.insert(callback, function() print("in cb of source3") end)
end, {"awful.spawn"}, {"source1"})

awful.rules.add_rule_source("source2", function(c, props, callback)
    print("applying source2")
    gears.table.crush(props, { tag = "2" })
    table.insert(callback, function() print("in cb of source2") end)
end, {"source3"}, {"source1"})

awful.spawn("arandr",
    { tag = "5" },
    function() print("in cb of spawn") end
)

shows that they work.

Btw, would it be a good idea if the order arguments of add_rule_source

awesome/lib/awful/rules.lua

Lines 260 to 262 in 0642d92

	function rules.add_rule_source(name, callback, depends_on, precede)
	depends_on = depends_on or {}
	precede = precede or {}

default to {"awful.spawn"} and {"awful.rules"} respectively so a rule source defined without them automatically get's put in between them? Which would probably be desirable for most users.

[jcrd]

@warptozero Thanks for clarifying callbacks intended use. The API documentation isn't explicit about it.

Still, the rule source callbacks themselves (given to add_rule_source) should be running in "first in, first executed" order, but by prepending to rule_sources the order is "first in, last executed". When setting properties in rule source callbacks, the first added callback will have the highest priority, which isn't the documented behavior if I'm understanding the comment correctly.

[warptozero]

Still, the rule source callbacks themselves (given to add_rule_source) should be running in "first in, first executed" order, but by prepending to rule_sources the order is "first in, last executed".

Yes exactly, and that's why the meaning of depends_on and precede are reversed. The calls to add_rule_sources in rules.lua already assume this so the default config works but your custom sources run backwards.

[Elv13]

As @psychon mentionned, the order is intentional. tl;dr, as already mentionned, the rules are applied only once after all rules sources are "consulted". This is also on purpose to avoid the case where extra code is called if a property is set multiple time. For example, changing the geometry will call a lot of code (the tiled layout, awful.ewmh, etc, etc, but more importantly, the client itself may or may not be notified it has been resized multiple time for nothing).

Plus, the property application order is itself very important. For example, setting the geometry before of after the titlebars have been added don't produce the same result. Same for the position and border_width.

Because of those 2 fact, the rule sources are designed to modify a property table that is then applied. This means they have the "right" to overwrite what the other sources added to the table. So, because of that, the last one to be executed has the final say in what is and isn't in the property table. Because of that the execution order is from "fallback values" to "this has to be the absolute value".

Anyhow, this pull request #2600 has more documentation about the rules. Some comments in awful.rules reflect earlier iteration of the design and probably need to be removed / updated.

@warptozero / @jcrd : May I ask how you (plan to?) use this API? I am very interested in these use cases since "soon" this API will also be available for the notifications and clipboard selection. It was originally mainly added to fix awful.spawn and integrate my own Tyrannical module without monkey patching awful.rules.

@Elv13 My only question now is: Should the order of callbacks be reversed as well? So that callbacks which are appended to the callbacks arguments last are run first?

I need to review this when I really have time, no in a drive-by comment. I was supposed to be "in vacation" since my last contract ended, but the word spread and now I am more booked than ever... I managed to get the notification view PR in "some commits still have luacheck problem but otherwise its done" state, but I doesn't look like I will have more time in the next few weeks.

[warptozero]

@Elv13 Thanks for the explanation even though you don't have a lot of time. I understand what you are saying, but I still think the terms have a reversed effect and meaning just by looking at the way add_rule_source is called.

awesome/lib/awful/rules.lua

Line 260 in 0642d92

function rules.add_rule_source(name, callback, depends_on, precede)

rules.add_rule_source("awful.rules", apply_awful_rules, {"awful.spawn"}, {})
rules.add_rule_source("awful.spawn", apply_spawn_rules, {}, {"awful.rules"})
rules.add_rule_source("awful.spawn_once", apply_singleton_rules, {"awful.spawn"}, {"awful.rules"})

awful.rules.add_rule_source(
    "tyrannical", apply_tyrannical_rules, {"snid", "awful.spawn"}, {"awful.rules"}
)

But I might be wrong, so someone should try to falsify this #2725 (comment). (Btw, I edited out a mistake I made as not to confuse things further.)

May I ask how you (plan to?) use this API? I am very interested in these use cases since "soon" this API will also be available for the notifications and clipboard selection.

Well I wrote a settings module that saves the state of it's properties to disk (serialized to a lua table) so for example widgets can change settings and they will be reserved on restart. Or the last selected tag, etc. It's also gears.object with signals and works really well.

Naturally I also used it to store things like screen layout and tags with properties and client rules. These rules are then compiled to a rule table and were appended to awful.rules.rules (which worked but I thought there's an API so that's probably the right way to do it). So this part has actually grown out to be something like Tyrannical, which I didn't choose back then because it was way more then I needed. So I guess these are the same kind of problems you were trying to solve with that API.

And I thought it would also make it easier to set priorities on individual rules and screens (by having differenent sources for each priority/screen), and make it easy to change the rules when a screen is added or removed.

Somewhat tangential, but I think the biggest issue I have with awesome is that I always keep running into big or subtle problems with screens and rules (on what tag/screen clients are launched, especially after a screen is added/removed). This is probably because I'm hacking things that I don't understand completely, but these rule anomalies that I can't seem to track down are my main point of frustration with awesome.

[jcrd]

@Elv13 I'm using the API to implement generic single instance IDs in a custom spawn library. When spawning a client, a unique ID is generated and set as an environment variable alongside an LD_PRELOAD hack which reads this environment variable and sets an X11 property on new windows. The rule source checks for this window property on newly managed clients and associates it with an ID to apply rules and pre-configurations. Another rule source exists to apply specific rules to certain clients (using the single instance ID) both when they're launched and when awesome is restarted, so it must depend on the first rule source to set the ID.

[Elv13]

Ah nice! This has been in my TODO list for years. Can you share this so I can use it in Tyrannical? AwesomeWM 4.3 have an half assed attempt done by reading /proc, but it has always been my intention to provide some LD_PRELOAD one too.

[jcrd]

@Elv13 The LD_PRELOAD tool is at wm-launch.
Although fairly rudimentary, I've made the accompanying awesome library available here.

[unai-ndz]

I also think the name of the properties should be reversed. Another example from the docs is the workaround for startup ids which is not working as intended.

It does set the startup_id but only after applying the rules so any rule using startup_id will not work. It works by switching the depends_on and precede arguments though.

As the workaround in itself is tricky here is the minimal code to reproduce this.

This sets the property but you have to reapply the rules to the client (awful.rules.apply(c)) for them to apply:

awful.rules.add_rule_source(
    "snid", function(c) c.custom_property = "custom_property" end, {}, {"awful.spawn", "awful.rules"}
)

-- On awful.rules.rules
{ rule = { class = "CLIENT_CLASS", custom_property = "custom_property" },
    properties = { floating = true }
},

function rules.add_rule_source(name, callback, depends_on, precede)

Like this you would expect that the snid rule gets executed first as it precedes both awful.spawn and awful.rules but in fact is executed last. If we switch the tables everything works as expected:

"snid", function(c) c.custom_property = "custom_property" end, {"awful.spawn", "awful.rules"}, {}