From 71fa11f0427fc66f2328cddbba865852fa47e0f1 Mon Sep 17 00:00:00 2001 From: Florian Date: Fri, 21 Jun 2024 18:29:40 +0200 Subject: [PATCH] Raise an error when using `include_dependency` with non-existent file or directory (#53286) Replaces #52105 Fixes #52063 There is a question about whether the `ispath & uperm` check should be moved inside the `_track_dependencies[]` check (like it was in #52105), which would make it such that any errors are thrown only during precompilation. --------- Co-authored-by: Qian Long Co-authored-by: Jameson Nash --- base/loading.jl | 13 ++++++++--- test/loading.jl | 24 ++++++++++++++++++- test/precompile.jl | 57 +++++++++++++++++++++++++++++++++++++++++----- 3 files changed, 84 insertions(+), 10 deletions(-) diff --git a/base/loading.jl b/base/loading.jl index 065b287b32dd6..f432268208008 100644 --- a/base/loading.jl +++ b/base/loading.jl @@ -2049,14 +2049,21 @@ const include_callbacks = Any[] const _concrete_dependencies = Pair{PkgId,UInt128}[] # these dependency versions are "set in stone", and the process should try to avoid invalidating them const _require_dependencies = Any[] # a list of (mod, abspath, fsize, hash, mtime) tuples that are the file dependencies of the module currently being precompiled const _track_dependencies = Ref(false) # set this to true to track the list of file dependencies -function _include_dependency(mod::Module, _path::AbstractString; track_content=true) +function _include_dependency(mod::Module, _path::AbstractString; track_content=true, + path_may_be_dir=false) prev = source_path(nothing) if prev === nothing path = abspath(_path) else path = normpath(joinpath(dirname(prev), _path)) end - if _track_dependencies[] + if !_track_dependencies[] + if !path_may_be_dir && !isfile(path) + throw(SystemError("opening file $(repr(path))", Libc.ENOENT)) + elseif path_may_be_dir && !Filesystem.isreadable(path) + throw(SystemError("opening file or folder $(repr(path))", Libc.ENOENT)) + end + else @lock require_lock begin if track_content hash = isdir(path) ? _crc32c(join(readdir(path))) : open(_crc32c, path, "r") @@ -2086,7 +2093,7 @@ no effect outside of compilation. Keyword argument `track_content` requires at least Julia 1.11. """ function include_dependency(path::AbstractString; track_content::Bool=false) - _include_dependency(Main, path, track_content=track_content) + _include_dependency(Main, path, track_content=track_content, path_may_be_dir=true) return nothing end diff --git a/test/loading.jl b/test/loading.jl index 9230bc75599e6..72589d4210adc 100644 --- a/test/loading.jl +++ b/test/loading.jl @@ -1549,7 +1549,29 @@ end end file = joinpath(depot, "dev", "non-existent.jl") - @test_throws SystemError("opening file $(repr(file))") include(file) + @test try + include(file); false + catch e + @test e isa SystemError + @test e.prefix == "opening file $(repr(file))" + true + end + touch(file) + @test include_dependency(file) === nothing + chmod(file, 0x000) + + # same for include_dependency: #52063 + dir = mktempdir() do dir + @test include_dependency(dir) === nothing + dir + end + @test try + include_dependency(dir); false + catch e + @test e isa SystemError + @test e.prefix == "opening file or folder $(repr(dir))" + true + end end end diff --git a/test/precompile.jl b/test/precompile.jl index ce606d166f695..e4d2b65c66c4b 100644 --- a/test/precompile.jl +++ b/test/precompile.jl @@ -6,6 +6,8 @@ using REPL # doc lookup function include("precompile_utils.jl") Foo_module = :Foo4b3a94a1a081a8cb +foo_incl_dep = :foo4b3a94a1a081a8cb +bar_incl_dep = :bar4b3a94a1a081a8cb Foo2_module = :F2oo4b3a94a1a081a8cb FooBase_module = :FooBase4b3a94a1a081a8cb @eval module ConflictingBindings @@ -75,6 +77,8 @@ precompile_test_harness(false) do dir Foo_file = joinpath(dir, "$Foo_module.jl") Foo2_file = joinpath(dir, "$Foo2_module.jl") FooBase_file = joinpath(dir, "$FooBase_module.jl") + foo_file = joinpath(dir, "$foo_incl_dep.jl") + bar_file = joinpath(dir, "$bar_incl_dep.jl") write(FooBase_file, """ @@ -123,11 +127,11 @@ precompile_test_harness(false) do dir # test that docs get reconnected @doc "foo function" foo(x) = x + 1 - include_dependency("foo.jl") - include_dependency("foo.jl") + include_dependency("$foo_incl_dep.jl") + include_dependency("$foo_incl_dep.jl") module Bar public bar - include_dependency("bar.jl") + include_dependency("$bar_incl_dep.jl") end @doc "Bar module" Bar # this needs to define the META dictionary via eval @eval Bar @doc "bar function" bar(x) = x + 2 @@ -270,6 +274,8 @@ precompile_test_harness(false) do dir oid_mat_int = objectid(a_mat_int) end """) + # Issue #52063 + touch(foo_file); touch(bar_file) # Issue #12623 @test __precompile__(false) === nothing @@ -412,8 +418,7 @@ precompile_test_harness(false) do dir modules, (deps, _, requires), required_modules, _... = Base.parse_cache_header(cachefile) discard_module = mod_fl_mt -> mod_fl_mt.filename @test modules == [ Base.PkgId(Foo) => Base.module_build_id(Foo) % UInt64 ] - # foo.jl and bar.jl are never written to disk, so they are not relocatable - @test map(x -> x.filename, deps) == [ Foo_file, joinpath("@depot", "foo.jl"), joinpath("@depot", "bar.jl") ] + @test map(x -> x.filename, deps) == [ Foo_file, joinpath("@depot", foo_file), joinpath("@depot", bar_file) ] @test requires == [ Base.PkgId(Foo) => Base.PkgId(string(FooBase_module)), Base.PkgId(Foo) => Base.PkgId(Foo2), Base.PkgId(Foo) => Base.PkgId(Test), @@ -422,7 +427,7 @@ precompile_test_harness(false) do dir @test !isempty(srctxt) && srctxt == read(Foo_file, String) @test_throws ErrorException Base.read_dependency_src(cachefile, "/tmp/nonexistent.txt") # dependencies declared with `include_dependency` should not be stored - @test_throws ErrorException Base.read_dependency_src(cachefile, joinpath(dir, "foo.jl")) + @test_throws ErrorException Base.read_dependency_src(cachefile, joinpath(dir, foo_file)) modules, deps1 = Base.cache_dependencies(cachefile) modules_ok = merge( @@ -1996,4 +2001,44 @@ precompile_test_harness("Generated Opaque") do load_path end end +precompile_test_harness("Issue #52063") do load_path + fname = joinpath(load_path, "i_do_not_exist.jl") + @test try + include_dependency(fname); false + catch e + @test e isa SystemError + @test e.prefix == "opening file or folder $(repr(fname))" + true + end + touch(fname) + @test include_dependency(fname) === nothing + chmod(fname, 0x000) + @test try + include_dependency(fname); false + catch e + @test e isa SystemError + @test e.prefix == "opening file or folder $(repr(fname))" + true + end broken=Sys.iswindows() + dir = mktempdir() do dir + @test include_dependency(dir) === nothing + chmod(dir, 0x000) + @test try + include_dependency(dir); false + catch e + @test e isa SystemError + @test e.prefix == "opening file or folder $(repr(dir))" + true + end broken=Sys.iswindows() + dir + end + @test try + include_dependency(dir); false + catch e + @test e isa SystemError + @test e.prefix == "opening file or folder $(repr(dir))" + true + end +end + finish_precompile_test!()