miral::ConfigFile

[AlanGriffiths]

A utility class to locate and (re)load configuration files when they change. The actual Loader processing of the file is one customization point, the reloading strategy another.

[AlanGriffiths]

Split this out of #3531 as it is generally useful

[tarek-y-ismail]

Might also succeed #3478

[Saviq]

There's one thing that we could consider here: monitoring all the applicable folders for new files. I mean, if a file shows isn't in $XDG_CONFIG_HOME on startup, but is added later, we won't consider it.

Also the XDG_CONFIG_DIRS handling is static - we only load from it once.

---

Could we have tests, please?

[Saviq]

Should we consider $PWD, too?

It's surprising that this: --display-config=static=foo.display doesn't look in $PWD for the config file.

[AlanGriffiths]

Should we consider $PWD, too?

I vote not. The XDG Base Directory spec. doesn't mention $PWD (which can also change during the life of the application). I would be more inclined to drop support for absolute paths and treat relative paths as relative to the base directory list. So that servers could organise configuration files in a subdirectory. (I adopted what we currently do for .display, which may not be ideal.)

It's surprising that this: --display-config=static=foo.display doesn't look in $PWD for the config file.

For arbitrary files I would agree. But config files typically don't load from the current directory.

There's one thing that we could consider here: monitoring all the applicable folders for new files. I mean, if a file shows isn't in $XDG_CONFIG_HOME on startup, but is added later, we won't consider it.

Yes we do.

Also the XDG_CONFIG_DIRS handling is static - we only load from it once.

We could, but is there any benefit to monitoring for system-wide configuration? Session wide configuration changes should be in $XDG_CONFIG_HOME.

Could we have tests, please?

We could. Just seemed a bit much for glue code that hits the filesystem

[mattkae]

Should we consider $PWD, too?

It's surprising that this: --display-config=static=foo.display doesn't look in $PWD for the config file.

Adding some paint to the bikeshed :) But --display-config=static=foo.display is kind of odd to me, as I would expect foo.display to be an absolute path to some file, rather than a file_name_ in a config directory. That's why $PWD is tempting in this case

[AlanGriffiths]

Adding some paint to the bikeshed :) But --display-config=static=foo.display is kind of odd to me

Before we paint the wrong bikeshed, this is about configuration files used by a compositor, not files specified on the command-line (or in the configuration).

[mattkae]

Before we paint the wrong bikeshed, this is about configuration files used by a compositor, not files specified on the command-line (or in the configuration).

True enough! At any rate, I think $PWD is a bit confusing to add to the places to search.

[RAOF]

We could, but is there any benefit to monitoring for system-wide configuration? Session wide configuration changes should be in $XDG_CONFIG_HOME.

I think we should be consistent between system-wide and user-local configuration behaviour.

Also, for the ubuntu-frame configuration do we try to stick it in the user-local or system-wide location?

[Saviq]

Also, for the ubuntu-frame configuration do we try to stick it in the user-local or system-wide location?

If running as daemon:

https://github.com/canonical/ubuntu-frame/blob/b3dbc1b7f32cb82e303efa74045c47ae87171b7e/snap/snapcraft.yaml#L79-L82

Otherwise, the defaults.

[AlanGriffiths]

OK, the following questions raised above have not been resolved:

1. How should we treat relative paths? They could be relative to the XDG base locations. (This would make monitoring more complex as we need to watch for subdirectory creation and for files within that.)
2. How do we treat absolute paths?
3. Should we monitor the XDG_CONFIG_DIRS paths? (This would make monitoring more complex as we need to prioritise one path over another.)

Also, not mentioned (yet):

4. Should we detect file deletion and load a config from a later base location?

The proposed code answers as follows:

1. How should we treat relative paths? Relative to $PWD, and monitor for changes
2. How do we treat absolute paths? As absolute paths, and monitor for changes
3. Should we monitor the XDG_CONFIG_DIRS paths? No
4. Should we detect file deletion and load a config from a later base location? No

Also, mentioned above, but I think resolved:

5. Should we look in $PWD? No

I think this is useful and understandable

[Saviq]

1. How should we treat relative paths? Relative to `$PWD`, and monitor for changes
2. How do we treat absolute paths? As absolute paths, and monitor for changes

I think I'm OK with this. But maybe we want to be more verbose about it? Logging which file we've loaded and/or are monitoring?

3. Should we monitor the `XDG_CONFIG_DIRS` paths? No

Sounds like a potential extension later.

It does relate to:

There's one thing that we could consider here: monitoring all the applicable folders for new files. I mean, if a file shows isn't in $XDG_CONFIG_HOME on startup, but is added later, we won't consider it.

Yes we do.

OK I miswrote. XDG_CONFIG_HOME (or rather, config_directory()) is the only folder we're monitoring. If things happen within "earlier" XDG_CONFIG_DIRS, we won't notice. But that may be extreme.

4. Should we detect file deletion and load a config from a later base location? No

My only problem with that is that we're behaving differently live than on startup.

5. Should we look in `$PWD`? No

👍

---

Thanks for the tests! Happy they helped :)

[AlanGriffiths]

OK I miswrote. XDG_CONFIG_HOME (or rather, config_directory()) is the only folder we're monitoring. If things happen within "earlier" XDG_CONFIG_DIRS, we won't notice. But that may be extreme.

There are no "earlier" XDG_CONFIG_DIRS: $XDG_CONFIG_HOME/$HOME/.config is the first place considered

[Saviq]

There are no "earlier" XDG_CONFIG_DIRS: $XDG_CONFIG_HOME/$HOME/.config is the first place considered

There are, if there's more than one in XDG_CONFIG_DIRS, and only the last one is where you find the file. Either way, we're not monitoring any XDG_CONFIG_DIRS, only looking at them at startup. That's something we should at least spell out. Unless I'm reading the code completely wrong, there's only ever one folder we're monitoring. And that's never any of XDG_CONFIG_DIRS.

[AlanGriffiths]

There are no "earlier" XDG_CONFIG_DIRS: $XDG_CONFIG_HOME/$HOME/.config is the first place considered

There are, if there's more than one in XDG_CONFIG_DIRS, and only the last one is where you find the file. Either way, we're not monitoring any XDG_CONFIG_DIRS, only looking at them at startup. That's something we should at least spell out. Unless I'm reading the code completely wrong, there's only ever one folder we're monitoring. And that's never any of XDG_CONFIG_DIRS.

We're in agreement on what happens, just not clear on the referent to "earlier"

[Saviq]

We're in agreement on what happens, just not clear on the referent to "earlier"

Anyway! I don't think it's a blocker, but we probably should spell it out that XDG_CONFIG_DIRS are only considered on startup.

[Saviq]

I would just like to see more logging here. But you have my ACK.

"Monitoring $path/$file for configuration changes"
"Loaded configuration from $path/$file"
"Failed to open $path/$file"

[RAOF]

Oh, no. While pondering

Should we monitor the XDG_CONFIG_DIRS paths? (This would make monitoring more complex as we need to prioritise one path over another.)

I was going to answer “Yes, we should monitor XDG_CONFIG_DIRS”. But then I considered again, and this is kinda unusual behaviour, and might trip people up (eg: saving partial config changes and having things break).

I do think that we should support changing the system-wide config at runtime, and the standard way of doing that is something like “reload config on SIGHUP”. And if we implemented that for system-wide config, would we still want to monitor the user-local file for changes?

[Saviq]

Oh, no. While pondering

To recap what we discussed:

• indeed maybe SIGHUP would be more appropriate to load the main .config file
  • it would be more in line with other long-running services
  • would allow for systemctl --user reload … when in a systemd session
  • avoids accidental / in-progress / incomplete saves
  • can be easily wrapped to cause live reloads
• display config we can keep reloading live, it's mostly used by Frame anyway - and so can use this util

[AlanGriffiths]

A (different, in addition) summary of discussion:

• This PR was never intended as the foundation of future dynamic configuration support
  • It just generalises what we do for .display (and could also support Frame's diagnostic) and makes it available
• We might generalise this to offer options: e.g. load-on-startup, auto-reload and reload-on SIGHUP

Maybe I should rename the class ConfigFile and add a strategy enum with load-on-startup and auto-reload options?

[Saviq]

Whitespace nit, otherwise good with me.