From 83b3dfe7ba14b9ebb2518564decc9eec34b76f3b Mon Sep 17 00:00:00 2001 From: Alberto Mengali Date: Wed, 28 Feb 2024 12:26:16 +0100 Subject: [PATCH 01/10] point to kaleido.exe directly on windows --- src/PlotlyKaleido.jl | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/PlotlyKaleido.jl b/src/PlotlyKaleido.jl index 1d2410f..c4d75be 100644 --- a/src/PlotlyKaleido.jl +++ b/src/PlotlyKaleido.jl @@ -33,7 +33,8 @@ function start(; kwargs..., ) is_running() && return - cmd = joinpath(Kaleido_jll.artifact_dir, "kaleido" * (Sys.iswindows() ? ".cmd" : "")) + bin_name = Sys.iswindows() ? joinpath("bin", "kaleido.exe") : "kaleido" + cmd = joinpath(Kaleido_jll.artifact_dir, bin_name) basic_cmds = [cmd, "plotly"] chromium_flags = ["--disable-gpu", Sys.isapple() ? "--single-process" : "--no-sandbox"] extra_flags = if plotly_version === missing From 4080fc07e4cc4de6bf953a13b24cfea7d21a1add Mon Sep 17 00:00:00 2001 From: Alberto Mengali Date: Thu, 29 Feb 2024 19:17:26 +0100 Subject: [PATCH 02/10] try alternative --- Project.toml | 7 ++++--- src/PlotlyKaleido.jl | 33 ++++++++++++++++++++++++++++----- test/runtests.jl | 5 +++++ 3 files changed, 37 insertions(+), 8 deletions(-) diff --git a/Project.toml b/Project.toml index bf4528d..7dec445 100644 --- a/Project.toml +++ b/Project.toml @@ -14,10 +14,11 @@ julia = "1.6" Kaleido_jll = "0.1, 0.2" [extras] -Test = "8dfed614-e22c-5e08-85e1-65c5234f0b40" -PlotlyLight = "ca7969ec-10b3-423e-8d99-40f33abb42bf" EasyConfig = "acab07b0-f158-46d4-8913-50acef6d41fe" PlotlyJS = "f0f68f2c-4968-5e81-91da-67840de0976a" +PlotlyLight = "ca7969ec-10b3-423e-8d99-40f33abb42bf" +Test = "8dfed614-e22c-5e08-85e1-65c5234f0b40" +Pkg = "44cfe95a-1eb2-52ea-b672-e2afdf69b78f" [targets] -test = ["Test", "PlotlyLight", "EasyConfig", "PlotlyJS"] +test = ["Test", "PlotlyLight", "EasyConfig", "PlotlyJS", "Pkg"] diff --git a/src/PlotlyKaleido.jl b/src/PlotlyKaleido.jl index c4d75be..37552c1 100644 --- a/src/PlotlyKaleido.jl +++ b/src/PlotlyKaleido.jl @@ -26,6 +26,25 @@ is_running() = isdefined(P, :proc) && isopen(P.stdin) && process_running(P.proc) restart(; kwargs...) = (kill_kaleido(); start(; kwargs...)) +#= +This function checks if the kaleido.cmd has only read permission, and if so, +creates a temporary kaleido.cmd that directly calls the binary to bypass +permission errors +=# +function maybe_copy_deps(dirpath = pwd()) + Sys.iswindows() || return + basedir = Kaleido_jll.artifact_dir + js_path = joinpath("js", "kaleido_scopes.js") + local_js = joinpath(dirpath, js_path) + mkpath(local_js |> dirname) # Ensure the js directory exists + artifact_js = joinpath(basedir, js_path) + # Copy js + cp(artifact_js, local_js; force = true) + # Copy the version + cp(joinpath(basedir, "version"), joinpath(dirpath, "version"); force = true) + return +end + function start(; plotly_version = missing, mathjax = missing, @@ -33,9 +52,10 @@ function start(; kwargs..., ) is_running() && return - bin_name = Sys.iswindows() ? joinpath("bin", "kaleido.exe") : "kaleido" - cmd = joinpath(Kaleido_jll.artifact_dir, bin_name) - basic_cmds = [cmd, "plotly"] + # The kaleido executable must be ran from the artifact directory + BIN = Cmd(kaleido(); dir = Kaleido_jll.artifact_dir) + # We push the mandatory plotly flag + push!(BIN.exec, "plotly") chromium_flags = ["--disable-gpu", Sys.isapple() ? "--single-process" : "--no-sandbox"] extra_flags = if plotly_version === missing (; kwargs...) @@ -73,13 +93,16 @@ function start(; push!(user_flags, "--$flag_name=$v") end end - BIN = Cmd(vcat(basic_cmds, chromium_flags, user_flags)) + # We add the flags to the BIN + append!(BIN.exec, chromium_flags, extra_flags) kstdin = Pipe() kstdout = Pipe() kstderr = Pipe() - kproc = + kproc = cd(Sys.iswindows() ? mktempdir() : Kaleido_jll.artifact_dir) do + maybe_copy_deps(pwd()) # This will do nothing outside of windows run(pipeline(BIN, stdin = kstdin, stdout = kstdout, stderr = kstderr), wait = false) + end process_running(kproc) || error("There was a problem starting up kaleido.") close(kstdout.in) diff --git a/test/runtests.jl b/test/runtests.jl index 11f26f1..94962cf 100644 --- a/test/runtests.jl +++ b/test/runtests.jl @@ -1,4 +1,9 @@ using Test +import Pkg +if Sys.iswindows() + # Fix kaleido tests on windows due to Kaleido_jll@v0.2.1 hanging + Pkg.add(;name = "Kaleido_jll", version = "0.1") +end @test_nowarn @eval using PlotlyKaleido @testset "Start" begin From 0058f31c229b3bf07626debacb223ad66b9609ca Mon Sep 17 00:00:00 2001 From: Alberto Mengali Date: Thu, 29 Feb 2024 19:20:15 +0100 Subject: [PATCH 03/10] remove hanging stuff --- src/PlotlyKaleido.jl | 23 +---------------------- 1 file changed, 1 insertion(+), 22 deletions(-) diff --git a/src/PlotlyKaleido.jl b/src/PlotlyKaleido.jl index 37552c1..e8c3495 100644 --- a/src/PlotlyKaleido.jl +++ b/src/PlotlyKaleido.jl @@ -26,25 +26,6 @@ is_running() = isdefined(P, :proc) && isopen(P.stdin) && process_running(P.proc) restart(; kwargs...) = (kill_kaleido(); start(; kwargs...)) -#= -This function checks if the kaleido.cmd has only read permission, and if so, -creates a temporary kaleido.cmd that directly calls the binary to bypass -permission errors -=# -function maybe_copy_deps(dirpath = pwd()) - Sys.iswindows() || return - basedir = Kaleido_jll.artifact_dir - js_path = joinpath("js", "kaleido_scopes.js") - local_js = joinpath(dirpath, js_path) - mkpath(local_js |> dirname) # Ensure the js directory exists - artifact_js = joinpath(basedir, js_path) - # Copy js - cp(artifact_js, local_js; force = true) - # Copy the version - cp(joinpath(basedir, "version"), joinpath(dirpath, "version"); force = true) - return -end - function start(; plotly_version = missing, mathjax = missing, @@ -99,10 +80,8 @@ function start(; kstdin = Pipe() kstdout = Pipe() kstderr = Pipe() - kproc = cd(Sys.iswindows() ? mktempdir() : Kaleido_jll.artifact_dir) do - maybe_copy_deps(pwd()) # This will do nothing outside of windows + kproc = run(pipeline(BIN, stdin = kstdin, stdout = kstdout, stderr = kstderr), wait = false) - end process_running(kproc) || error("There was a problem starting up kaleido.") close(kstdout.in) From 565fcdfd496e835ace064e0a18e84017092fb0cc Mon Sep 17 00:00:00 2001 From: Alberto Mengali Date: Thu, 29 Feb 2024 19:32:04 +0100 Subject: [PATCH 04/10] remove `@test_nowarn` on start() --- test/runtests.jl | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/runtests.jl b/test/runtests.jl index 94962cf..2333448 100644 --- a/test/runtests.jl +++ b/test/runtests.jl @@ -6,8 +6,8 @@ if Sys.iswindows() end @test_nowarn @eval using PlotlyKaleido +PlotlyKaleido.start() @testset "Start" begin - @test_nowarn PlotlyKaleido.start() @test PlotlyKaleido.is_running() end From 8d8f7254527b583f759f6d6e1fa0dcdc35d33827 Mon Sep 17 00:00:00 2001 From: Alberto Mengali Date: Fri, 1 Mar 2024 07:46:53 +0100 Subject: [PATCH 05/10] put bck test_nowarn on non-windows --- test/runtests.jl | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/test/runtests.jl b/test/runtests.jl index 2333448..9cce873 100644 --- a/test/runtests.jl +++ b/test/runtests.jl @@ -6,8 +6,12 @@ if Sys.iswindows() end @test_nowarn @eval using PlotlyKaleido -PlotlyKaleido.start() -@testset "Start" begin + @testset "Start" begin + if Sys.iswindows() + PlotlyKaleido.start() + else + @test_nowarn PlotlyKaleido.start() + end @test PlotlyKaleido.is_running() end From c462fc1bc99e13afb8e95415efa3ed5a476a48d9 Mon Sep 17 00:00:00 2001 From: Alberto Mengali Date: Fri, 1 Mar 2024 07:47:12 +0100 Subject: [PATCH 06/10] warn about windows problem on README --- README.md | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/README.md b/README.md index 1ca5252..4a9d1a9 100644 --- a/README.md +++ b/README.md @@ -52,4 +52,9 @@ PlotlyKaleido.kill_kaleido() To enable LaTeX (using MathJax v2) in plots, use the keyword argument `mathjax`: ```julia PlotlyKaleido.start(mathjax=true) # start Kaleido server with MathJax enabled -``` \ No newline at end of file +``` + +## Windows Note +Many people one Windows have issues with the latest (0.2.1) version of the Kaleido library (see for example [discourse](https://discourse.julialang.org/t/plotlyjs-causes-errors-cant-figure-out-how-to-use-plotlylight-how-to-use-plotly-from-julia/108853/29), [this PR's comment](https://github.com/JuliaPlots/PlotlyKaleido.jl/pull/17#issuecomment-1969325440) and [this issue](https://github.com/plotly/Kaleido/issues/134) on the Kaleido repository). + +Many people have succesfully fixed this problem on windows by downgrading the kaleido library to version 0.1.0 (see [the previously mentioned issue](https://github.com/plotly/Kaleido/issues/134)). If you experience issues with `PlotlyKaleido.start()` hanging on windows, you may want try adding `Kaledido_jll@v0.1` explicitly to your project environment to fix this. \ No newline at end of file From f678ac4b2f96f73e89d2ab46339007dd05e09ead Mon Sep 17 00:00:00 2001 From: Alberto Mengali Date: Sat, 2 Mar 2024 09:06:08 +0100 Subject: [PATCH 07/10] testnowarn error --- test/runtests.jl | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/test/runtests.jl b/test/runtests.jl index 9cce873..fd01649 100644 --- a/test/runtests.jl +++ b/test/runtests.jl @@ -7,11 +7,11 @@ end @test_nowarn @eval using PlotlyKaleido @testset "Start" begin - if Sys.iswindows() - PlotlyKaleido.start() - else + # if Sys.iswindows() + # PlotlyKaleido.start() + # else @test_nowarn PlotlyKaleido.start() - end + # end @test PlotlyKaleido.is_running() end From ebe9ff210f8dcc720ead9a258a0cc751d08b974a Mon Sep 17 00:00:00 2001 From: Alberto Mengali Date: Sat, 2 Mar 2024 09:27:44 +0100 Subject: [PATCH 08/10] revert test_nowarn error --- test/runtests.jl | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/test/runtests.jl b/test/runtests.jl index fd01649..9cce873 100644 --- a/test/runtests.jl +++ b/test/runtests.jl @@ -7,11 +7,11 @@ end @test_nowarn @eval using PlotlyKaleido @testset "Start" begin - # if Sys.iswindows() - # PlotlyKaleido.start() - # else + if Sys.iswindows() + PlotlyKaleido.start() + else @test_nowarn PlotlyKaleido.start() - # end + end @test PlotlyKaleido.is_running() end From be759e41c71be804922137a18d68ab121450ea87 Mon Sep 17 00:00:00 2001 From: Alberto Mengali Date: Sat, 2 Mar 2024 10:51:24 +0100 Subject: [PATCH 09/10] Throw an error with a suggestion fix if kaleido seems to hang --- README.md | 12 +++++++++++- src/PlotlyKaleido.jl | 15 ++++++++++++++- 2 files changed, 25 insertions(+), 2 deletions(-) diff --git a/README.md b/README.md index 4a9d1a9..823c65a 100644 --- a/README.md +++ b/README.md @@ -57,4 +57,14 @@ PlotlyKaleido.start(mathjax=true) # start Kaleido server with MathJax enabled ## Windows Note Many people one Windows have issues with the latest (0.2.1) version of the Kaleido library (see for example [discourse](https://discourse.julialang.org/t/plotlyjs-causes-errors-cant-figure-out-how-to-use-plotlylight-how-to-use-plotly-from-julia/108853/29), [this PR's comment](https://github.com/JuliaPlots/PlotlyKaleido.jl/pull/17#issuecomment-1969325440) and [this issue](https://github.com/plotly/Kaleido/issues/134) on the Kaleido repository). -Many people have succesfully fixed this problem on windows by downgrading the kaleido library to version 0.1.0 (see [the previously mentioned issue](https://github.com/plotly/Kaleido/issues/134)). If you experience issues with `PlotlyKaleido.start()` hanging on windows, you may want try adding `Kaledido_jll@v0.1` explicitly to your project environment to fix this. \ No newline at end of file +Many people have succesfully fixed this problem on windows by downgrading the kaleido library to version 0.1.0 (see [the previously mentioned issue](https://github.com/plotly/Kaleido/issues/134)). If you experience issues with `PlotlyKaleido.start()` hanging on windows, you may want try adding `Kaledido_jll@v0.1` explicitly to your project environment to fix this. You can do so by either doing: +```julia +add Kaleido_jll@v0.1 +``` +inside the REPL package enviornment, or by calling the following code in the REPL directly: +```julia +begin + import Pkg + Pkg.add(; name = "Kaleido_jll", version = "0.1") +end +``` \ No newline at end of file diff --git a/src/PlotlyKaleido.jl b/src/PlotlyKaleido.jl index e8c3495..f2398cf 100644 --- a/src/PlotlyKaleido.jl +++ b/src/PlotlyKaleido.jl @@ -26,6 +26,19 @@ is_running() = isdefined(P, :proc) && isopen(P.stdin) && process_running(P.proc) restart(; kwargs...) = (kill_kaleido(); start(; kwargs...)) +function readline_noblock(io) + for i in 1:30 + if bytesavailable(io) > 0 + return readline(io) + end + sleep(0.1) + end + error("It looks like the kaleido process is hanging. +If you are on windows this might be caused by known problems with Kaleido v0.2 on windows. +You might want to try forcing a downgrade of the kaleido library to 0.1. +Check the Package Readme at https://github.com/JuliaPlots/PlotlyKaleido.jl/tree/main#windows-note for more details") +end + function start(; plotly_version = missing, mathjax = missing, @@ -95,7 +108,7 @@ function start(; P.stderr = kstderr P.proc = kproc - res = readline(P.stdout) # {"code": 0, "message": "Success", "result": null, "version": "0.2.1"} + res = readline_noblock(P.stdout) # {"code": 0, "message": "Success", "result": null, "version": "0.2.1"} length(res) == 0 && error("Kaleido startup failed.") code = JSON.parse(res)["code"] code == 0 || error("Kaleido startup failed with code $code.") From 6fab6e3839251863de80d3383fe2bea4dcc9b746 Mon Sep 17 00:00:00 2001 From: Alberto Mengali Date: Sat, 2 Mar 2024 11:15:00 +0100 Subject: [PATCH 10/10] fix noblock function as bytesavailable does not work here --- src/PlotlyKaleido.jl | 27 ++++++++++++++++++++++----- 1 file changed, 22 insertions(+), 5 deletions(-) diff --git a/src/PlotlyKaleido.jl b/src/PlotlyKaleido.jl index f2398cf..f2631c7 100644 --- a/src/PlotlyKaleido.jl +++ b/src/PlotlyKaleido.jl @@ -26,17 +26,34 @@ is_running() = isdefined(P, :proc) && isopen(P.stdin) && process_running(P.proc) restart(; kwargs...) = (kill_kaleido(); start(; kwargs...)) +# The content of this function is inspired from https://discourse.julialang.org/t/readline-with-default-value-if-no-input-after-timeout/100388/2?u=disberd function readline_noblock(io) - for i in 1:30 - if bytesavailable(io) > 0 - return readline(io) + msg = Channel{String}(1) + + task = Task() do + try + put!(msg, readline(io)) + catch + put!(msg, "Stopped") end - sleep(0.1) end - error("It looks like the kaleido process is hanging. + + interrupter = Task() do + sleep(5) + if !istaskdone(task) + Base.throwto(task, InterruptException()) + end + end + + schedule(interrupter) + schedule(task) + wait(task) + out = take!(msg) + out === "Stopped" && error("It looks like the kaleido process is hanging. If you are on windows this might be caused by known problems with Kaleido v0.2 on windows. You might want to try forcing a downgrade of the kaleido library to 0.1. Check the Package Readme at https://github.com/JuliaPlots/PlotlyKaleido.jl/tree/main#windows-note for more details") + return out end function start(;