Xdg basedir spec

[jrabinow]

Description

Implements XDG Base Directory Specification
Motivation: https://farbenmeer.de/de/blog/the-power-of-the-xdg-base-directory-specification
I'm aware of this PR already being open. But it looks like it doesn't implement the full spec and seems to be going into limbo, so I gave this a shot.

With these changes, taskwarrior still supports the current file locations, but make the new locations the default for new users. As suggested by @corbolais here, if files exist in both old and new locations, taskwarrior will print a warning and uses the old locations.

This is the first C++ code I've ever written -> improvement suggestions welcome.

Additional information...

• Have you run the test suite? Yes, I don't think any of the failing tests are due to my code but in case I'm mistaken let me know and I'll fix them

cd test && ./problems
Failed:
bash_completion.t                   4
dateformat.t                        1
dependencies.t                      1
filter.t                            5
project.t                           1
quotes.t                            2
search.t                            1
tw-1688.t                           1
tw-1718.t                           1
undo.t                              2
version.t                           1

Unexpected successes:

Skipped:
dependencies.t                      3
export.t                            1
feature.default.project.t           4
hooks.on-modify.t                   1
import.t                            1
recurrence.t                        1
tw-1999.t                           2
wait.t                              1

Expected failures:
dependencies.t                      2
hyphenate.t                         1
project.t                           1
[Exit 1]

[sl4mmy]

I don't think we should break existing users' setups like this. Also, please try to use consistent code style to match the existing code.

[corbolais]

Thank you for your effort.

Never break my existing setup!

Nor any usr's existing setup for that matter. As @sl4mmy already mentioned there should be a soft migration path.

If a usr has .taskrc leave it alone. Period.

Never move usr stuff in any occasion on your own. Think of individual config environments or backup strategies, breaking badly w/o a use noticing right away!

You cannot possibly know anything about local policies, decisions or prefs. Potentially destroying usr data is evil. Think of moving data or config files on a filesystem becoming full w/o proper checks or handling them atomically at all. Possibly rendering usr data unusable, corrupt or into entirely oblivious bit noise. May BOFH get hold of you if you do!

[jrabinow]

Thanks for the migration plan ideas and the review comments in general. I'll fix the formatting stuff, please review my proposed migration plan here #2316 (comment)

Also if one of you has ideas around #2316 (comment) they're most welcome

[jrabinow]

I added support for legacy file paths as requested. Please note that the changes here shouldn't be merged in to 2.6.0 branch until GothenburgBitFactory/libshared#25 has been reviewed and merged in, as otherwise these changes here will fail to create the new directories if the parent directories don't exist.

Once the other PR is merged in, I will create a new PR in taskwarrior that upgrades libshared to the latest version.

[jrabinow]

My last concern is around windows support - I take it windows isn't supported outside of cygwin environments? If so, I don't expect any issues on that front.

[sl4mmy]

I haven't actually tried your latest changes yet, but they read nicely to me. Thanks for updating your PR and incorporating our feedback! 👍

[pbeckingham]

I'm against XDG, always have been. Relocating the config file and data directory has been supported for 14 years.

[jrabinow]

@pbeckingham indeed, there's already support for overrides, and I understand your view on the standard being nonsense. Yet this feature has been requested and thumbed up quite a few times by the community (#438 which itself references a previous issue, #1105, #2203, #2290) and 2 people have created PRs (#2248, #2316), or people are developing their own workarounds.
I've done my best here to address all the concerns brought up previously, despite the fact that I disagree with some of the readings (deletion of $XDG_RUNTIME_DIR should be handled by gnome/kde/whatever, $XDG_DATA_HOME doesn't seem to be for storing read-only data found in /usr/share)

Setting aside the quality of the standard for a moment: if you feel this PR is making the codebase dirty or goes against your vision, I get that, otherwise I'd like to request you reconsider not your opinion of the standard but the value this PR would bring to the community (and to the maintainers who wouldn't have to deal with this issue on a recurring basis!). - is there anything else I could do to help convince you that this PR is worth merging in?

Apologies for insisting in this way, and thanks again for maintaining TW

[jrabinow]

I added support for legacy file paths as requested. Please note that the changes here shouldn't be merged in to 2.6.0 branch until GothenburgBitFactory/libshared#25 has been reviewed and merged in, as otherwise these changes here will fail to create the new directories if the parent directories don't exist.

Once the other PR is merged in, I will create a new PR in taskwarrior that upgrades libshared to the latest version.

#2352

[tbabej]

@jrabinow First of all, let me thank you for all the effort you put in here. To me, the discussion around supporting the XDG standard really comes down to two quite distinct issues:

1. Should we attempt to make the XDG the default?
2. Should we try to give first-class experience to the users that want TW to comply with XDG?

While the answer to (1) can be subjective and ultimately comes down to user's preference (I for one, have no strong opinion, Paul does, and other participants that are raising the issue do as well, only the other way 🙂 ), the answer to the (2) is - yes, absolutely.

I think we can currently agree the user who wants TW to comply with XDG has to jump through a couple of hoops, in particular:

1. Location for the taskrc has to be overridden via an environment variable
2. The data.location configuration needs to be modified to the hardcoded path to the expanded ${XDG_DATA_HOME}/task/ because Taskwarrior currently does not support reading environment variables
3. To remove the message about the overridden taskrc location, user has to remove header from their verbosity settings.

That's objectively a lot of steps, especially for a wish that is likely to be encountered by first-time users (that are already trying to keep their $HOME tidy with XDG).

So what could we do to be more accomodating for XDG users?

1. Taskwarrior should check the $XDG_CONFIG_HOME/taskrc if the taskrc was not found in its default location
2. The data.location should be defined by the .taskrc. Now to support the dynamic values of XDG_DATA_HOME, we would need support for environment variables in the taskrc file, like [TW-1745] Allow for use of environment variables in config file #1769 suggests. On top of that, it would need to support the default values for such variables, so being able to parse data.location=${XDG_DATA_HOME:-~/.local/share}/task (syntax taken from shell parameter expansion syntax)

While (2) would take a bit more work, we can do (1) quite easily. Then making the transition easy would be a matter of putting the following into the default content of the taskrc file:

# Files
data.location=~/.task

# To use the default location of the XDG directories,
# move this configuration file from ~/.taskrc to ~/.config/taskrc and uncomment below

# data.location=~/.local/share/task
# hooks.location=~/.config/task/hooks

(with (1) implemented)

If we manage to get environment variable support into taskrc parsing, the default content of the taskrc file could look like:

# Files
data.location=~/.task

# To use the default location of the XDG directories,
# move this configuration file from ~/.taskrc to $XDG_CONFIG_HOME/taskrc and uncomment below

# data.location=${XDG_DATA_HOME:-~/.local/share}/task
# hooks.location=${XDG_CONFIG_HOME:-~/.config}/task/hooks

(with (2) implemented)

I'd be happy to help review and merge PRs implementing (1) and (if volunteers are found) (2) into the codebase. But having (1) implemented would already alleviate 80% of the pain experienced here for 20% of the effort.

[jrabinow]

Right, so 1/ is already implemented by this PR. I used a directory $XDG_CONFIG_HOME/task/taskrc in case we want to add additional config files in the future - but I can easily change that to $XDG_CONFIG_HOME/taskrc if you'd like.
I tried to implement all of this in the most straightforward way without rewriting too much, but I'm afraid the overall flow can still be confusing - if you have suggestions or comments on how to improve the implementation, they are quite welcome.

This PR also changes taskwarrior to check XDG_DATA_HOME envvar, I can remove that change if you'd like.
I will add changes for the header verbosity settings as soon as possible, most likely this week-end at earliest. Same thing for the default contents of taskrc, as well as revert to ~/.taskrc for new users and removing the documentation updates.
Please let me know if I missed anything

[tbabej]

I used a directory $XDG_CONFIG_HOME/task/taskrc in case we want to add additional config files in the future

No, you're right. This is the right way to do it, hooks will go into that directory as well (via a setting in the taskrc)

This file is only ever used if the current default ~/.taskrc isn't found - which also means it's the default for new users.

Let's keep the default for new users to ~/.taskrc for now. New users could also be just old users on new machines :)

This PR also changes taskwarrior to check XDG_DATA_HOME envvar, I can remove that change if you'd like.

Let;s remove this for now. A general environment variable mechanism in the taskrc (aka what we refer to as (2) above) would be preferred.

I will add changes for the header verbosity settings as soon as possible

This will not be actually needed, we are not actually doing an override through the env-var, so the header message should not be generated. I was just mentioning that above that this is what happens if one tries to get XDG setup working with using custom TASKRC environment variable.

Please let me know if I missed anything

I'd also remove the message warning users about using "legacy" taskrc location if they use ~/.taskrc, as the aim of (1) as discussed above is not to deprecate current directory layout, but to provide better support for XDG-driven alternative.

Thank you for the patience!

[tbabej]

@jrabinow Any chance of addressing the above? Given that I'd be happy to merge this :)

[jrabinow]

Hi @tbabej sorry I wanted to get to it this week-end, but unfortunately that proved optimistic on my end... will get to it this coming week-end at latest. Hopefully I can find some time this evening or tomorrow evening, but no promises

[jrabinow]

@tbabej ready for review :-)

┌─[01:43:40]─[julien@the_machine]
└──> ~/.../Code/taskwarrior/test(xdg_basedir_spec=|6m) $ >> ./problems
Failed:
lexer.t                             4

Unexpected successes:

Skipped:
dependencies.t                      3
export.t                            1
feature.default.project.t           4
hooks.on-modify.t                   1
import.t                            1
recurrence.t                        1
tw-1999.t                           2
wait.t                              1

Expected failures:
dependencies.t                      2
filter.t                            4
hyphenate.t                         1
project.t                           1
search.t                            1

[tbabej]

@jrabinow Julien, thanks for the hard work here. Tested and works as expected.

[jrabinow]

Hi @tbabej thank you for reviewing and merging (and sorry about the formatting 😬 )

I have a question about this commit. To quote the spec:
There is a single base directory relative to which user-specific configuration files should be written. This directory is defined by the environment variable $XDG_CONFIG_HOME.
$XDG_CONFIG_HOME defines the base directory relative to which user specific configuration files should be stored. If $XDG_CONFIG_HOME is either not set or empty, a default equal to $HOME/.config should be used.

To me, with the added commit, this reads as if I explicitly set XDG_CONFIG_HOME=$HOME/.config, TW will attempt to open $HOME/.config/.config/task/taskrc

As a result I believe this last commit deviates from the spec and should be reverted - or am I missing something?

[tbabej]

The commit fixes a bug (another instance of falling for GothenburgBitFactory/libshared#32), but introduces another, as you correctly point out 😅 Fixed in f0e34a7.

[jrabinow]

Got it, makes sense. Good catch!

[h0adp0re]

I'm sorry for necroing this but how would one go about using the features implemented here? Currently I still have to export TASKRC and TASKDATA for it to work. Then task tells me about the override every time when invoked.

[jrabinow]

https://github.com/GothenburgBitFactory/taskwarrior/blob/2.6.0/ChangeLog#L27 this changeset has not shipped yet. If you'd like to have the goods right away, the simplest option is probably to clone the repo and build the 2.6.0 branch

[h0adp0re]

Got it, thanks @jrabinow.