Properly use XDG specs #2592
Properly use XDG specs #2592
Conversation
|
Oh great, I'd been waiting until we supported only the versions of OTP and above that included the XDG stuff to bother with this, and this was added in 19, so good to go! |
|
Except for all the tests failing :) |
Before setting `XDG_{CONFIG,CACHE}_HOME` didn't changed Rebar3 behaviour
which would still, forcefully, use `$HOME/.config` and `$HOME/.cache`
respectively. Now it will properly read these environment variables and
work accordingly.
hauleth
force-pushed
the
fix/properly-use-xdg-paths
branch
from
37fdf32
to
49e6d13
Compare
|
@tsloughter you need to reprove workflows. |
|
I've got to lookinto stopping it from making me manually allow those.. |
|
@tsloughter after this will merge you will no longer need to manually allow these for me, but you still will need that for other first-time contributors as it is security measure against malicious 3rd parties that used GitHub Actions for crypto mining and other dubious stuff from the GitHub and maintainers perspective. |
| @@ -133,16 +133,22 @@ home_dir() -> | |||
| {ok, [[Home]]} = init:get_argument(home), | |||
| Home. | |||
|
|
|||
| basedir(Type) -> | |||
| filename:basedir(Type, "rebar3", #{os => linux}). | |||
There was a problem hiding this comment.
Oh, hm, if we are going to do this we probably shouldn't hardcode linux. It may change where stuff goes in windows and osx but probably better to be right than continue being wrong? But not sure...
@ferd thoughts?
There was a problem hiding this comment.
Replace my "may" with "will". https://erlang.org/doc/man/filename.html#basedir-3
There was a problem hiding this comment.
I am on macOS and I prefer my development tooling to use XDG rather than macOS practices ;)
There was a problem hiding this comment.
This may unfortunately blow out things in the global rebar.config file and existing hex auth and be considered backwards incompatible in very surprising ways.
If we're to support the new stuff, it should ideally come with supporting both at once with a warning or information message asking the user to move content, which they may hate if they have to use various rebar3 versions for various OTP support (or when building with mix, which bundles older ones).
Forcing the linux path likely keeps things compatible with the current mechanism on all operating systems though, since it's what we forced everywhere (would have to double check windows)
There was a problem hiding this comment.
Yea. I suppose we can wait for rebar4.
There was a problem hiding this comment.
@ferd in what way it can blow things? It would only change behaviour if user have set XDG_CONFIG_HOME.
There was a problem hiding this comment.
Using it with the linux path everywhere should remain the same although there's a check needed for windows because I don't know if they're fully equivalent. Following the basedir fully would break stuff.
There was a problem hiding this comment.
@ferd what check for windows are you referring to?
I think this PR is good to merge except for a question I have on line 151.
There was a problem hiding this comment.
I think we're fine if we keep forcing the linux format. I just double-checked, the thing that worried me is that there could be a conversion issue between what was considered the home_dir() in windows as of today vs. how the Linux path would translate.
I did a check on my old win10 machine and it seems to be identical and good to go.
|
|
||
| rebar_config_dir(State) -> | ||
| case os:getenv("REBAR_GLOBAL_CONFIG_DIR") of | ||
| false -> | ||
| rebar_state:get(State, global_rebar_dir, home_dir()); | ||
| rebar_state:get(State, global_rebar_dir, undefined); |
There was a problem hiding this comment.
Why is this now undefined?
Before setting
XDG_{CONFIG,CACHE}_HOMEdidn't changed Rebar3 behaviour which would still, forcefully, use$HOME/.configand$HOME/.cacherespectively. Now it will properly read these environment variables and work accordingly.