Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Reverse order for parsing files in XDG_CONFIG_DIRS #4630

Merged
merged 4 commits into from
Nov 6, 2024

Conversation

bartoldeman
Copy link
Contributor

https://specifications.freedesktop.org/basedir-spec/latest/ says precedence should be

XDG_CONFIG_HOME > 1st entry of XDG_CONFIG_DIRS > 2nd entry ...

EasyBuild had this reversed for XDG_CONFIG_DIRS

Fixes #4582

https://specifications.freedesktop.org/basedir-spec/latest/
says precedence should be

XDG_CONFIG_HOME > 1st entry of XDG_CONFIG_DIRS > 2nd entry ...

EasyBuild had this reversed for XDG_CONFIG_DIRS

Fixes easybuilders#4582
@bartoldeman bartoldeman added this to the 5.0 milestone Sep 12, 2024
@bartoldeman
Copy link
Contributor Author

There's a bit of an order of priority issue here with wildcards:
Presently we have this output for example, with XDG_CONFIG_DIRS=$HOME/hi:$HOME/lo

* user-level: ${XDG_CONFIG_HOME:-$HOME/.config}/easybuild/config.cfg
  -> /home/oldeman/.config/easybuild/config.cfg => found
* system-level: ${XDG_CONFIG_DIRS:-/etc}/easybuild.d/*.cfg
  -> {/cvmfs/soft.computecanada.ca/gentoo/2023/x86-64-v3/etc/xdg, /home/oldeman/hi, /home/oldeman/lo}/easybuild.d/*.cfg => /home/oldeman/hi/easybuild.d/bla.cfg, /home/oldeman/hi/easybuild.d/foo.cfg, /home/oldeman/lo/easybuild.d/bla.cfg, /home/oldeman/lo/easybuild.d/foo.cfg

Default list of existing configuration files (5): /home/oldeman/hi/easybuild.d/bla.cfg, /home/oldeman/hi/easybuild.d/foo.cfg, /home/oldeman/lo/easybuild.d/bla.cfg, /home/oldeman/lo/easybuild.d/foo.cfg, /home/oldeman/.config/easybuild/config.cfg

In the present PR I've only reversed the XDG_CONFIG_DIRS-derived list but did not reverse the alphabetical order for wildcards; I also did not change the output after ->
so we now have:

Default list of existing configuration files (5): /home/oldeman/lo/easybuild.d/bla.cfg, /home/oldeman/lo/easybuild.d/foo.cfg, /home/oldeman/hi/easybuild.d/bla.cfg, /home/oldeman/hi/easybuild.d/foo.cfg, /home/oldeman/.config/easybuild/config.cfg

This list is fed to configparser, last one has priority.

If I also change the order in the list displayed after -> then that looks confusing since XDG_CONFIG_DIRS would have lo and hi reversed, which is then weird looking syntax, e.g.

* system-level: ${XDG_CONFIG_DIRS:-/etc}/easybuild.d/*.cfg
  -> {/cvmfs/soft.computecanada.ca/gentoo/2023/x86-64-v3/etc/xdg, /home/oldeman/hi, /home/oldeman/lo}/easybuild.d/*.cfg => /home/oldeman/lo/easybuild.d/bla.cfg, /home/oldeman/lo/easybuild.d/foo.cfg, /home/oldeman/hi/easybuild.d/bla.cfg, /home/oldeman/hi/easybuild.d/foo.cfg

where the visual expansion flips order, so I decided not to change the output there. If anyone has a better idea how to display, let me know!

@boegel boegel added the EasyBuild-5.0 EasyBuild 5.0 label Sep 18, 2024
@boegel
Copy link
Member

boegel commented Sep 23, 2024

@bartoldeman Can we tweak the language used in --show-default-configfiles so it's crystal-clear what the priority is between the configuration files that are present?

@bartoldeman
Copy link
Contributor Author

I change the language as follows, also moving the list on a separate line:

Default list of existing configuration files (5, most important last):
/home/oldeman/lo/easybuild.d/bla.cfg, /home/oldeman/lo/easybuild.d/foo.cfg, /home/oldeman/hi/easybuild.d/bla.cfg, /home/oldeman/hi/easybuild.d/foo.cfg, /home/oldeman/.config/easybuild/config.cfg

E.g.:
Default list of existing configuration files (5, most important last):
/home/oldeman/lo/easybuild.d/bla.cfg, /home/oldeman/lo/easybuild.d/foo.cfg, /home/oldeman/hi/easybuild.d/bla.cfg, /home/oldeman/hi/easybuild.d/foo.cfg, /home/oldeman/.config/easybuild/config.cfg
@bartoldeman
Copy link
Contributor Author

This should be good to go @boegel

@boegel boegel merged commit 234ad84 into easybuilders:5.0.x Nov 6, 2024
36 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants