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

Make __precompile__(false) non-fatal for cachecompile / requires #28402

Merged
merged 2 commits into from
Aug 2, 2018
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
59 changes: 37 additions & 22 deletions base/loading.jl
Original file line number Diff line number Diff line change
Expand Up @@ -918,7 +918,6 @@ function _require(pkg::PkgId)
try
toplevel_load[] = false
# perform the search operation to select the module file require intends to load
name = pkg.name
path = locate_package(pkg)
if path === nothing
throw(ArgumentError("""
Expand All @@ -939,8 +938,8 @@ function _require(pkg::PkgId)
# but it was not handled by the precompile loader, complain
for (concrete_pkg, concrete_build_id) in _concrete_dependencies
if pkg == concrete_pkg
@warn """Module $name with build ID $concrete_build_id is missing from the cache.
This may mean module $name does not support precompilation but is imported by a module that does."""
@warn """Module $(pkg.name) with build ID $concrete_build_id is missing from the cache.
This may mean $pkg does not support precompilation but is imported by a module that does."""
if JLOptions().incremental != 0
# during incremental precompilation, this should be fail-fast
throw(PrecompilableError())
Expand All @@ -952,14 +951,19 @@ function _require(pkg::PkgId)
if (0 == ccall(:jl_generating_output, Cint, ())) || (JLOptions().incremental != 0)
# spawn off a new incremental pre-compile task for recursive `require` calls
# or if the require search declared it was pre-compiled before (and therefore is expected to still be pre-compilable)
cachefile = compilecache(pkg)
m = _require_from_serialized(cachefile)
if isa(m, Exception)
if !precompilableerror(m)
@warn "The call to compilecache failed to create a usable precompiled cache file for module $name" exception=m
cachefile = compilecache(pkg, path)
if isa(cachefile, Exception)
if !precompilableerror(cachefile)
@warn "The call to compilecache failed to create a usable precompiled cache file for $pkg" exception=m
end
# fall-through to loading the file locally
else
return
m = _require_from_serialized(cachefile)
if isa(m, Exception)
@warn "The call to compilecache failed to create a usable precompiled cache file for $pkg" exception=m
else
return
end
end
end
end
Expand Down Expand Up @@ -1108,18 +1112,18 @@ function create_expr_cache(input::String, output::String, concrete_deps::typeof(
in = io.in
try
write(in, """
begin
$(Base.load_path_setup_code())
Base._track_dependencies[] = true
empty!(Base._concrete_dependencies)
""")
begin
$(Base.load_path_setup_code())
Base._track_dependencies[] = true
Base.empty!(Base._concrete_dependencies)
""")
for (pkg, build_id) in concrete_deps
pkg_str = if pkg.uuid === nothing
"Base.PkgId($(repr(pkg.name)))"
else
"Base.PkgId(Base.UUID(\"$(pkg.uuid)\"), $(repr(pkg.name)))"
end
write(in, "push!(Base._concrete_dependencies, $pkg_str => $(repr(build_id)))\n")
write(in, "Base.push!(Base._concrete_dependencies, $pkg_str => $(repr(build_id)))\n")
end
write(io, "end\0")
uuid_tuple = uuid === nothing ? (0, 0) : convert(NTuple{2, UInt64}, uuid)
Expand All @@ -1128,7 +1132,14 @@ function create_expr_cache(input::String, output::String, concrete_deps::typeof(
if source !== nothing
write(in, "task_local_storage()[:SOURCE_PATH] = $(repr(source))\0")
end
write(in, "Base.include(Base.__toplevel__, $(repr(abspath(input))))\0")
write(in, """
try
Base.include(Base.__toplevel__, $(repr(abspath(input))))
catch ex
Base.precompilableerror(ex) || Base.rethrow(ex)
Base.@debug "Aborting `createexprcache'" exception=(Base.ErrorException("Declaration of __precompile__(false) not allowed"), Base.catch_backtrace())
Base.exit(125) # we define status = 125 means PrecompileableError
end\0""")
# TODO: cleanup is probably unnecessary here
if source !== nothing
write(in, "delete!(task_local_storage(), :SOURCE_PATH)\0")
Expand All @@ -1152,10 +1163,11 @@ This can be used to reduce package load times. Cache files are stored in
for important notes.
"""
function compilecache(pkg::PkgId)
# decide where to get the source file from
name = pkg.name
path = locate_package(pkg)
path === nothing && throw(ArgumentError("$pkg not found during precompilation"))
return compilecache(pkg, path)
end
function compilecache(pkg::PkgId, path::String)
# decide where to put the resulting cache file
cachefile = abspath(DEPOT_PATH[1], cache_file_entry(pkg))
cachepath = dirname(cachefile)
Expand All @@ -1170,17 +1182,20 @@ function compilecache(pkg::PkgId)
# run the expression and cache the result
verbosity = isinteractive() ? CoreLogging.Info : CoreLogging.Debug
if isfile(cachefile)
@logmsg verbosity "Recompiling stale cache file $cachefile for module $name"
@logmsg verbosity "Recompiling stale cache file $cachefile for $pkg"
else
@logmsg verbosity "Precompiling module $name"
@logmsg verbosity "Precompiling $pkg"
end
if success(create_expr_cache(path, cachefile, concrete_deps, pkg.uuid))
p = create_expr_cache(path, cachefile, concrete_deps, pkg.uuid)
if success(p)
# append checksum to the end of the .ji file:
open(cachefile, "a+") do f
write(f, _crc32c(seekstart(f)))
end
elseif p.exitcode == 125
return PrecompilableError()
else
error("Failed to precompile $name to $cachefile.")
error("Failed to precompile $pkg to $cachefile.")
end
return cachefile
end
Expand Down
45 changes: 13 additions & 32 deletions stdlib/Pkg/src/API.jl
Original file line number Diff line number Diff line change
Expand Up @@ -445,52 +445,33 @@ precompile() = precompile(Context())
function precompile(ctx::Context)
printpkgstyle(ctx, :Precompiling, "project...")

pkgids = [Base.PkgId(UUID(uuid), name) for (name, uuid) in ctx.env.project["deps"] if !(UUID(uuid) in keys(ctx.stdlibs))]
if ctx.env.pkg !== nothing && isfile( joinpath( dirname(ctx.env.project_file), "src", ctx.env.pkg.name * ".jl"))
pkgids = [Base.PkgId(UUID(uuid), name) for (name, uuid) in ctx.env.project["deps"] if !(UUID(uuid) in keys(ctx.stdlibs))]
if ctx.env.pkg !== nothing && isfile( joinpath( dirname(ctx.env.project_file), "src", ctx.env.pkg.name * ".jl") )
push!(pkgids, Base.PkgId(ctx.env.pkg.uuid, ctx.env.pkg.name))
end

needs_to_be_precompiled = String[]
# TODO: since we are a complete list, but not topologically sorted, handling of recursion will be completely at random
for pkg in pkgids
paths = Base.find_all_in_cache_path(pkg)
sourcepath = Base.locate_package(pkg)
if sourcepath == nothing
# XXX: this isn't supposed to be fatal
cmderror("couldn't find path to $(pkg.name) when trying to precompilie project")
end
found_matching_precompile = false
stale = true
for path_to_try in paths::Vector{String}
staledeps = Base.stale_cachefile(sourcepath, path_to_try)
if !(staledeps isa Bool)
found_matching_precompile = true
end
staledeps === true && continue
# TODO: else, this returns a list of packages that may be loaded to make this valid (the topological list)
stale = false
break
end
if !found_matching_precompile
# Don't bother attempting to precompile packages that appear to contain `__precompile__(false)`
source = read(sourcepath, String)
if !occursin(r"__precompile__\(false\)", source)
push!(needs_to_be_precompiled, pkg.name)
end
if stale
printpkgstyle(ctx, :Precompiling, pkg.name)
Base.compilecache(pkg, sourcepath)
end
end

# Perhaps running all the imports in the same process would avoid some overheda.
# Julia starts pretty fast though (0.3 seconds)
code = join(["import " * pkg for pkg in needs_to_be_precompiled], '\n') * "\nexit(0)"
for (i, pkg) in enumerate(needs_to_be_precompiled)
code = """
$(Base.load_path_setup_code())
import $pkg
"""
printpkgstyle(ctx, :Precompiling, pkg * " [$i of $(length(needs_to_be_precompiled))]")
run(pipeline(ignorestatus(```
$(Base.julia_cmd()) -O$(Base.JLOptions().opt_level) --color=no --history-file=no
--startup-file=$(Base.JLOptions().startupfile != 2 ? "yes" : "no")
--compiled-modules="yes"
--depwarn=no
--eval $code
```)))
end
return nothing
nothing
end


Expand Down
14 changes: 6 additions & 8 deletions test/precompile.jl
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ try

write(FooBase_file,
"""
false && __precompile__(false)
module $FooBase_module
import Base: hash, >
struct fmpz end
Expand Down Expand Up @@ -279,18 +280,15 @@ try
Baz_file = joinpath(dir, "Baz.jl")
write(Baz_file,
"""
__precompile__(false)
true && __precompile__(false)
module Baz
baz() = 1
end
""")

@test_warn "ERROR: LoadError: Declaring __precompile__(false) is not allowed in files that are being precompiled.\nStacktrace:\n [1] __precompile__" try
Base.compilecache(Base.PkgId("Baz")) # from __precompile__(false)
error("__precompile__ disabled test failed")
catch exc
isa(exc, ErrorException) || rethrow(exc)
occursin("__precompile__(false)", exc.msg) && rethrow(exc)
end
@test Base.compilecache(Base.PkgId("Baz")) == Base.PrecompilableError() # due to __precompile__(false)
@eval using Baz
@test Base.invokelatest(Baz.baz) == 1
Copy link

Choose a reason for hiding this comment

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

Something in 1.7.0 is breaking this on some machines.

Copy link

Choose a reason for hiding this comment

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


# Issue #12720
FooBar1_file = joinpath(dir, "FooBar1.jl")
Expand Down