Add persistent_tasks test (#174)

* Add persistent_tasks test

* Run JuliaFormatter, response to review comments

Co-authored-by: "Lars Göttgens <lars.goettgens@rwth-aachen.de>"

* Improve robustness

Flushing the io may prevent some of the CI hangs;
also print diagnostics in case of unexpected outcomes.

* More JuliaFormatter

* Escape paths (windows)

* Update src/persistent_tasks.jl

Co-authored-by: Max Horn <max@quendi.de>

* fails -> broken

* Add to CHANGELOG

* Adapt interface

* Update changelog

* Activate new test in `test_smoke`

* Update src/persistent_tasks.jl

Co-authored-by: Max Horn <max@quendi.de>

* Restore the previous environment

* Apply suggestions from code review

Co-authored-by: Lars Göttgens <lars.goettgens@gmail.com>

* Turn `test_persistent_tasks` on by default

---------

Co-authored-by: Max Horn <max@quendi.de>
Co-authored-by: Lars Göttgens <lars.goettgens@rwth-aachen.de>
Co-authored-by: Lars Göttgens <lars.goettgens@gmail.com>

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))

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

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

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

test/pkgs/PersistentTasks/PersistentTask/Project.toml

@@ -0,0 +1,2 @@
+name = "PersistentTask"
+uuid = "e5c298b6-d81d-47aa-a9ed-5ea983e22986"

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

test/pkgs/PersistentTasks/TransientTask/Project.toml

@@ -0,0 +1,2 @@
+name = "TransientTask"
+uuid = "94ae9332-58b0-4342-989c-0a7e44abcca9"

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

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"

test/pkgs/PersistentTasks/UsesBoth/src/UsesBoth.jl

@@ -0,0 +1,6 @@
+module UsesBoth
+
+using TransientTask
+using PersistentTask
+
+end # module UsesBoth

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

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