Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Run Pkg.precompile before tests in same julia setup as tests for correct caching #3281

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion Project.toml
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ UUIDs = "cf7118a7-6976-5b1a-9a39-7adc72f591a4"
p7zip_jll = "3f19e933-33d8-53b3-aaab-bd5110c3b7a0"

[compat]
HistoricalStdlibVersions = "0.1"
HistoricalStdlibVersions = "1.2"

[extras]
Test = "8dfed614-e22c-5e08-85e1-65c5234f0b40"
Expand Down
81 changes: 56 additions & 25 deletions src/Operations.jl
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,8 @@ import ..Artifacts: ensure_artifact_installed, artifact_names, extract_all_hashe
artifact_exists, select_downloadable_artifacts
using Base.BinaryPlatforms
import ...Pkg
import ...Pkg: pkg_server, Registry, pathrepr, can_fancyprint, printpkgstyle, stderr_f, OFFLINE_MODE, UPDATED_REGISTRY_THIS_SESSION, RESPECT_SYSIMAGE_VERSIONS
import ...Pkg: pkg_server, Registry, pathrepr, can_fancyprint, printpkgstyle, stderr_f, OFFLINE_MODE
import ...Pkg: UPDATED_REGISTRY_THIS_SESSION, RESPECT_SYSIMAGE_VERSIONS, should_autoprecompile

#########
# Utils #
Expand Down Expand Up @@ -1573,17 +1574,30 @@ function free(ctx::Context, pkgs::Vector{PackageSpec}; err_if_free=true)
end
end

function gen_test_code(source_path::String;
coverage=false,
julia_args::Cmd=``,
test_args::Cmd=``)
function gen_test_code(source_path::String; coverage, julia_args::Cmd, test_args::Cmd)
test_file = testfile(source_path)
code = """
$(Base.load_path_setup_code(false))
cd($(repr(dirname(test_file))))
append!(empty!(ARGS), $(repr(test_args.exec)))
include($(repr(test_file)))
"""
return gen_subprocess_cmd(code; coverage, julia_args)
end

function gen_test_precompile_code(; coverage, julia_args::Cmd, test_args::Cmd)
# Note that we cannot load the dev-ed Pkg here during Pkg testing
# so the `Pkg.precompile` that is run here is the one in the sysimage
code = """
Pkg = Base.require(Base.PkgId(Base.UUID("44cfe95a-1eb2-52ea-b672-e2afdf69b78f"), "Pkg"))
$(Base.load_path_setup_code(false))
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't get why this is needed. If the tests start with the correct project, why isn't just a Pkg.precompile enough?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I assumed that importing Pkg into the test process might be breaking, so I left that untouched

append!(empty!(ARGS), $(repr(test_args.exec)))
Pkg.precompile(warn_loaded = false)
"""
return gen_subprocess_cmd(code; coverage, julia_args)
end

function gen_subprocess_cmd(code::String; coverage, julia_args)
coverage_arg = if coverage isa Bool
coverage ? string("@", source_path) : "none"
elseif coverage isa AbstractString
Expand Down Expand Up @@ -1874,28 +1888,24 @@ function test(ctx::Context, pkgs::Vector{PackageSpec};
test_fn !== nothing && test_fn()
sandbox_ctx = Context(;io=ctx.io)
status(sandbox_ctx.env, sandbox_ctx.registries; mode=PKGMODE_COMBINED, io=sandbox_ctx.io, ignore_indent = false, show_usagetips = false)
Pkg._auto_precompile(sandbox_ctx, warn_loaded = false)
printpkgstyle(ctx.io, :Testing, "Running tests...")
flush(ctx.io)
cmd = gen_test_code(source_path; coverage=coverage, julia_args=julia_args, test_args=test_args)
p = run(pipeline(ignorestatus(cmd), stdout = sandbox_ctx.io, stderr = stderr_f()), wait = false)
interrupted = false
try
wait(p)
catch e
if e isa InterruptException
interrupted = true
print("\n")
printpkgstyle(ctx.io, :Testing, "Tests interrupted. Exiting the test process\n", color = Base.error_color())
# Give some time for the child interrupt handler to print a stacktrace and exit,
# then kill the process if still running
if timedwait(() -> !process_running(p), 4) == :timed_out
kill(p, Base.SIGKILL)

if should_autoprecompile()
# Precompile in a child process with the test julia args to ensure native caches match test setup
cmd = gen_test_precompile_code(; coverage, julia_args, test_args)
p, interrupted = subprocess_handler(cmd, ctx, sandbox_ctx, "Precompilation of test environment interrupted. Exiting the test precompilation process")
if !success(p)
if interrupted
return
else
printpkgstyle(ctx.io, :Testing, "Precompilation of test environment failed. Continuing to tests", color = Base.warn_color())
end
else
rethrow()
end
end

printpkgstyle(ctx.io, :Testing, "Running tests...")
flush(ctx.io)
cmd = gen_test_code(source_path; coverage, julia_args, test_args)
p, interrupted = subprocess_handler(cmd, ctx, sandbox_ctx, "Tests interrupted. Exiting the test process")
if success(p)
printpkgstyle(ctx.io, :Testing, pkg.name * " tests passed ")
elseif !interrupted
Expand Down Expand Up @@ -1945,7 +1955,28 @@ function test(ctx::Context, pkgs::Vector{PackageSpec};
end
end


# Handles the interrupting of a subprocess gracefully to avoid orphaning
function subprocess_handler(cmd::Cmd, ctx, sandbox_ctx, error_msg::String)
p = run(pipeline(ignorestatus(cmd), stdout = sandbox_ctx.io, stderr = stderr_f()), wait = false)
interrupted = false
try
wait(p)
catch e
if e isa InterruptException
interrupted = true
print("\n")
printpkgstyle(ctx.io, :Testing, "$error_msg\n", color = Base.error_color())
# Give some time for the child interrupt handler to print a stacktrace and exit,
# then kill the process if still running
if timedwait(() -> !process_running(p), 4) == :timed_out
kill(p, Base.SIGKILL)
end
else
rethrow()
end
end
return p, interrupted
end

# Display

Expand Down
3 changes: 2 additions & 1 deletion src/Pkg.jl
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,7 @@ stdout_f() = something(DEFAULT_IO[], stdout)
const PREV_ENV_PATH = Ref{String}("")

can_fancyprint(io::IO) = (io isa Base.TTY) && (get(ENV, "CI", nothing) != "true")
should_autoprecompile() = Base.JLOptions().use_compiled_modules == 1 && get_bool_env("JULIA_PKG_PRECOMPILE_AUTO"; default="true")

include("../ext/LazilyInitializedFields/LazilyInitializedFields.jl")

Expand Down Expand Up @@ -740,7 +741,7 @@ end
##################

function _auto_precompile(ctx::Types.Context, pkgs::Vector{String}=String[]; warn_loaded = true, already_instantiated = false)
if Base.JLOptions().use_compiled_modules == 1 && get_bool_env("JULIA_PKG_PRECOMPILE_AUTO"; default="true")
if should_autoprecompile()
Pkg.precompile(ctx, pkgs; internal_call=true, warn_loaded = warn_loaded, already_instantiated = already_instantiated)
end
end
Expand Down