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

Support XDG base directory specification #109

Closed
Nikoli opened this issue Jun 12, 2013 · 30 comments
Closed

Support XDG base directory specification #109

Nikoli opened this issue Jun 12, 2013 · 30 comments

Comments

@Nikoli
Copy link
Contributor

Nikoli commented Jun 12, 2013

Please add support for XDG base directory specification.
http://standards.freedesktop.org/basedir-spec/basedir-spec-latest.html

@ghost
Copy link

ghost commented Jun 12, 2013

It's overcomplicated and doesn't really make anything better. You can get the same effect with setting the environment variable MPV_HOME=/home/user/.config/mpv (although that will still not be XDG conform, of course).

Rejected.

@ghost ghost closed this as completed Jun 12, 2013
@Nikoli
Copy link
Contributor Author

Nikoli commented Jun 12, 2013

It is just 3 vars with defaults. I think this is good explanation why xdg is useful:
https://bugs.launchpad.net/qbittorrent/+bug/502198/comments/2

mpv already has cache-like dir ~/.mpv/watch_later/.
Having configs in one dir ~/.config/ makes backups much simpler and keeps 'ls -la ~' output short enough. Turning ~/ into trash can is not the best thing to do. You do not need to be something big and gui like VLC to support XDG, now even such small tools like htop respect XDG:

$ du -sh /usr/bin/htop 
151K    /usr/bin/htop
$ ldd /usr/bin/htop 
        linux-vdso.so.1 (0x0000032102410000)
        libncursesw.so.5 => /lib64/libncursesw.so.5 (0x0000032101f89000)
        libm.so.6 => /lib64/libm.so.6 (0x0000032101c9b000)
        libc.so.6 => /lib64/libc.so.6 (0x00000321018f6000)
        libdl.so.2 => /lib64/libdl.so.2 (0x00000321016f2000)
        /lib64/ld-linux-x86-64.so.2 (0x00000321021f1000)
$ du -sb ~/.config/htop/htoprc 
620     /home/user/.config/htop/htoprc

@ghost
Copy link

ghost commented Jun 12, 2013

htop isn't conform to this XDG spec. For instance, it ignores XDG_CONFIG_DIRS.

If I support just XDG_CONFIG_HOME, people will without doubt come and whine that XDG_CONFIG_DIRS is not supported and demand that it is implemented. So I'd rather just go with the simpler choice and use the classic convention.

While storing config files under ~/.config is a quite simple idea, XDG goes further and turns everything into bloat as usual.

Turning ~/ into trash can is not the best thing to do.

Why is turning ~/.config into a trash can instead better?

@Nikoli
Copy link
Contributor Author

Nikoli commented Jun 12, 2013

If I support just XDG_CONFIG_HOME, people will without doubt come and whine that XDG_CONFIG_DIRS is not supported and demand that it is implemented.

Most likely nobody will ever notice even if you just change ~/.mpv/ to ~/.config/mpv/ without respecting $XDG_CONFIG_HOME.

Why is turning ~/.config into a trash can instead better?

Because it is in right place and hidden.
~/ is default dir for user, it is opened by default in a lot apps. ~/.config/ is never opened by default in any app. Yes, dot files and dirs are hidden by default, but users often disable this hiding. Seeing just .cache, .config and .local is much better then seeing hundreds of .myappname.
One more example: why /bin and /lib dirs exist, why not just put all files into /? Why turning /usr/bin/ and /usr/lib/ into trash can is better then turning /? Would you be happy to see binutils, cairo, icu, upower, wireshark and a lot other dirs in / instead of /usr/lib/?

@dubhater
Copy link
Contributor

Yes, dot files and dirs are hidden by default, but users often disable this hiding.

That's not mpv's problem, now, is it?

@Bilalh
Copy link
Contributor

Bilalh commented Jun 12, 2013

changing ~/.mpv/ to ~/.config/mpv/ would break people's current custom keybindings

@Nikoli
Copy link
Contributor Author

Nikoli commented Jun 13, 2013

That's not mpv's problem, now, is it?

Not supporting standards is mpv problem. If it possible to make user experience better, why not do?

changing ~/.mpv/ to ~/.config/mpv/ would break people's current custom keybindings

When VLC migrated to XDG paths, it automatically moved all configs to new locations. All settings were kept.

@jleclanche
Copy link
Contributor

If I support just XDG_CONFIG_HOME, people will without doubt come and whine that XDG_CONFIG_DIRS is not supported and demand that it is implemented.

Please don't assume that. There's almost no app out there that actually uses $XDG_CONFIG_DIRS. It's only useful if you want flexible system-wide settings.
$XDG_CONFIG_HOME is useful. It lets you categorize your config files separately. Version them, encrypt them, run backups. Moving your config there without respecting every other character in the basedir spec is not a sin; it's a huge bonus for some people. I gave further details in #142.

@ghost ghost reopened this Aug 15, 2013
@ghost
Copy link

ghost commented Aug 15, 2013

OK, maybe we'll consider implementing half of the spec. Or to be precise, implement the $XDG_CONFIG_HOME part only, which is indeed relatively simple.

It's usually not a good idea to implement standards only partially, but in this case it might be justified: most things on Linux don't implement it fully, and most people do not need the "extended" part of the basedir spec at all.

So I'd implement it like this (pseudo-code):

old_home = expand("$HOME/.mpv/");
xdg_home = expand("$XDG_CONFIG_HOME/mpv");
if (path_exists(old_home)) {
    print_warning("use mv %s %s migrate to new config home", old_home, xdg_home)
    config_home = old_home;
} else {
   config_home = xdg_home;
}

Opinions?

@jleclanche
Copy link
Contributor

Looks good (don't forget to check if $XDG_CONFIG_HOME is not defined, in which case you use $HOME/.config).

I'd take the deprecation a step further and actually move the config files to $xch/mpv and print("note: your files have been migrated to..."). Since mpv is a relatively recent project, you should be ok with that for a release or two.

@jleclanche
Copy link
Contributor

I have to run off for the night, but thank you for considering this! I know it doesn't seem like it but it's important to some people ;)

@ChrisK2
Copy link
Member

ChrisK2 commented Aug 15, 2013

Looks good (don't forget to check if $XDG_CONFIG_HOME is not defined, in which case you use $HOME/.config).

Yeah, $XDG_CONFIG_HOME is not defined on OSX but ~/.config/ exists for me since various other tools created it and just put their stuff there anyway. So I would suggest to check if ~/.config/ exists if $XDG_CONFIG_HOME isn't set and then just go with that.

@ghost
Copy link

ghost commented Aug 15, 2013

Yes, XDG_CONFIG_HOME isn't always set on Linux either, I just didn't bother including that in the pseudo-code.

Anyway... you're saying it should use ~/.config on OSX too?

@ChrisK2
Copy link
Member

ChrisK2 commented Aug 15, 2013

Yes please.

@Jessidhia
Copy link
Member

I'll try and implement this; will also try and use external libraries to do the heavy lifting because implementing everything in mpv itself looks like too much effort.

Also, the spec does say that if XDG_CONFIG_HOME isn't set that it should go to ~/.config. This is obvious retarded on Windows so I'll make up an adhoc "amendment" to the spec that makes sense in it (using appdata, etc).

@ghost
Copy link

ghost commented Aug 15, 2013

will also try and use external libraries to do the heavy lifting

In that case, don't waste your time.

because implementing everything in mpv itself looks like too much effort.

It shouldn't be much effort. It's literally just about concatenating an environment variable with a string. No thanks, I don't want the useless extra complexity of full xdg-basedir support.

@jleclanche
Copy link
Contributor

I'll try and implement this; will also try and use external libraries to do the heavy lifting because implementing everything in mpv itself looks like too much effort.

If it's too much effort then you're looking at supporting way too much. Just check the env variable and fall back to ~/.config.

As for windows, you can safely keep the old behaviour on there; basedir is not recommended for windows.

@Jessidhia
Copy link
Member

It shouldn't be much effort. It's literally just about concatenating an environment variable with a string.

That's the wrong behavior though.

As for windows, you can safely keep the old behaviour on there; basedir is not recommended for windows.

Considering its maintainers, it's not recommended for anything not-linux. However, putting the config file in a folder in the CWD is also wrong; there are specific system paths made for this.

@jleclanche
Copy link
Contributor

That's the wrong behavior though.

No, it's not. You're confusing "wrong" and "incomplete". Implementing a part of the xdg spec is not only possible, it's the recommended behaviour for standalone applications (as they rarely need the flexibility that the system-wide variables offer). I work with xdg specs a lot and I'm a big evangelist of basedir but I've never recommended anyone to implement the full spec.

Considering its maintainers, it's not recommended for anything not-linux. However, putting the config file in a folder in the CWD is also wrong; there are specific system paths made for this.

Not sure where you got CWD from. %APPDATA% is the standard place for it on Windows. And the basedir spec is used by plenty of Linux->OSX ports so it's not necessarily a bad idea to use it there, but it would be more standard to use ~/Library/Preferences afaik.

@ghost
Copy link

ghost commented Aug 16, 2013

On OSX, we normally use the Linux convention, or if the OSX bundle is used, a native OSX API for the config path.

On Windows, some users want to start mpv out of a single directory, which means the config file will be next to mpv.exe, instead of in a system defined config dir.

@rossy
Copy link
Member

rossy commented Aug 16, 2013

The situation on Windows seems pretty straightforward to me if the exe directory is treated the same as the bundle directory on OSX. If I understand path.c correctly, it checks the user config path, an optional bundle path and then the system config path.

If the user config path is $XDG_CONFIG_HOME/mpv on Linux, I think it should be %APPDATA%\mpv on Windows. When it searches the bundle directory on OSX, it should search the application directory on Windows, and when searching the system directory $sysconfdir/mpv on Linux, it could search %PROGRAMDATA%\mpv on Windows. This makes %APPDATA% on Windows the same as $XDG_CONFIG_HOME on systems that use XDG base dirs, which might simplify the logic a bit.

I'm not sure if it should try reading %XDG_CONFIG_HOME% on Windows as well. Cygwin users might have $HOME and $XDG_CONFIG_HOME set for other programs and they probably wouldn't expect a native Windows program to read them. Currently mpv reads %HOME% on Windows and uses it in preference to the exe directory, but maybe it shouldn't.

@Jessidhia
Copy link
Member

It's on a separate branch for now; if that implementation is OK I'll merge to master and close the bug.

It now ignores HOME on native windows, but MPV_HOME still has priority over everything else.

@Jessidhia
Copy link
Member

Yay for forgotten debug code :D

@Jessidhia
Copy link
Member

Updated in response to @wm4's comments.

Jessidhia pushed a commit that referenced this issue Aug 18, 2013
Only uses XDG_CONFIG_HOME and XDG_CACHE_HOME. Appropriate, equivalent(ish)
behavior is implemented for Windows. Cygwin is treated as not-windows. The
watch_later functionality now stores its files in the cache dir.

Still supports the ~/.mpv path if it exists, but warns about the change to
the new one. That codepath is toggleable by the SUPPORT_OLD_CONFIG internal
define, in case it's ever wanted. Note that it is ignored if it's a
symlink, to allow symlinking to the new path to support older versions.

On Windows, the config files are now stored on %AppData%\mpv, unless there
is an "mpv" dir in the same folder as the mpv.exe. Cache files always go
on %LocalAppData%\mpv, but it's possible to make it put the files in the
exe-local "mpv" dir by changing the ALWAYS_LOCAL_APPDATA define to 0.
@jleclanche
Copy link
Contributor

What's the status on this? will it be merged in?

@ghost
Copy link

ghost commented Feb 9, 2014

Ask @Kovensky.

@ghost ghost added wontfix and removed wip labels Apr 8, 2014
@ghost
Copy link

ghost commented Apr 8, 2014

Empirically, Kovensky has no interest in finishing this, so this is a WONTFIX.

@ghost ghost closed this as completed Apr 8, 2014
@jleclanche
Copy link
Contributor

That's annoying. I don't see the benefit in closing this though, it's still a valid request :/

@ghost ghost added developer-needed and removed wontfix labels May 17, 2014
@ghost
Copy link

ghost commented May 17, 2014

Well ok...

Still needs someone to implement this, though.

@ghost ghost reopened this May 17, 2014
@ghost ghost mentioned this issue Jun 19, 2014
ghost pushed a commit that referenced this issue Jun 26, 2014
Search $XDG_CONFIG_HOME and $XDG_CONFIG_DIRS for config files.
This also negates the need to have separate user and global variants of
mp_find_config_file()

Closes #864, #109.

Signed-off-by: wm4 <wm4@nowhere>
@ghost
Copy link

ghost commented Jun 30, 2014

Obsoleted.

@ghost ghost closed this as completed Jun 30, 2014
This issue was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

7 participants