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

Better integration with xdg dirs. #10838

Merged
merged 4 commits into from
May 9, 2023
Merged

Conversation

Dudemanguy
Copy link
Member

@Dudemanguy Dudemanguy commented Nov 8, 2022

Fixes #9147 and also supercedes #8184.

This basically does four things here.

  1. Splits off the paths for macos into its own separate file (path-darwin.c).
  2. Changes the internal path selection to allow for selecting different types (i.e. so you can get more than just the config directory.
  3. Moves the watch_later directory to XDG_STATE_DIR (no change for the stuff that doesn't use path-unix.c).
  4. Sets the cache directory to XDG_CACHE_DIR and it's used by default if applicable options aren't explicitly set (--cache-dir, --icc-cache-dir, and --gpu-shader-cache-dir). On non-linux/bsd OSes, this means cache would start being saved by default in the default config dir so it's a little more invasive than the other changes. For the icc and gpu shaders, cache being saved or not is now controlled with the --icc-cache and --gpu-shader-cache options.

@Dudemanguy
Copy link
Member Author

It's HOME in the code but yeah let me fix the commit messages.

@Dudemanguy Dudemanguy force-pushed the xdg-dirs branch 2 times, most recently from f66ff9e to fe31c8b Compare November 15, 2022 23:48
@txtsd
Copy link

txtsd commented Nov 29, 2022

Came here to make an issue about watch_later being in $XDG_CONFIG_HOME. Glad to see it's already being worked on. Hopefully it will be in the next release.

Can you also make it so the existing watch_later folder is moved from $XDG_CONFIG_HOME if found?

@Dudemanguy
Copy link
Member Author

Can you also make it so the existing watch_later folder is moved from $XDG_CONFIG_HOME if found?

Not impossible of course, but that strikes me as unnecessary. The hypothetical move should only happen on certain platforms and should only happen once. I think it's better for the user to just move the directory themselves instead of trying to code this.

@txtsd
Copy link

txtsd commented Nov 29, 2022

Fair enough. Thought it was worth asking.

@Dudemanguy
Copy link
Member Author

I reworked the last commit so it instead adds two new options: --icc-cache and --gpu-shader-cache. These are both off by default (i.e. no cache is saved). If you enable those and leave the respective directory options unset (--icc-cache-dir and --gpu-shader-cache-dir), then it will save the files in XDG_CACHE_HOME now. This matches the --cache and --cache-dir behavior.

@Dudemanguy Dudemanguy force-pushed the xdg-dirs branch 5 times, most recently from 08cf800 to 6c2213e Compare January 4, 2023 21:41
@Dudemanguy
Copy link
Member Author

Dudemanguy commented Jan 4, 2023

I've rewritten the implementation, and this approach now has a few differences.

  • The global --config-dir option now has no effect on cache or state data. On my previous implementation, setting this option would always force everything to be written into that directory.
  • --cache-dir is now a global option essentially equivalent to --config-dir but for cache. This deprecates --icc-cache-dir and --gpu-shader-cache-dir (they alias to --cache-dir now).

Doesn't really matter to users, but internally the path selection api now takes into account these potential global paths (--cache-dir, --config-dir, and --watch-later-directory). So different platforms do not have to actually write code for these path types. It will fallback to "home" (aka the config directory) if it does not exist.

Seems to work just fine for me (linux) but more testing would be appreciated. Especially on macOS.

@Dudemanguy Dudemanguy force-pushed the xdg-dirs branch 4 times, most recently from 2d929fc to 826c410 Compare January 10, 2023 05:22
@Dudemanguy Dudemanguy force-pushed the xdg-dirs branch 3 times, most recently from f2d8661 to 1c02ff6 Compare March 2, 2023 16:03
@Dudemanguy Dudemanguy force-pushed the xdg-dirs branch 2 times, most recently from c7d78c9 to 59ac119 Compare April 29, 2023 19:09
DOCS/interface-changes.rst Outdated Show resolved Hide resolved
Dudemanguy added 3 commits May 2, 2023 17:53
macOS really has completely different path conventions that mpv doesn't
take into account and it treats it just like any other old unix-like
system. This means mpv enforces certain conventions on it (like all the
XDG stuff) that doesn't really apply. Since we'd like to use more of
this but at the same time not distrupt mac users even more, let's just
copy and paste the current code to a new file, update the build and call
it a day. This way, the paths of these two platforms can more freely
diverge.
Currently, nothing new is actually implemented but the idea is simply to
just pass a type string all the way up from mp_find_user_file down to
actually getting the platform path. This allows for selecting different
directories besides the user's native config directory. See the next
commit for an implementation.
A pain point for some users is the fact that watch_later is stored in
the ~/.config directory when it's really not configuration data. Roughly
2 years ago, XDG_STATE_DIR was added to the XDG Base Directory
Specification[0] and its description, user-specific state data, actually
perfectly matches what watch_later data is for. Let's go ahead and use
this directory as the default for watch_later. This change only affects
non-darwin unix-like systems (i.e. Linux, BSDs, etc.). The directory
doesn't move for anyone else.

Internally, quite a few things change with regards to the path
selection. If the platform in question does not have a statedir concept,
then the path selection will simply return "home" instead (old
behavior). Fixes mpv-player#9147.

[0]: https://gitlab.freedesktop.org/xdg/xdg-specs/-/commit/4f2884e16db35f2962d9b64312917c81be5cb54b
@Dudemanguy
Copy link
Member Author

I realized my previous approach was flawed with regards to runtime changes of the directories, so I rewrote it yet again. This time, the --icc-cache-dir and --gpu-shader-cache-dir options are preserved because it turns out it's easier to keep them, and I also rely less on the global state. I'll try to get more testing in later to be sure, but hopefully this goes in soon.

@Dudemanguy Dudemanguy force-pushed the xdg-dirs branch 4 times, most recently from 643cc21 to 5d261e8 Compare May 9, 2023 16:00
This adds cache as a possible path for mpv to internally pick
(~/.cache/mpv for non-darwin unix-like systems, the usual config
directory for everyone else). For gpu shader cache and icc cache,
controlling whether or not to write such files is done with the new
--gpu-shader-cache and --icc-cache options respectively. Additionally,
--cache-on-disk no longer requires explicitly setting the --cache-dir
option. The old options, --cache-dir, --gpu-shader-cache-dir, and
--icc-cache-dir simply set an override for the directory to save cache
files. If unset, then the cache is saved in XDG_CACHE_HOME.
@Dudemanguy Dudemanguy merged commit 4502522 into mpv-player:master May 9, 2023
@Dudemanguy Dudemanguy deleted the xdg-dirs branch May 9, 2023 20:37
@txtsd
Copy link

txtsd commented May 10, 2023

Awesome!

@sfan5 sfan5 mentioned this pull request May 21, 2023
kmk3 pushed a commit to netblue30/firejail that referenced this pull request Aug 3, 2023
The new version of mpv changed the path of the watch_later folder to
~/.local/state/mpv/watch_later.

See mpv-player/mpv#10838
kmk3 added a commit to kmk3/firejail that referenced this pull request Aug 3, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Move watch_later to $XDG_STATE_HOME
3 participants