diff --git a/CHANGELOG.md b/CHANGELOG.md index 1d579814..7e961383 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,6 +7,12 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## [Unreleased] +### Added + +- Two additions check whether packages might block precompilation on Julia 1.10 or higher: ([#174](https://github.com/JuliaTesting/Aqua.jl/pull/174)) + + `test_persistent_tasks` tests whether "your" package can safely be used as a dependency for downstream packages. This test is enabled for the default testsuite, but you can opt-out by supplying `persistent_tasks=false` to `test_all`. [BREAKING] + + `find_persistent_tasks_deps` is useful if "your" package hangs upon precompilation: it runs `test_persistent_tasks` on all the things you depend on, and may help isolate the culprit(s). + ### Changed - The minimum requirement for julia was raised from `1.0` to `1.4`. ([#221](https://github.com/JuliaTesting/Aqua.jl/pull/221)) 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 2078fdda..8d403081 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 @static if VERSION >= v"1.7-" @@ -21,6 +21,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) @@ -35,6 +36,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`). @@ -50,6 +52,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; @@ -61,6 +64,7 @@ function test_all( deps_compat = true, project_toml_formatting = true, piracy = true, + persistent_tasks = true, ) @testset "Method ambiguity" begin if ambiguities !== false @@ -102,6 +106,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..af963208 --- /dev/null +++ b/src/persistent_tasks.jl @@ -0,0 +1,171 @@ +""" + Aqua.test_persistent_tasks(package) + +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 +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 set conditions that allow them to cleanly exit. + +# Arguments +- `package`: a top-level `Module` or `Base.PkgId`. + +# Keyword Arguments +- `broken::Bool = false`: If true, it uses `@test_broken` instead of + `@test`. +- `tmax::Real = 5`: the maximum time (in seconds) to wait after loading the + package before forcibly shutting down the precompilation process (triggering + a test failure). +""" +function test_persistent_tasks(package::PkgId; broken::Bool = false, kwargs...) + if broken + @test_broken !has_persistent_tasks(package; kwargs...) + else + @test !has_persistent_tasks(package; kwargs...) + end +end + +function test_persistent_tasks(package::Module; kwargs...) + test_persistent_tasks(PkgId(package); kwargs...) +end + +function has_persistent_tasks(package::PkgId; tmax = 10) + result = root_project_or_failed_lazytest(package) + result isa LazyTestResult && error("Unable to locate Project.toml") + return !precompile_wrapper(result, tmax) +end + +""" + Aqua.find_persistent_tasks_deps(package; broken = Dict{String,Bool}(), kwargs...) + +Test all the dependencies of `package` with [`Aqua.test_persistent_tasks`](@ref). +On Julia 1.10 and higher, it returns a list of all dependencies failing the test. +These are likely the ones blocking precompilation of your package. + +Any additional kwargs (e.g., `tmax`) are passed to [`Aqua.test_persistent_tasks`](@ref). +""" +function find_persistent_tasks_deps(package::PkgId; kwargs...) + result = root_project_or_failed_lazytest(package) + result isa LazyTestResult && error("Unable to locate Project.toml") + prj = TOML.parsefile(result) + deps = get(prj, "deps", Dict{String,Any}()) + filter!(deps) do (name, uuid) + id = PkgId(UUID(uuid), name) + return has_persistent_tasks(id; kwargs...) + end + return [name for (name, _) in deps] +end + +function find_persistent_tasks_deps(package::Module; kwargs...) + find_persistent_tasks_deps(PkgId(package); kwargs...) +end + +function precompile_wrapper(project, tmax) + prev_project = Base.active_project() + isdefined(Pkg, :respect_sysimage_versions) && Pkg.respect_sysimage_versions(false) + try + pkgdir = dirname(project) + pkgname = get(TOML.parsefile(project), "name", nothing) + if isnothing(pkgname) + @error "Unable to locate package name in $project" + return false + end + wrapperdir = tempname() + wrappername, _ = only(Pkg.generate(wrapperdir)) + Pkg.activate(wrapperdir) + Pkg.develop(PackageSpec(path = pkgdir)) + statusfile = joinpath(wrapperdir, "done.log") + 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("$(escape_string(statusfile))", "w") do io + println(io, "done") + flush(io) +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(statusfile) && process_running(proc) + sleep(0.5) + end + if !isfile(statusfile) + @error "Unexpected error: $statusfile was not created, but precompilation exited" + return false + 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 + finally + isdefined(Pkg, :respect_sysimage_versions) && Pkg.respect_sysimage_versions(true) + Pkg.activate(prev_project) + end +end diff --git a/test/pkgs/PersistentTasks/PersistentTask/Project.toml b/test/pkgs/PersistentTasks/PersistentTask/Project.toml new file mode 100644 index 00000000..a2322040 --- /dev/null +++ b/test/pkgs/PersistentTasks/PersistentTask/Project.toml @@ -0,0 +1,2 @@ +name = "PersistentTask" +uuid = "e5c298b6-d81d-47aa-a9ed-5ea983e22986" diff --git a/test/pkgs/PersistentTasks/PersistentTask/src/PersistentTask.jl b/test/pkgs/PersistentTasks/PersistentTask/src/PersistentTask.jl new file mode 100644 index 00000000..13d62057 --- /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..85a3af01 --- /dev/null +++ b/test/pkgs/PersistentTasks/TransientTask/Project.toml @@ -0,0 +1,2 @@ +name = "TransientTask" +uuid = "94ae9332-58b0-4342-989c-0a7e44abcca9" 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..9e6dc81f --- /dev/null +++ b/test/pkgs/PersistentTasks/UsesBoth/Project.toml @@ -0,0 +1,6 @@ +name = "UsesBoth" +uuid = "96f12b6e-60f8-43dc-b131-049a88a2f499" + +[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..82554089 --- /dev/null +++ b/test/test_persistent_tasks.jl @@ -0,0 +1,32 @@ +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 + @test !Aqua.has_persistent_tasks(getid("TransientTask")) + + result = Aqua.find_persistent_tasks_deps(getid("TransientTask")) + @test result == [] + + if Base.VERSION >= v"1.10-" + @test Aqua.has_persistent_tasks(getid("PersistentTask")) + + result = Aqua.find_persistent_tasks_deps(getid("UsesBoth")) + @test result == ["PersistentTask"] + end + filter!(str -> !occursin("PersistentTasks", str), LOAD_PATH) +end + +end diff --git a/test/test_smoke.jl b/test/test_smoke.jl index 3a3f5a00..fe78891a 100644 --- a/test/test_smoke.jl +++ b/test/test_smoke.jl @@ -16,6 +16,7 @@ Aqua.test_all( deps_compat = false, project_toml_formatting = false, piracy = false, + persistent_tasks = false, ) end # module