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

add io.mpv.Mpv #1057

Closed
wants to merge 18 commits into from
Closed

add io.mpv.Mpv #1057

wants to merge 18 commits into from

Conversation

paulcarroty
Copy link

@gasinvein
Copy link
Member

I believe the ID should be io.mpv.Mpv. Currently, it suggest that github.com domain is owned by Mpv.

@flathubbot
Copy link

Queued test build for io.mpv.Mpv.

@flathubbot
Copy link

Started test build 5095

@flathubbot
Copy link

Build 5095 failed

io.mpv.Mpv.json Outdated Show resolved Hide resolved
@flathubbot
Copy link

Queued test build for io.mpv.Mpv.

@flathubbot
Copy link

Started test build 5098

@flathubbot
Copy link

Build 5098 failed

Copy link

@Erick555 Erick555 left a comment

Choose a reason for hiding this comment

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

Please update dependencies

io.mpv.Mpv.json Outdated Show resolved Hide resolved
io.mpv.Mpv.json Outdated Show resolved Hide resolved
io.mpv.Mpv.json Outdated Show resolved Hide resolved
@nedrichards nedrichards changed the title add io.github.Mpv add io.mpv.Mpv Jul 3, 2019
@nedrichards
Copy link
Member

bot, build io.mpv.Mpv

@flathubbot
Copy link

Queued test build for io.mpv.Mpv.

@flathubbot
Copy link

Started test build 5100

@flathubbot
Copy link

Build 5100 failed

@AsciiWolf
Copy link
Contributor

bot, build io.mpv.Mpv

@flathubbot
Copy link

Queued test build for io.mpv.Mpv.

@flathubbot
Copy link

Started test build 5102

@flathubbot
Copy link

Build 5102 failed

@AsciiWolf
Copy link
Contributor

bot, build io.mpv.Mpv

@flathubbot
Copy link

Queued test build for io.mpv.Mpv.

@flathubbot
Copy link

Started test build 5104

@flathubbot
Copy link

Build 5138 successful
To test this build, install it from the testing repository:

flatpak install --user https://dl.flathub.org/build-repo/4919/io.mpv.Mpv.flatpakref

@flathubbot
Copy link

Queued test build for io.mpv.Mpv.

@flathubbot
Copy link

Started test build 5173

@nedrichards
Copy link
Member

OK then, so the only thing that stands in the way is an upstream PR pushing the appdata file - since that's generally useful for everyone on Linux, not just flatpak. Sad that upstream don't want to be involved, do you have a list of maintainers, just @paulcarroty?

It'd be nice to send a PR to https://github.com/mpv-player/mpv.io as well once this is added to get on this page https://mpv.io/installation/

@TingPing
Copy link
Member

TingPing commented Jul 4, 2019

I imagine the reason upstream prefers snap is because its primary used from the cli and flatpak requires a different command, making all existing docs/workflows unusable.

@flathubbot
Copy link

Build 5173 successful
To test this build, install it from the testing repository:

flatpak install --user https://dl.flathub.org/build-repo/4954/io.mpv.Mpv.flatpakref

@nedrichards
Copy link
Member

bot, build io.mpv.Mpv

@flathubbot
Copy link

Queued test build for io.mpv.Mpv.

@flathubbot
Copy link

Started test build 5184

@flathubbot
Copy link

Build 5184 successful
To test this build, install it from the testing repository:

flatpak install --user https://dl.flathub.org/build-repo/4966/io.mpv.Mpv.flatpakref

@AsciiWolf
Copy link
Contributor

bot, build io.mpv.Mpv

@flathubbot
Copy link

Queued test build for io.mpv.Mpv.

@flathubbot
Copy link

Started test build 5191

"--device=all",
"--share=network",
"--socket=pulseaudio",
"--filesystem=host",
Copy link
Member

Choose a reason for hiding this comment

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

Does MPV ever need write access? If not,

Suggested change
"--filesystem=host",
"--filesystem=host:ro",

Copy link
Member

Choose a reason for hiding this comment

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

Plugins are stored in MPV config dir, which is mounted read-writable even with host:ro.

Copy link
Member

Choose a reason for hiding this comment

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

Can you point to a specific plugin that writes to a directory outside xdg-config/xdg-data?

Copy link
Member

Choose a reason for hiding this comment

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

Isn't it too specific use-case to expose whole filesystem for all users? Users who actually need write access to host, may add it via overrides.

Copy link
Member

Choose a reason for hiding this comment

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

It'd be appreciated if you could stop resolving conversations that are still in progress.

Choose a reason for hiding this comment

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

Screenshot needs write access, though I guess only to xdg-pictures.

@flathubbot
Copy link

Build 5191 successful
To test this build, install it from the testing repository:

flatpak install --user https://dl.flathub.org/build-repo/4973/io.mpv.Mpv.flatpakref

@barthalion
Copy link
Member

Have you submitted appdata upstream or you will pretend no one asked for that?

@barthalion
Copy link
Member

Appdata is not specific to flatpak.

@nedrichards
Copy link
Member

@paulcarroty specifically an upstream PR #1057 (comment) it doesn't have to be merged but it does have to be offered.

@gasinvein
Copy link
Member

gasinvein commented Jul 5, 2019

Where does the info about that MPV developers prefer snap comes from, btw? I didn't find neither official snap package, nor issues/PRs related to it. There is only request to provide AppImage, closed with wontfix.

@TingPing
Copy link
Member

TingPing commented Jul 5, 2019

mpv-player/mpv#2987

I think its worth trying again with more detail and a PR. In those 3 years every distros app store supports this standard now.

@TingPing
Copy link
Member

We've decided to not move forward with this at the moment.

You have a history of abandoning packages here and have not been the easiest to work with so we would prefer a different maintainer for this application.

I'll leave this open for a bit if anybody else wants to step up.

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.

10 participants