PhoenicisOrg / scripts

Phoenicis scripts
GNU Lesser General Public License v3.0
64 stars 49 forks source link

Change GitHub releases utility #1148

Closed plata closed 4 years ago

plata commented 4 years ago

see https://github.com/PhoenicisOrg/scripts/blob/master/Utils/Functions/Net/GithubReleases/script.js

The approach is wrong. Some scripts assumes that the download URL is formed in a certain way based on the release version. It would be better to return the complete json and then use tarball_url directly.

My suggestion would be to replace

const versions = JSON.parse(cat(releasesFile)).map(version => version.tag_name);

by

const versions = JSON.parse(cat(releasesFile));
Zemogiter commented 4 years ago

I've tried it on local repo and I get this error:

[ERROR] org.phoenicis.multithreading.ControlledThreadPoolExecutorService (l.64) - Download of https://github.com/doitsujin/dxvk/releases/download/[object Object]/dxvk-[object Object].tar.gz has failed
    at org.phoenicis.tools.http.Downloader.saveConnectionToStream(Downloader.java:307)
    at org.phoenicis.tools.http.Downloader.get(Downloader.java:260)
    at org.phoenicis.tools.http.Downloader.get(Downloader.java:237)
    at org.phoenicis.tools.http.Downloader.get(Downloader.java:101)
    at org.phoenicis.tools.http.Downloader.get(Downloader.java:81)
    at <js> get(Unnamed:135-144:3538-3839)
    at <js> get(Unnamed:110-117:2786-3026)
    at <js> go(Unnamed:70-76:2648-2915)
    at <js> :anonymous(Unnamed:20:661-679)
    at <js> go(Unnamed:83:3245-3279)
    at com.oracle.truffle.polyglot.ObjectProxyHandler.invoke(HostInteropReflect.java:678)
    at com.sun.proxy.$Proxy68.go(Unknown Source)
    at org.phoenicis.javafx.components.application.skin.ApplicationInformationPanelSkin.lambda$installScript$7(ApplicationInformationPanelSkin.java:237)
    at org.phoenicis.scripts.session.PhoenicisInteractiveScriptSession.eval(PhoenicisInteractiveScriptSession.java:35)
    at org.phoenicis.scripts.interpreter.BackgroundScriptInterpreter.lambda$createInteractiveSession$1(BackgroundScriptInterpreter.java:45)
    at java.base/java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1128)
    at java.base/java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:628)
    at java.base/java.lang.Thread.run(Thread.java:834)
Caused by host exception: org.phoenicis.tools.http.DownloadException: Download of https://github.com/doitsujin/dxvk/releases/download/[object Object]/dxvk-[object Object].tar.gz has failed

[WARNING] 
org.graalvm.polyglot.PolyglotException: Download of https://github.com/doitsujin/dxvk/releases/download/[object Object]/dxvk-[object Object].tar.gz has failed
    at org.phoenicis.tools.http.Downloader.saveConnectionToStream (Downloader.java:307)
    at org.phoenicis.tools.http.Downloader.get (Downloader.java:260)
    at org.phoenicis.tools.http.Downloader.get (Downloader.java:237)
    at org.phoenicis.tools.http.Downloader.get (Downloader.java:101)
    at org.phoenicis.tools.http.Downloader.get (Downloader.java:81)
    at <js>.get (Unnamed:135)
    at <js>.get (Unnamed:110)
    at <js>.go (Unnamed:70)
    at <js>.:anonymous (Unnamed:20)
    at <js>.go (Unnamed:83)
    at com.oracle.truffle.polyglot.ObjectProxyHandler.invoke (HostInteropReflect.java:678)
    at com.sun.proxy.$Proxy68.go (Unknown Source)
    at org.phoenicis.javafx.components.application.skin.ApplicationInformationPanelSkin.lambda$installScript$7 (ApplicationInformationPanelSkin.java:237)
    at org.phoenicis.scripts.session.PhoenicisInteractiveScriptSession.eval (PhoenicisInteractiveScriptSession.java:35)
    at org.phoenicis.scripts.interpreter.BackgroundScriptInterpreter.lambda$createInteractiveSession$1 (BackgroundScriptInterpreter.java:45)
    at java.util.concurrent.ThreadPoolExecutor.runWorker (ThreadPoolExecutor.java:1128)
    at java.util.concurrent.ThreadPoolExecutor$Worker.run (ThreadPoolExecutor.java:628)
    at java.lang.Thread.run (Thread.java:834)

I guess we need the map object after all because the version number is stored there.

ImperatorS79 commented 4 years ago

The approach is wrong. Some scripts assumes that the download URL is formed in a certain way based on the release version

I do not see in which case the current approach would not work. Could you link an exemple ? AFAIK It worked with DXVK, FAudio an Gallium 9 when I did it.

plata commented 4 years ago

E.g. https://github.com/PhoenicisOrg/scripts/pull/1039

ImperatorS79 commented 4 years ago

And where is the problem ? The utility send you the tags of the versions which you use to access the url of the archive (which are build based on the tag).

plata commented 4 years ago

The problem is that the URL is not necessarily using exactly the same name as the version (at least that's how I understood @Zemogiter 's problem).

madoar commented 4 years ago

@plata can you provide some pseudocode how you would solve the issue?

plata commented 4 years ago

Like I wrote in the PR description: in the GitHub releases

const versions = JSON.parse(cat(releasesFile));
return versions;

when it's used (pseudocode)

download(versions[tarball_url])
Zemogiter commented 4 years ago

Oddly enough, after removing local repo directory, downloading a fresh one, downloading my PRs I've tried The Sims 4 and the issue regarding D9VK download is gone.

plata commented 4 years ago

ok...

Zemogiter commented 4 years ago

However, I just tried installing DXVK in Space Engineers prefix via verb and got this error:

[ERROR] org.phoenicis.multithreading.ControlledThreadPoolExecutorService (l.64) - Download of https://github.com/doitsujin/dxvk/releases/download/v1.5/dxvk-v1.5.tar.gz has failed
    at org.phoenicis.tools.http.Downloader.saveConnectionToStream(Downloader.java:307)
    at org.phoenicis.tools.http.Downloader.get(Downloader.java:260)
    at org.phoenicis.tools.http.Downloader.get(Downloader.java:237)
    at org.phoenicis.tools.http.Downloader.get(Downloader.java:101)
    at org.phoenicis.tools.http.Downloader.get(Downloader.java:81)
    at <js> get(Unnamed:135-144:3538-3839)
    at <js> get(Unnamed:110-117:2786-3026)
    at <js> go(Unnamed:70-76:2648-2915)
    at <js> install(Unnamed:119:4293-4345)
    at org.graalvm.polyglot.Value.invokeMember(Value.java:459)
    at org.phoenicis.engines.VerbsManager.lambda$installVerb$0(VerbsManager.java:71)
    at org.phoenicis.scripts.session.PhoenicisInteractiveScriptSession.eval(PhoenicisInteractiveScriptSession.java:35)
    at org.phoenicis.scripts.interpreter.BackgroundScriptInterpreter.lambda$createInteractiveSession$1(BackgroundScriptInterpreter.java:45)
    at java.base/java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1128)
    at java.base/java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:628)
    at java.base/java.lang.Thread.run(Thread.java:834)
Caused by host exception: org.phoenicis.tools.http.DownloadException: Download of https://github.com/doitsujin/dxvk/releases/download/v1.5/dxvk-v1.5.tar.gz has failed

[WARNING] 
org.graalvm.polyglot.PolyglotException: Download of https://github.com/doitsujin/dxvk/releases/download/v1.5/dxvk-v1.5.tar.gz has failed
    at org.phoenicis.tools.http.Downloader.saveConnectionToStream (Downloader.java:307)
    at org.phoenicis.tools.http.Downloader.get (Downloader.java:260)
    at org.phoenicis.tools.http.Downloader.get (Downloader.java:237)
    at org.phoenicis.tools.http.Downloader.get (Downloader.java:101)
    at org.phoenicis.tools.http.Downloader.get (Downloader.java:81)
    at <js>.get (Unnamed:135)
    at <js>.get (Unnamed:110)
    at <js>.go (Unnamed:70)
    at <js>.install (Unnamed:119)
    at org.graalvm.polyglot.Value.invokeMember (Value.java:459)
    at org.phoenicis.engines.VerbsManager.lambda$installVerb$0 (VerbsManager.java:71)
    at org.phoenicis.scripts.session.PhoenicisInteractiveScriptSession.eval (PhoenicisInteractiveScriptSession.java:35)
    at org.phoenicis.scripts.interpreter.BackgroundScriptInterpreter.lambda$createInteractiveSession$1 (BackgroundScriptInterpreter.java:45)
    at java.util.concurrent.ThreadPoolExecutor.runWorker (ThreadPoolExecutor.java:1128)
    at java.util.concurrent.ThreadPoolExecutor$Worker.run (ThreadPoolExecutor.java:628)
    at java.lang.Thread.run (Thread.java:834)

Tried D9VK on The Sims 3 prefix and no error whatsoever.

plata commented 4 years ago

Is the URL correct?

Zemogiter commented 4 years ago

@plata No

plata commented 4 years ago

So my proposal is still valid.

madoar commented 4 years ago

I think this is a blocker for the next release. The change seems quite straight forward. If I find some time over the next days I will try to fix this.

Zemogiter commented 4 years ago

After applying #1152 and updating POL5 to the latest commit the error when using DXVK verb have changed:

[ERROR] org.phoenicis.multithreading.ControlledThreadPoolExecutorService (l.64) - TypeError: invokeMember (menu) on JavaObject[org.phoenicis.scripts.wizard.UiSetupWizardImplementation@707ce5da (org.phoenicis.scripts.wizard.UiSetupWizardImplementation)] failed due to: no applicable overload found (overloads: [Method[public org.phoenicis.scripts.ui.MenuItem org.phoenicis.scripts.wizard.UiSetupWizardImplementation.menu(java.lang.String,java.util.List,java.lang.String)], Method[public org.phoenicis.scripts.ui.MenuItem org.phoenicis.scripts.wizard.UiSetupWizardImplementation.menu(java.lang.String,java.util.List)]], arguments: [Please select the version. (String), DynamicObject<JSArray>@35ce3283 (DynamicObjectBasic), DynamicObject<JSUserObject>@f36587d (DynamicObjectBasic)])
    at <js> install(Unnamed:116:4178-4245)
    at org.graalvm.polyglot.Value.invokeMember(Value.java:459)
    at org.phoenicis.engines.VerbsManager.lambda$installVerb$0(VerbsManager.java:71)
    at org.phoenicis.scripts.session.PhoenicisInteractiveScriptSession.eval(PhoenicisInteractiveScriptSession.java:35)
    at org.phoenicis.scripts.interpreter.BackgroundScriptInterpreter.lambda$createInteractiveSession$1(BackgroundScriptInterpreter.java:45)
    at java.base/java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1128)
    at java.base/java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:628)
    at java.base/java.lang.Thread.run(Thread.java:834)

The same error is displayed when trying D9VK

plata commented 4 years ago

This looks like a different issue.

Zemogiter commented 4 years ago

Forgot to mention I've also changed the github releases utility acording to the OP

madoar commented 4 years ago

Forgot to mention I've also changed the github releases utility acording to the OP

This is most likely the reason for your error message. You're passing the version objects to the menu method which expects string objects as its input. You first need to "fetch" the names by calling versions.map(version => version.tag_name)