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

Use default value $XDG_CONFIG_DIRS from XDG basedir spec: /etc/xdg (instead of /etc) #4591

Merged
merged 3 commits into from
Sep 6, 2024

Conversation

runiq
Copy link

@runiq runiq commented Aug 1, 2024

The spec mandates /etc/xdg instead of /etc for the default value of XDG_CONFIG_DIRS.

I'm pretty sure this requires documentation changes because this is potentially a breaking change, but I'm not entirely sure where. I hope a project member can point me to where. 😶💦

Fixes #4581

The spec mandates `/etc/xdg` instead of `/etc` for the default value of `XDG_CONFIG_DIRS` [1].

[1] https://specifications.freedesktop.org/basedir-spec/basedir-spec-latest.html#variables
@boegel boegel changed the title Use XDG_CONFIG_DIRS value from XDG basedir spec Use XDG_CONFIG_DIRS value from XDG basedir spec: /etc/xdg (instead of /etc) Aug 13, 2024
@boegel boegel changed the title Use XDG_CONFIG_DIRS value from XDG basedir spec: /etc/xdg (instead of /etc) Use default value $XDG_CONFIG_DIRS from XDG basedir spec: /etc/xdg (instead of /etc) Aug 13, 2024
@boegel boegel added change EasyBuild-5.0 EasyBuild 5.0 labels Aug 13, 2024
@boegel boegel added this to the 5.0 milestone Aug 13, 2024
@boegel
Copy link
Member

boegel commented Aug 14, 2024

Since we're not following the spec w.r.t. the default value if $XDG_CONFIG_DIRS we should indeed fix this, but we should also try and limit the impact of this change.

We could for example at least print a warning message when we find one or more configuration files at /etc/easybuild.d/*.cfg (but only when $XDG_CONFIG_DIRS is not set?) to indicate that those configuration files will now be ignored, and that they should be moved to /etc/xdg/easybuild.d/*.cfg?

@boegel boegel added the bug fix label Aug 14, 2024
@boegel
Copy link
Member

boegel commented Aug 26, 2024

We could print a warning if $XDG_CONFIG_DIRS is not defined and *.cfg files are found in /etc/easybuild.d/, which instructs the user to define $XDG_CONFIG_DIRS to /etc as workaround (which will affect other stuff too though), or move those configuration files to /etc/xdg/easybuild.d/.

bartoldeman added a commit to bartoldeman/easybuild-framework that referenced this pull request Aug 30, 2024
If you don't set XDG_CONFIG_DIRS and files are present in
/etc/easybuild.d we now get

WARNING: Deprecated functionality, will no longer work in EasyBuild v6.0: Using /etc/easybuild.d is deprecated. Please use /etc/xdg/easybuild.d instead or add /etc to XDG_CONFIG_DIRS; see https://docs.easybuild.io/deprecated-functionality/ for more information

This addresses
easybuilders#4591 (comment)
@bartoldeman bartoldeman self-assigned this Aug 30, 2024
If you don't set XDG_CONFIG_DIRS and files are present in
/etc/easybuild.d we now get

WARNING: Deprecated functionality, will no longer work in EasyBuild v6.0: Using /etc/easybuild.d is deprecated. Please use /etc/xdg/easybuild.d instead or add /etc to XDG_CONFIG_DIRS; see https://docs.easybuild.io/deprecated-functionality/ for more information

This addresses
easybuilders#4591 (comment)
Copy link
Contributor

@bartoldeman bartoldeman left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

Copy link
Contributor

@bartoldeman bartoldeman left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@bartoldeman bartoldeman merged commit bdfa568 into easybuilders:5.0.x Sep 6, 2024
35 checks passed
@runiq runiq deleted the 5.0.x-etc-xdg branch September 6, 2024 13:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants