castwide / solargraph

A Ruby language server.
https://solargraph.org
MIT License
1.87k stars 154 forks source link

Comply with XDG Base directory specification #664

Closed jasonkarns closed 4 months ago

jasonkarns commented 1 year ago

I'm stumbled here after running a project for a current client and having a .solargraph/ directory suddenly show up in my home directory.

I haven't used solargraph or know what it is, though (after some digging) discovered that it's listed in our project's Gemfile. Anyway, the basedir specification has been out for quite a while, and it would be beneficial to developers if this tool were to comply with the specification.

Config files should go in $XDG_CONFIG_HOME, cache in $XDG_CACHE_HOME and any persisted data to $XDG_DATA_HOME.

alichtman commented 9 months ago

I'm here for the same reason.

The fix should be made here: https://github.com/castwide/solargraph/blob/0b65280d3efdbd07d608517328476d9ed267d4b6/lib/solargraph/cache.rb#L12

If the default ~/.solargraph already exists, use that, otherwise use $XDG_CACHE_HOME/solargraph

jasonkarns commented 8 months ago

bump

castwide commented 8 months ago

I think we need to take a slightly different tack. Use XDG_CACHE_HOME if it's set, otherwise, use ~/.solargraph. The reasoning is that the directory will be created if it doesn't exist.

If that behavior seems appropriate, the change is on master and will be included in the next release (later this month).

alichtman commented 8 months ago

With that implementation, you may have people with both a ~/.solargraph directory and an XDG solargraph directory. I'd suggest migrating ~/.solargraph if it exists and the XDG vars are set.

castwide commented 8 months ago

Migration seems like an unnecessary step. Worst case, some users have an old cache they're not using anymore. It shouldn't be a problem because the cache is fungible. The Cache module will save the cache in the preferred location, and the old cache is safe to discard. If you see a reason a migration would be necessary, I'm willing to reconsider.

alichtman commented 8 months ago

Thanks for taking care of this issue!

jasonkarns commented 8 months ago

Use XDG_CACHE_HOME if it's set, otherwise, use ~/.solargraph

This approach (and the implementation in 37ae8560385c3dd989d02a60ecb2ee3b5bac0015) is not compliant with the XDG specification! The spec defines default location for the XDG_CACHE_HOME variable; meaning the env var is not required to be set.

A safer approach, is to first check for the existence of the old directory (~/.solargraph). If it exists, use it. (Thus current installations will not change behaviors based on env var presence.) If the directory does not exist, use the XDG_CACHE_HOME location (with the spec-defined default if env var is unset).

This approach does not alter current installations; maintains existing caches (no migration necessary), is compliant with the XDG spec, and treats the spec-compliant directory as the preferred location moving forward. Aware-users would simply need to manually delete the old ~/.solargraph directory to be upgraded to the new, compliant location and behavior.

jasonkarns commented 7 months ago

For clarity, the specification defines the default for XDG_CACHE_HOME:

If $XDG_CACHE_HOME is either not set or empty, a default equal to $HOME/.cache should be used.

https://specifications.freedesktop.org/basedir-spec/basedir-spec-latest.html

castwide commented 7 months ago

@jasonkarns Noted. The default without any configuration is the XDG standard $HOME/.cache/solargraph. Users can still override it with SOLARGRAPH_CACHE if necessary (which I would expect to be rare).

castwide commented 7 months ago

Released in v0.50.0.