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

feat: added update command to update uosc #700

Merged
merged 4 commits into from
Oct 19, 2023
Merged

feat: added update command to update uosc #700

merged 4 commits into from
Oct 19, 2023

Conversation

tomasklaen
Copy link
Owner

updater.mp4

If anyone can test it on linux/mac, it'd be great, but be aware that it's going to replace your current uosc in ~/.config/mpv with the latest release, which is 4.7.

The only issue this has now (that I know of) is that to do a windows portable update, I need to know the path to mpv binary so the updating process can run in its working directory.

Is there a way to get it somehow? And is there some arg to mp.command_native_async() to set the working directory? Can't find info on any of these. But I haven't tried super hard, so hopefully it's still doable :)

@dyphire
Copy link
Contributor

dyphire commented Oct 14, 2023

I prefer to use this mpv_manager script to uniformly manage mpv scripts and shader updates, which is simpler and more convenient. Maybe you just need to link it in the Docs and provide a usage method.
Example:

[
  {
    "git":"https://github.com/tomasklaen/uosc",
    "whitelist":"scripts$",
    "dest":"~~/scripts",
    "branch":"main"
  }
]

@xfzv
Copy link
Contributor

xfzv commented Oct 14, 2023

I'm also using another method to keep uosc updated but I think that it's a very nice addition.

I'm a Linux user so I've just tried it. Maybe I'm doing something wrong but it doesn't seem to work on my end.

After running script-binding uosc/update I'm getting this:

image

After restarting mpv, it looks like I'm still on the updater branch, not on 4.7. I can still run script-binding uosc/update and I get the same behavior than on the screenshot.


Edit: I've just tried the unix.sh installation script and it works just fine (directories/files from the updater branch are replaced with 4.7 ones):

./unix.sh
Deleting old and deprecated uosc files and directories.
Downloading: https://github.com/tomasklaen/uosc/releases/latest/download/uosc.zip
  % Total    % Received % Xferd  Average Speed   Time    Time     Time  Current
                                 Dload  Upload   Total   Spent    Left  Speed
  0     0    0     0    0     0      0      0 --:--:-- --:--:-- --:--:--     0
  0     0    0     0    0     0      0      0 --:--:-- --:--:-- --:--:--     0
100  278k  100  278k    0     0   355k      0 --:--:-- --:--:-- --:--:--  864k
Extracting: /tmp/uosc.zip
Archive:  /tmp/uosc.zip
  inflating: /home/xfzv/.config/mpv/fonts/uosc_icons.otf
  inflating: /home/xfzv/.config/mpv/fonts/uosc_textures.ttf
  inflating: /home/xfzv/.config/mpv/scripts/uosc.lua
   creating: /home/xfzv/.config/mpv/scripts/uosc_shared/
   creating: /home/xfzv/.config/mpv/scripts/uosc_shared/elements/
  inflating: /home/xfzv/.config/mpv/scripts/uosc_shared/elements/BufferingIndicator.lua
  inflating: /home/xfzv/.config/mpv/scripts/uosc_shared/elements/Button.lua
  inflating: /home/xfzv/.config/mpv/scripts/uosc_shared/elements/Controls.lua
  inflating: /home/xfzv/.config/mpv/scripts/uosc_shared/elements/Curtain.lua
  inflating: /home/xfzv/.config/mpv/scripts/uosc_shared/elements/CycleButton.lua
  inflating: /home/xfzv/.config/mpv/scripts/uosc_shared/elements/Element.lua
  inflating: /home/xfzv/.config/mpv/scripts/uosc_shared/elements/Elements.lua
  inflating: /home/xfzv/.config/mpv/scripts/uosc_shared/elements/Menu.lua
  inflating: /home/xfzv/.config/mpv/scripts/uosc_shared/elements/PauseIndicator.lua
  inflating: /home/xfzv/.config/mpv/scripts/uosc_shared/elements/Speed.lua
  inflating: /home/xfzv/.config/mpv/scripts/uosc_shared/elements/Timeline.lua
  inflating: /home/xfzv/.config/mpv/scripts/uosc_shared/elements/TopBar.lua
  inflating: /home/xfzv/.config/mpv/scripts/uosc_shared/elements/Volume.lua
  inflating: /home/xfzv/.config/mpv/scripts/uosc_shared/elements/WindowBorder.lua
   creating: /home/xfzv/.config/mpv/scripts/uosc_shared/lib/
  inflating: /home/xfzv/.config/mpv/scripts/uosc_shared/lib/ass.lua
  inflating: /home/xfzv/.config/mpv/scripts/uosc_shared/lib/menus.lua
  inflating: /home/xfzv/.config/mpv/scripts/uosc_shared/lib/std.lua
  inflating: /home/xfzv/.config/mpv/scripts/uosc_shared/lib/text.lua
  inflating: /home/xfzv/.config/mpv/scripts/uosc_shared/lib/utils.lua
  inflating: /home/xfzv/.config/mpv/scripts/uosc_shared/main.lua
Deleting: /tmp/uosc.zip
uosc has been installed.

@xfzv
Copy link
Contributor

xfzv commented Oct 14, 2023

Maybe there's something wrong here?

cff7fa8#diff-b3987901b1d4877a672cb09d6728c46ad4964868098aa7dc5d589fd0d7785f3bR19-R20

I don't know how the command is actually executed from the Updater.lua file but I think the double quotes might be missing.

  • This doesn't do anything (no quotes):
$ /bin/bash -c $(curl -fsSL https://raw.githubusercontent.com/tomasklaen/uosc/HEAD/installers/unix.sh)
  • This fails (single quotes):
$ /bin/bash -c '$(curl -fsSL https://raw.githubusercontent.com/tomasklaen/uosc/HEAD/installers/unix.sh)'
/bin/bash: line 1: #!/bin/bash: No such file or directory
  • This works (double quotes):
$ /bin/bash -c "$(curl -fsSL https://raw.githubusercontent.com/tomasklaen/uosc/HEAD/installers/unix.sh)"
Deleting old and deprecated uosc files and directories.
Downloading: https://github.com/tomasklaen/uosc/releases/latest/download/uosc.zip
  % Total    % Received % Xferd  Average Speed   Time    Time     Time  Current
                                 Dload  Upload   Total   Spent    Left  Speed
  0     0    0     0    0     0      0      0 --:--:-- --:--:-- --:--:--     0
  0     0    0     0    0     0      0      0 --:--:-- --:--:-- --:--:--     0
100  278k  100  278k    0     0   287k      0 --:--:-- --:--:-- --:--:-- 4884k

[...]

The README mentions using double quotes by the way:

uosc/README.md

Line 58 in 3e685c5

/bin/bash -c "$(curl -fsSL https://raw.githubusercontent.com/tomasklaen/uosc/HEAD/installers/unix.sh)"

@tomasklaen
Copy link
Owner Author

tomasklaen commented Oct 15, 2023

The quotes are usually omitted when string is passed as an array argument. Just as there are no quotes in powershell command above either, and it works fine. But duno, maybe it's different for bash? Did you try adding the quotes to the args array?

All right, I'll install some linux VM, and test it later.

@po5
Copy link
Contributor

po5 commented Oct 15, 2023

In my case it seems to be caused by the wrong shebang:
[uosc] bash: line 1: #!/bin/bash: No such file or directory
This is the proper portable shebang, same used in thumbfast:
#!/usr/bin/env bash
You'll want to update the bash call in Updater.lua as well.

@tomasklaen
Copy link
Owner Author

I'll look into it more tomorrow, but in the meantime:

@po5 you seem to have a lot of experience with running subprocesses from mpv scripts :) Is it possible to:

  • Get the stdout as it is coming in asynchronously? Currently the updater just uses mp.command_native_async() which just accumulates the whole stdout and dumps it to the final callback. I'd like to display the progress messages so it doesn't feel frozen.
  • Get path to directory with current mpv binary?

@christoph-heinrich
Copy link
Contributor

christoph-heinrich commented Oct 15, 2023

idk about having scripts update themselves. It's very reminiscent of the windows ecosystem where each application has it's own update mechanism built in (although I heard something that package mangers are becoming more common?).

I guess it could be useful for people that only use/update uosc and no other scripts.

@tomasklaen
Copy link
Owner Author

It's very reminiscent of the windows ecosystem where each application has it's own update mechanism built in

Yeah I absolutely hate it too, but what else are they supposed to do when there's no package manager? Windows store absolutely sucks.

And it's the same here. If I could make the update screen be a UI for some mpv script manager, I would, but there's nothing established or not abandoned, let alone something with an API. So at least I make it for uosc.

I guess it could be useful for people that only use/update uosc and no other scripts.

That's exactly my case on my media laptop :) I just want to quickly update uosc from time to time and get to watching. That's the main reason I'm adding this updater - I want to make my life easier.

@po5
Copy link
Contributor

po5 commented Oct 16, 2023

To run the update script, this works: {'bash', '-c', 'source <(curl -fsSL https://raw.githubusercontent.com/tomasklaen/uosc/HEAD/installers/unix.sh)'}

Get the stdout as it is coming in asynchronously

Don't think you can do that, but you can redirect output to a file and read it through lua.

Get path to directory with current mpv binary

What are you trying to do with this? I expect this to only touch the mpv config dir, whose path you already have access to.

@tomasklaen
Copy link
Owner Author

To run the update script, this works: {'bash', '-c', 'source <(curl -fsSL https://raw.githubusercontent.com/tomasklaen/uosc/HEAD/installers/unix.sh)'}

Thank you! I was just scratching my head on this :)

What are you trying to do with this? I expect this to only touch the mpv config dir, whose path you already have access to.

I need to run the subprocess in the same directory as mpv.exe, so that the windows.ps1 install script can detect if there's portable_config folder next to the current mpv.exe, and use that as the config root.

Or maybe I should instead figure out how to set some MPV_CONFIG_DIR environment variable for the subprocess, and use that in the installers when present.

Pointers for each would be appreciated :)

@po5
Copy link
Contributor

po5 commented Oct 16, 2023

I need to run the subprocess in the same directory as mpv.exe [...] detect if there's portable_config folder

So you don't need the mpv.exe path, only the correct config path? If the user previously installed uosc in that dir, then it will be running from that dir, and that's what will be returned by mp.get_script_directory().
You can then go up a few levels if the process really needs to run from the root dir.

@tomasklaen
Copy link
Owner Author

Yeah getting the scripts directory has never been the problem :) The problem is either 1.) getting mpv binary directory, or 2.) how to define an env variable for the mp.command_native_async() call.

Both of these would allow me to solve the problem. The env variable solution would be better, as then the updater wouldn't have to guess the mpv config directory on linux systems (I'm appending the unix installer with detecting Flatpak installs atm, but with the env variable at least updating wouldn't have to guess).

I don't have much experience writing and running bash/powershell scripts if you can't tell :)

@tomasklaen
Copy link
Owner Author

tomasklaen commented Oct 17, 2023

So I've solved most issues. The updater now always updates exactly the uosc instance that started it. The config path is just added to existing environment variables as MPV_CONFIG_DIR, and installers pick it up.

The only issue is this command args table:

{'/bin/bash', '-c', 'source <(curl -fsSL https://raw.githubusercontent.com/tomasklaen/uosc/HEAD/installers/unix.sh)'

Works fine on PopOS, but on Ubuntu I get "curl: command not found" error, even though curl is installed (using the linux install command from readme in terminal works fine), and I'm out of ideas how to fix it :(

If it wasn't enough, this error is not recognized as an error by mp.command_native_async(), and I get success=true and result.status=0, so the UI reports it as "successfully installed"...

@christoph-heinrich
Copy link
Contributor

christoph-heinrich commented Oct 17, 2023

What if you download the script to /tmp/ and then run it with bash in a separate mp. command_native_async() call, that way an unsuccessful curl execution would also not result in success=true and result.status=0. That would also allow passing arguments to the script, instead of having to set environment variables.

I don't know what to do about the curl call failure. You can try /bin/env curl like in the shebang, but no idea if it'll help.

@tomasklaen
Copy link
Owner Author

I did which curl which told me it's at /usr/bin/curl, so I hardcoded it into the command:

local url = 'https://raw.githubusercontent.com/tomasklaen/uosc/HEAD/installers/unix.sh'
local args = {'/bin/bash', '-c', 'source <(/usr/bin/curl -fsSL ' .. url .. ')'}

And the result:

image

If I try first downloading the script with:

local args = {'curl', '-Ls', '-o', '/tmp/uosc-unix-installer.sh', url}

I get:

[uosc] success: true 
[uosc] error: nil 
[uosc] result: {"status" = -3, "stdout" = "", "killed_by_us" = false, "stderr" = "", "error_string" = "init"} 

No stdout or err, just an empty fail.

If I try {'which', 'curl'} first to get path to curl, I get:

[uosc] success: true 
[uosc] error: nil 
[uosc] result: {"error_string" = "", "stdout" = "", "stderr" = "", "status" = 1, "killed_by_us" = false} 

Another empty error.

{'/bin/env', 'curl', '--help'}:

[uosc] success: true 
[uosc] error: nil 
[uosc] result: {"status" = -3, "killed_by_us" = false, "stdout" = "", "error_string" = "init", "stderr" = "/bin/env: ‘curl’: No such file or directory
[uosc] "} 

{'/bin/env curl', '--help'}:

[uosc] success: true 
[uosc] error: nil 
[uosc] result: {"stderr" = "", "stdout" = "", "status" = -3, "error_string" = "init", "killed_by_us" = false} 

All commands above executed with:

mp.command_native_async({
	name = 'subprocess',
	capture_stderr = true,
	capture_stdout = true,
	playback_only = false,
	args = args,
}, function(success, result, error)
	print('success:', utils.to_string(success))
	print('error:', utils.to_string(error))
	print('result:', utils.to_string(result))
end)

I don't know what else to try.

This is Ubuntu 23 btw, and only when running anything through mp.command_native_async(). Using these commands in terminal works fine. In PopOS this PR works fine too.

It's a fresh install of ubuntu-23.10.1-desktop-amd64.iso and then running sudo apt install curl as it wasn't preinstalled. I think this is the most popular Linux distro? So kinda important to make it work there.

Oh wait, the mpv is installed as a snap package. I think that might have something to do with it.

@tomasklaen
Copy link
Owner Author

All right so:

Env Works
Windows ✔️
Linux (apt) ✔️
Linux (flatpak) ✔️
Linux (snap) We're not allowed to access commands like curl even if they're installed.
MacOS image

I think I'm just going to add a note that update command doesn't work everywhere, ask for help if someone knows about a solution, and be done with it.

@christoph-heinrich
Copy link
Contributor

Splitting up There has been an error. See above for clues. might enable the first part to be reused for some other error in the future.

Also isn't An error has occured more common in error messages? It's fine either way.

@tomasklaen tomasklaen merged commit 8763d3f into main Oct 19, 2023
@tomasklaen tomasklaen deleted the updater branch October 27, 2023 07:47
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.

5 participants