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

player: set default cache dir on macOS #11939

Merged
merged 1 commit into from
Jul 15, 2023
Merged

player: set default cache dir on macOS #11939

merged 1 commit into from
Jul 15, 2023

Conversation

m154k1
Copy link
Contributor

@m154k1 m154k1 commented Jul 14, 2023

Since shader/icc caches are enabled by default now ~/.config/mpv may contain a lot of cache files on macOS. Most likely, users want to have only configuration files here so change default cache path to macOS specific directory.

@Dudemanguy
Copy link
Member

Should probably be after #11918. Instead of modifying that section in mpv.rst. It would probably be better to make a separate FILES ON MACOS part like how there is one for Windows. Also cc @Akemi.

@Akemi
Copy link
Member

Akemi commented Jul 14, 2023

i totally don't see a reason to use any env variables completely specific to linux and have nothing to do with any specifications of macOS.

we rejected such changes previously. either we do it properly and migrate all paths to macOS conventions, which in return will raise the ire of one group of mac users, or do it completely as linux, which will raise the ire of the other group.

either we don't change anything or do it properly the macOS way. a half assed middle way is imo something we should not desire. personally i always wanted to change everything to the macOS standard ways but was also stopped back in the days.

changing everything also might have the side effect of breaking all existing configs/setups.

@Dudemanguy
Copy link
Member

Yeah I dunno about the environment variables and if anyone actually uses them on macOS. I don't think adding at least a cache directory hurts. I don't think anybody wants cache in their config (and it wasn't there before).

@m154k1
Copy link
Contributor Author

m154k1 commented Jul 14, 2023

@Akemi Hmm. If we choose to fully switch to macOS conventions, how the paths should look like? Maybe something like this:
~~home = ~/Library/Application Support/io.mpv/config
~~cache = ~/Library/Caches/io.mpv
~~state = ~/Library/Application Support/io.mpv/state

Or just implement only cache dir for now like @Dudemanguy suggested?

@m154k1 m154k1 changed the title player: use XDG base directories on macOS @m154k1 player: set default cache dir on macOS Jul 14, 2023
@m154k1 m154k1 changed the title @m154k1 player: set default cache dir on macOS player: set default cache dir on macOS Jul 14, 2023
@m154k1
Copy link
Contributor Author

m154k1 commented Jul 14, 2023

I've removed env variables (except config one which was there before) and set ~~cache to ~/Library/Caches/io.mpv. Probably should be fine for now. If it's ok I'll update doc after #11918.

@m154k1
Copy link
Contributor Author

m154k1 commented Jul 15, 2023

@Akemi @Dudemanguy I've updated the doc. Please, let me know if it's ok to go with only cache dir change. This shouldn't harm users and allow us to implement other macOS-specific dirs in future.

DOCS/man/mpv.rst Outdated Show resolved Hide resolved
DOCS/man/mpv.rst Outdated Show resolved Hide resolved
osdep/path-darwin.c Outdated Show resolved Hide resolved
Use ~/Library/Caches/io.mpv for caches instead of ~~home.
@m154k1
Copy link
Contributor Author

m154k1 commented Jul 15, 2023

@Dudemanguy updated, thank you :)

Copy link
Member

@Dudemanguy Dudemanguy left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, @Akemi is this sensible to you?

@Akemi
Copy link
Member

Akemi commented Jul 15, 2023

@Akemi Hmm. If we choose to fully switch to macOS conventions, how the paths should look like? Maybe something like this:
~~home = ~/Library/Application Support/io.mpv/config
~~cache = ~/Library/Caches/io.mpv
~~state = ~/Library/Application Support/io.mpv/state

if we ever do this this seems okay.

LGTM, @Akemi is this sensible to you?

yes this is very reasonable. the cache files shouldn't be in the config dir at all.

just mentioning it, i don't have a working mac right now and can't test anything. so this would need merging without testing from my side.

@Dudemanguy
Copy link
Member

@m154k1: Thanks for taking care of this!

@Dudemanguy Dudemanguy merged commit b077cf7 into mpv-player:master Jul 15, 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.

3 participants