diff --git a/docs/src/index.md b/docs/src/index.md index 9327acee..85c22213 100644 --- a/docs/src/index.md +++ b/docs/src/index.md @@ -1,4 +1,4 @@ -# Aqua.jl: +# Aqua.jl: ## *A*uto *QU*ality *A*ssurance for Julia packages Aqua.jl provides functions to run a few automatable checks for Julia packages: @@ -13,6 +13,7 @@ Aqua.jl provides functions to run a few automatable checks for Julia packages: `compat` entry. * `Project.toml` formatting is compatible with Pkg.jl output. * There are no "obvious" type piracies. +* The package does not create any persistent Tasks that might block precompilation of dependencies. ## Quick usage @@ -55,14 +56,14 @@ recommended to add a version bound for Aqua.jl. test = ["Aqua", "Test"] ``` -If your package supports Julia pre-1.2, you need to use the second approach, +If your package supports Julia pre-1.2, you need to use the second approach, although you can use both approaches at the same time. !!! warning In normal use, `Aqua.jl` should not be added to `[deps]` in `YourPackage/Project.toml`! ### ...to your tests? -It is recommended to create a separate file `YourPackage/test/Aqua.jl` that gets included in `YourPackage/test/runtests.jl` +It is recommended to create a separate file `YourPackage/test/Aqua.jl` that gets included in `YourPackage/test/runtests.jl` with either ```julia diff --git a/src/Aqua.jl b/src/Aqua.jl index 2f203016..98ba7b91 100644 --- a/src/Aqua.jl +++ b/src/Aqua.jl @@ -1,7 +1,7 @@ module Aqua using Base: PkgId, UUID -using Pkg: Pkg, TOML +using Pkg: Pkg, TOML, PackageSpec using Test try @@ -22,6 +22,7 @@ include("stale_deps.jl") include("deps_compat.jl") include("project_toml_formatting.jl") include("piracy.jl") +include("persistent_tasks.jl") """ test_all(testtarget::Module) @@ -36,6 +37,7 @@ Run following tests in isolated testset: * [`test_deps_compat(testtarget)`](@ref test_deps_compat) * [`test_project_toml_formatting(testtarget)`](@ref test_project_toml_formatting) * [`test_piracy(testtarget)`](@ref test_piracy) +* [`test_persistent_tasks(testtarget)`](@ref test_persistent_tasks) The keyword argument `\$x` (e.g., `ambiguities`) can be used to control whether or not to run `test_\$x` (e.g., `test_ambiguities`). @@ -51,6 +53,7 @@ passed to `\$x` to specify the keyword arguments for `test_\$x`. - `deps_compat = true` - `project_toml_formatting = true` - `piracy = true` +- `persistent_tasks = true` """ function test_all( testtarget::Module; @@ -62,6 +65,7 @@ function test_all( deps_compat = true, project_toml_formatting = true, piracy = true, + persistent_tasks = true, ) @testset "Method ambiguity" begin if ambiguities !== false @@ -103,6 +107,11 @@ function test_all( test_piracy(testtarget; askwargs(piracy)...) end end + @testset "Persistent tasks" begin + if persistent_tasks !== false + test_persistent_tasks(testtarget; askwargs(persistent_tasks)...) + end + end end end # module diff --git a/src/persistent_tasks.jl b/src/persistent_tasks.jl new file mode 100644 index 00000000..3e21caa4 --- /dev/null +++ b/src/persistent_tasks.jl @@ -0,0 +1,145 @@ +""" + Aqua.test_persistent_tasks(package; tmax=5) + +Test whether loading `package` creates persistent `Task`s +which may block precompilation of dependent packages. + +# Motivation + +Julia 1.10 and higher wait for all running `Task`s to finish +before writing out the precompiled (cached) version of the package. +One consequence is that a package that launches +`Task`s in its `__init__` function may precompile successfully, +but block precompilation of any packages that depend on it. + +# Example + +Let's create a dummy package, `PkgA`, that launches a persistent `Task`: + +```julia + +```julia +module PkgA +const t = Ref{Any}() # to prevent the Timer from being garbage-collected +__init__() = t[] = Timer(0.1; interval=1) # create a persistent `Timer` `Task` +end +``` + +`PkgA` will precompile successfully, because `PkgA.__init__()` does not +run when `PkgA` is precompiled. However, + +```julia +module PkgB +using PkgA +end +``` + +fails to precompile: `using PkgA` runs `PkgA.__init__()`, which +leaves the `Timer` `Task` running, and that causes precompilation +of `PkgB` to hang. + +# How the test works + +This test works by launching a Julia process that tries to precompile a +dummy package similar to `PkgB` above, modified to signal back to Aqua when +`PkgA` has finished loading. The test fails if the gap between loading `PkgA` +and finishing precompilation exceeds time `tmax`. + +# How to fix failing packages + +Often, the easiest fix is to modify the `__init__` function to check whether the +Julia process is precompiling some other package; if so, don't launch the +persistent `Task`s. + +``` +function __init__() + # Other setup code here + if ccall(:jl_generating_output, Cint, ()) == 0 # if we're not precompiling... + # launch persistent tasks here + end +end +``` + +In more complex cases, you may need to set up independently-callable functions +to launch the tasks and cleanly shut them down. + +# Arguments +- `package`: a top-level `Module` or `Base.PkgId`. + +# Keyword Arguments +- `tmax::Real`: the maximum time (in seconds) to wait between loading the + package and forced shutdown of the precompilation process. +""" +function test_persistent_tasks(package::PkgId; tmax=5, fails::Bool=false) + @testset "$package persistent_tasks" begin + result = root_project_or_failed_lazytest(package) + result isa LazyTestResult && return result + @test fails ⊻ precompile_wrapper(result, tmax) + end +end + +function test_persistent_tasks(package::Module; kwargs...) + test_persistent_tasks(PkgId(package); kwargs...) +end + +""" + Aqua.test_persistent_tasks_deps(package; kwargs...) + +Test all the dependencies of `package` with [`Aqua.test_persistent_tasks`](@ref). +""" +function test_persistent_tasks_deps(package::PkgId; fails=Dict{String,Bool}(), kwargs...) + result = root_project_or_failed_lazytest(package) + result isa LazyTestResult && return result + prj = TOML.parsefile(result) + deps = get(prj, "deps", nothing) + @testset "$result" begin + if deps === nothing + return LazyTestResult("$package", "`$result` does not have `deps`", true) + else + for (name, uuid) in deps + id = PkgId(UUID(uuid), name) + test_persistent_tasks(id; fails=get(fails, name, false), kwargs...) + end + end + end +end + +function test_persistent_tasks_deps(package::Module; kwargs...) + test_persistent_tasks_deps(PkgId(package); kwargs...) +end + +function precompile_wrapper(project, tmax) + pkgdir = dirname(project) + pkgname = basename(pkgdir) + wrapperdir = tempname() + wrappername, wrapperuuid = only(Pkg.generate(wrapperdir)) + Pkg.activate(wrapperdir) + Pkg.develop(PackageSpec(path=pkgdir)) + open(joinpath(wrapperdir, "src", wrappername * ".jl"), "w") do io + println(io, """ + module $wrappername + using $pkgname + # Signal Aqua from the precompilation process that we've finished loading the package + open(joinpath("$wrapperdir", "done.log"), "w") do io + println(io, "done") + end + end + """) + end + # Precompile the wrapper package + cmd = `$(Base.julia_cmd()) --project=$wrapperdir -e 'using Pkg; Pkg.precompile()'` + proc = run(cmd; wait=false) + while !isfile(joinpath(wrapperdir, "done.log")) + sleep(0.1) + end + # Check whether precompilation finishes in the required time + t = time() + while process_running(proc) && time() - t < tmax + sleep(0.1) + end + success = !process_running(proc) + if !success + kill(proc) + end + return success +end diff --git a/test/pkgs/PersistentTasks/PersistentTask/Project.toml b/test/pkgs/PersistentTasks/PersistentTask/Project.toml new file mode 100644 index 00000000..9f5cf185 --- /dev/null +++ b/test/pkgs/PersistentTasks/PersistentTask/Project.toml @@ -0,0 +1,4 @@ +name = "PersistentTask" +uuid = "e5c298b6-d81d-47aa-a9ed-5ea983e22986" +authors = ["Tim Holy "] +version = "0.1.0" diff --git a/test/pkgs/PersistentTasks/PersistentTask/src/PersistentTask.jl b/test/pkgs/PersistentTasks/PersistentTask/src/PersistentTask.jl new file mode 100644 index 00000000..5778f94a --- /dev/null +++ b/test/pkgs/PersistentTasks/PersistentTask/src/PersistentTask.jl @@ -0,0 +1,6 @@ +module PersistentTask + +const t = Ref{Any}() +__init__() = t[] = Timer(0.1; interval=1) # create a persistent `Timer` `Task` + +end # module PersistentTask diff --git a/test/pkgs/PersistentTasks/TransientTask/Project.toml b/test/pkgs/PersistentTasks/TransientTask/Project.toml new file mode 100644 index 00000000..e69799be --- /dev/null +++ b/test/pkgs/PersistentTasks/TransientTask/Project.toml @@ -0,0 +1,4 @@ +name = "TransientTask" +uuid = "94ae9332-58b0-4342-989c-0a7e44abcca9" +authors = ["Tim Holy "] +version = "0.1.0" diff --git a/test/pkgs/PersistentTasks/TransientTask/src/TransientTask.jl b/test/pkgs/PersistentTasks/TransientTask/src/TransientTask.jl new file mode 100644 index 00000000..7a8d09ec --- /dev/null +++ b/test/pkgs/PersistentTasks/TransientTask/src/TransientTask.jl @@ -0,0 +1,5 @@ +module TransientTask + +__init__() = Timer(0.1) # create a transient `Timer` `Task` + +end # module TransientTask diff --git a/test/pkgs/PersistentTasks/UsesBoth/Project.toml b/test/pkgs/PersistentTasks/UsesBoth/Project.toml new file mode 100644 index 00000000..98a85e9f --- /dev/null +++ b/test/pkgs/PersistentTasks/UsesBoth/Project.toml @@ -0,0 +1,8 @@ +name = "UsesBoth" +uuid = "96f12b6e-60f8-43dc-b131-049a88a2f499" +authors = ["Tim Holy "] +version = "0.1.0" + +[deps] +PersistentTask = "e5c298b6-d81d-47aa-a9ed-5ea983e22986" +TransientTask = "94ae9332-58b0-4342-989c-0a7e44abcca9" diff --git a/test/pkgs/PersistentTasks/UsesBoth/src/UsesBoth.jl b/test/pkgs/PersistentTasks/UsesBoth/src/UsesBoth.jl new file mode 100644 index 00000000..7f45db8d --- /dev/null +++ b/test/pkgs/PersistentTasks/UsesBoth/src/UsesBoth.jl @@ -0,0 +1,6 @@ +module UsesBoth + +using TransientTask +using PersistentTask + +end # module UsesBoth diff --git a/test/test_persistent_tasks.jl b/test/test_persistent_tasks.jl new file mode 100644 index 00000000..7082ded9 --- /dev/null +++ b/test/test_persistent_tasks.jl @@ -0,0 +1,28 @@ +module TestPersistentTasks + +include("preamble.jl") +using Base: PkgId, UUID +using Pkg: TOML + +function getid(name) + path = joinpath(@__DIR__, "pkgs", "PersistentTasks", name) + if path ∉ LOAD_PATH + pushfirst!(LOAD_PATH, path) + end + prj = TOML.parsefile(joinpath(path, "Project.toml")) + return PkgId(UUID(prj["uuid"]), prj["name"]) +end + + +@testset "PersistentTasks" begin + Aqua.test_persistent_tasks(getid("TransientTask")) + Aqua.test_persistent_tasks_deps(getid("TransientTask")) + + if all((Base.VERSION.major, Base.VERSION.minor) .>= (1, 10)) + Aqua.test_persistent_tasks(getid("PersistentTask"); fails=true) + Aqua.test_persistent_tasks_deps(getid("UsesBoth"); fails=Dict("PersistentTask" => true)) + end + filter!(str -> !occursin("PersistentTasks", str), LOAD_PATH) +end + +end