Change ConfigFile to load after server init

[tarek-y-ismail]

Should allow initial config reads to interact with server components properly.

Also, tweaked tests a bit to be more explicit about failures. More importantly, tests now start the server manually to allow each test's ConfigFile instance to be set up. This brings the test behavior closer to the real use case in server_example.cpp

[AlanGriffiths]

This isn't working:

./miral-test --gtest_filter=TestConfigFile.with_reload_on_change_a_file_in_xdg_config_home_is_reloaded --gtest_repeat=10

Will show around 50% failures

(The patch I shared on MM doesn't do any better)

[tarek-y-ismail]

Will show around 50% failures

Replacing std::cerr with FAIL() in wait_for_load shows that this also happens on main. Though it seems more frequent with the changes in this PR. Increasing the timeout up to 10 seconds doesn't help so the issue must be deeper

[hbatagelo]

It all seems reasonable to me, and the tests pass consistently when run locally. However, lxd::ubuntu-24.04:spread/build/ubuntu:ubsan is failing with init_and_start() timed out. Maybe increase the timeout?

[AlanGriffiths]

I now have to wonder: is this actually what we want to do?

Before this we were loading a config file immediately and the problem was that we wanted to apply it to objects that don't exist at that time.

But another scenario is to want to use configuration values in initialising the program. For that the current behaviour of loading the file immediately is the better timing.

Instead of deferring loading the configuration as proposed here, it would be possible, and more flexible, to defer applying the configuration.

[tarek-y-ismail]

So here's what i think we can do, in addition to the existing on_load lambda. We can pass another delayed_apply lambda that the user can call at whatever point they want. If the use wants to load and apply the settings at the same time, the can forego the extra lambda. What do you think?

[AlanGriffiths]

So here's what i think we can do, in addition to the existing on_load lambda. We can pass another delayed_apply lambda that the user can call at whatever point they want. If the use wants to load and apply the settings at the same time, the can forego the extra lambda. What do you think?

I think the user can do all that without bothering ConfigFile: they take the data provided by on_load and store it for later use.

For example, in miral::InputConfiguration::Self::apply() the if statement:

        if (auto const idh = input_device_hub.lock())
        {
            idh->for_each_mutable_input_device([&m](auto& input_device)
            {
                m.self->apply_to(input_device);
            });
        }

Currently there is no else branch, but that could store the value for use by the init_ callback.

[tarek-y-ismail]

I don't even think we need to fiddle with InputConfiguration. Something like:

miral::InputConfiguration input_configuration;
    
// Share mouse/touchpad/keyboard settings between initial load and reloads
auto mouse = input_configuration.mouse();
auto touchpad = input_configuration.touchpad();
auto keyboard = input_configuration.keyboard();

// Load and try to apply settings immediately, repeat whenever the file changes.
miral::ConfigFile test{runner, "mir_demo_server.input", miral::ConfigFile::Mode::reload_on_change,
	[&](auto& in, auto path) 
	{
	    // Existing config loading/parsing code

            // Apply settings
            input_configuration.mouse(mouse);
            input_configuration.touchpad(touchpad);
            input_configuration.keyboard(keyboard);
	}
}};

// Apply settings on compositor startup
runner.add_start_callback(
[&]
{
    input_configuration.mouse(mouse);
    input_configuration.touchpad(touchpad);
    input_configuration.keyboard(keyboard);
});

Seems to do the trick. There's a little duplication but I'm okay with that, what do you think? If you're okay with doing things this way, feel free to close this PR (sorry Harlen :))

[AlanGriffiths]

There's a little duplication but I'm okay with that

Well, the duplication does reflect shared requirement, rather than shared implementation. Maybe extract an apply_input_settings() function?