Skip to content

Raise an error when using include_dependency with non-existent file or directory #53286

New issue

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

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

Already on GitHub? # to your account

Merged
merged 19 commits into from
Jun 21, 2024

Conversation

fatteneder
Copy link
Member

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.

@fatteneder
Copy link
Member Author

The newly added checks now cause include() to throw earlier, instead of when attempting to open() a file.
This then changes the error type thrown by include() from a SystemError to an ArgumentError.
This doesn't seem right ...

The alternative is throwing during precompilation, but then there is this potentially confusing behavior that include_dependency() does not throw when you call it from the REPL. (the behavior itself is ok, its just that it is confusing to debug then)

Any thoughts?

fatteneder added a commit that referenced this pull request Feb 20, 2024
As discussed here:
#53286 (comment)

Readds the methods that were removed in
#12819.
@fatteneder fatteneder force-pushed the patch-1 branch 2 times, most recently from 4cfaf5e to 959f6b1 Compare February 25, 2024 13:01
@fatteneder
Copy link
Member Author

fatteneder commented Feb 25, 2024

As it currently is, I would have to update tests from Distributed, but it lives in its own repo.
I thought I would do that with a PR to Distributed that fixes the test with a @static VERSION check, but for that I need a concrete dev.### nr which I only get after this being merged.

What's the procedure to get this fixed?

KristofferC pushed a commit that referenced this pull request Feb 26, 2024
As discussed here:
#53286 (comment)

Readds the methods that were removed in
#12819.

(cherry picked from commit ea2b255)
tecosaur pushed a commit to tecosaur/julia that referenced this pull request Mar 4, 2024
@fatteneder fatteneder requested a review from vtjnash March 22, 2024 12:49
Copy link
Member

@vtjnash vtjnash left a comment

Choose a reason for hiding this comment

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

It is maybe a bit awkward that setting _track_dependencies[] still changes what errors are detected and how they get detected inside _include_dependency, but this seems at least somewhat better

@fatteneder fatteneder added compiler:precompilation Precompilation of modules backport 1.11 Change should be backported to release-1.11 labels Apr 22, 2024
@fatteneder fatteneder force-pushed the patch-1 branch 2 times, most recently from 80ac921 to c49e179 Compare April 23, 2024 18:32
@KristofferC KristofferC mentioned this pull request Apr 25, 2024
59 tasks
KristofferC added a commit that referenced this pull request May 28, 2024
Backported PRs:
- [x] #53665 <!-- use afoldl instead of tail recursion for tuples -->
- [x] #53976 <!-- LinearAlgebra: LazyString in interpolated error
messages -->
- [x] #54005 <!-- make `view(::Memory, ::Colon)` produce a Vector -->
- [x] #54010 <!-- Overload `Base.literal_pow` for `AbstractQ` -->
- [x] #54069 <!-- Allow PrecompileTools to see MI's inferred by foreign
abstract interpreters -->
- [x] #53750 <!-- inference correctness: fields and globals can revert
to undef -->
- [x] #53984 <!-- Profile: fix heap snapshot is valid char check -->
- [x] #54102 <!-- Explicitly compute stride in unaliascopy for SubArray
-->
- [x] #54070 <!-- Fix integer overflow in `skip(s::IOBuffer,
typemax(Int64))` -->
- [x] #54013 <!-- Support case-changes to Annotated{String,Char}s -->
- [x] #53941 <!-- Fix writing of AnnotatedChars to AnnotatedIOBuffer -->
- [x] #54137 <!-- Fix typo in docs for `partialsortperm` -->
- [x] #54129 <!-- use correct size when creating output data from an
IOBuffer -->
- [x] #54153 <!-- Fixup IdSet docstring -->
- [x] #54143 <!-- Fix `make install` from tarballs -->
- [x] #54151 <!-- LinearAlgebra: Correct zero element in
`_generic_matvecmul!` for block adj/trans -->
- [x] #54213 <!-- Add `public` statement to `Base.GC` -->
- [x] #54222 <!-- Utilize correct tbaa when emitting stores of unions.
-->
- [x] #54233 <!-- set MAX_OS_WRITE on unix -->
- [x] #54255 <!-- fix `_checked_mul_dims` in the presence of 0s and
overflow. -->
- [x] #54259 <!-- Fix typo in `readuntil` -->
- [x] #54251 <!-- fix typo in gc_mark_memory8 when chunking a large
array -->
- [x] #54276 <!-- Fix solve for complex `Hermitian` with non-vanishing
imaginary part on diagonal -->
- [x] #54248 <!-- ensure package callbacks are invoked when no valid
precompile file exists for an "auto loaded" stdlib -->
- [x] #54308 <!-- Implement eval-able AnnotatedString 2-arg show -->
- [x] #54302 <!-- Specialised substring equality for annotated strs -->
- [x] #54243 <!-- prevent `package_callbacks` to run multiple time for a
single package -->
- [x] #54350 <!-- add a precompile signature to Artifacts code that is
used by JLLs -->
- [x] #54331 <!-- correctly track freed bytes in
jl_genericmemory_to_string -->
- [x] #53509 <!-- revert moving "creating packages" from Pkg.jl -->
- [x] #54335 <!-- When accessing the data pointer for an array, first
decay it to a Derived Pointer -->
- [x] #54239 <!-- Make sure `fieldcount` constant-folds for `Tuple{...}`
-->
- [x] #54288
- [x] #54067
- [x] #53715 <!-- Add read/write specialisation for IOContext{AnnIO} -->
- [x] #54289 <!-- Rework annotation ordering/optimisations -->
- [x] #53815 <!-- create phantom task for GC threads -->
- [x] #54130 <!-- inference: handle `LimitedAccuracy` in
`handle_global_assignment!` -->
- [x] #54428 <!-- Move ConsoleLogging.jl into Base -->
- [x] #54332 <!-- Revert "add unsetindex support to more copyto methods
(#51760)" -->
- [x] #53826 <!-- Make all command-line options documented in all
related files -->
- [x] #54465 <!-- typeintersect: conservative typevar subtitution during
`finish_unionall` -->
- [x] #54514 <!-- typeintersect: followup cleanup for the nothrow path
of type instantiation -->
- [x] #54499 <!-- make `@doc x` work without REPL loaded -->
- [x] #54210 <!-- attach finalizer in `mmap` to the correct object -->
- [x] #54359 <!-- Pkg REPL: cache `pkg_mode` lookup -->

Non-merged PRs with backport label:
- [ ] #54471 <!-- Actually setup jit targets when compiling
packageimages instead of targeting only one -->
- [ ] #54457 <!-- Make `String(::Memory)` copy -->
- [ ] #54323 <!-- inference: fix too conservative effects for recursive
cycles -->
- [ ] #54322 <!-- effects: add new `@consistent_overlay` macro -->
- [ ] #54191 <!-- make `AbstractPipe` public -->
- [ ] #53957 <!-- tweak how filtering is done for what packages should
be precompiled -->
- [ ] #53882 <!-- Warn about cycles in extension precompilation -->
- [ ] #53707 <!-- Make ScopedValue public -->
- [ ] #53452 <!-- RFC: allow Tuple{Union{}}, returning Union{} -->
- [ ] #53402 <!-- Add `jl_getaffinity` and `jl_setaffinity` -->
- [ ] #53286 <!-- Raise an error when using `include_dependency` with
non-existent file or directory -->
- [ ] #52694 <!-- Reinstate similar for AbstractQ for backward
compatibility -->
- [ ] #51479 <!-- prevent code loading from lookin in the versioned
environment when building Julia -->
@KristofferC KristofferC mentioned this pull request May 29, 2024
60 tasks
@fatteneder fatteneder added the merge me PR is reviewed. Merge when all tests are passing label Jun 7, 2024
@fatteneder fatteneder removed the merge me PR is reviewed. Merge when all tests are passing label Jun 12, 2024
@fingolfin
Copy link
Member

Unfortunately this has a conflict now :-(

Since depending on a non-existent file or directory is strange, raise an error instead of silently triggering precompilation.

Fixes JuliaLang#52063
@fatteneder fatteneder force-pushed the patch-1 branch 3 times, most recently from 28d4c59 to a4c2b01 Compare June 21, 2024 07:19
@fatteneder
Copy link
Member Author

There was a bit of a struggle with the tests on windows:
We use isreadable in one case, but that can give inaccurate results on windows, so I decided to skip the affected tests on windows.

@fatteneder fatteneder added the merge me PR is reviewed. Merge when all tests are passing label Jun 21, 2024
@vchuravy vchuravy merged commit 71fa11f into JuliaLang:master Jun 21, 2024
8 checks passed
@fatteneder fatteneder deleted the patch-1 branch June 21, 2024 17:17
@oscardssmith oscardssmith removed the merge me PR is reviewed. Merge when all tests are passing label Jun 21, 2024
KristofferC added a commit that referenced this pull request Jun 25, 2024
Backported PRs:
- [x] #54361 <!-- [LBT] Upgrade to v5.9.0 -->
- [x] #54474 <!-- Unalias source from dest in copytrito -->
- [x] #54548 <!-- Fixes for bitcast bugs with LLVM 17 / opaque pointers
-->
- [x] #54191 <!-- make `AbstractPipe` public -->
- [x] #53402 <!-- Add `jl_getaffinity` and `jl_setaffinity` -->
- [x] #53356 <!-- Rename at-scriptdir project argument to at-script and
search upwards for Project.toml -->
- [x] #54545 <!-- typeintersect: fix incorrect innervar handling under
circular env -->
- [x] #54586 <!-- Set storage class of julia globals to dllimport on
windows to avoid auto-import weirdness. Forward port of #54572 -->
- [x] #54587 <!-- Accomodate for rectangular matrices in `copytrito!`
-->
- [x] #54617 <!-- CLI: Use `GetModuleHandleExW` to locate libjulia.dll
-->
- [x] #54605 <!-- Allow libquadmath to also fail as it is not available
on all systems -->
- [x] #54634 <!-- Fix trampoline assembly for build on clang 18 on apple
silicon -->
- [x] #54635 <!-- Aggressive constprop in trevc! to stabilize triangular
eigvec -->
- [x] #54645 <!-- ensure we set the right value to gc_first_tid -->
- [x] #54554 <!-- make elsize public -->
- [x] #54648 <!-- Construct LazyString in error paths for tridiag -->
- [x] #54658 <!-- fix missing uuid check on extension when finding the
location of an extension -->
- [x] #54594 <!-- Switch to Pkg mode prompt immediately and load Pkg in
the background -->
- [x] #54669 <!-- Improve error message in inplace transpose -->
- [x] #54671 <!-- Add boundscheck in bindingkey_eq to avoid OOB access
due to data race -->
- [x] #54672 <!-- make: Fix `sed` command for LLVM libraries with no
symbol versioning -->
- [x] #54624 <!-- more precise aliasing checks for SubArray -->
- [x] #54679 <!-- 🤖 [master] Bump the Distributed stdlib from 6a07d98 to
6c7cdb5 -->
- [x] #54604 <!-- Fix tbaa annotation on union selector bytes inside of
structs -->
- [x] #54690 <!-- Fix assertion/crash when optimizing function with dead
basic block -->
- [x] #54704 <!-- LazyString in reinterpretarray error messages -->
- [x] #54718 <!-- fix prepend StackOverflow issue -->
- [x] #54674 <!-- Reimplement dummy pkg prompt as standard prompt -->
- [x] #54737 <!-- LazyString in interpolated error messages involving
types -->
- [x] #54642 <!-- Document GenericMemory and AtomicMemory -->
- [x] #54713 <!-- make: use `readelf` for LLVM symbol version detection
-->
- [x] #54760 <!-- REPL: improve prompt! async function handler -->
- [x] #54606 <!-- fix double-counting and non-deterministic results in
`summarysize` -->
- [x] #54759 <!-- REPL: Fully populate the dummy Pkg prompt -->
- [x] #54702 <!-- lowering: Recognize argument destructuring inside
macro hygiene -->
- [x] #54678 <!-- Don't let setglobal! implicitly create bindings -->
- [x] #54730 <!-- Fix uuidkey of exts in fast path of `require_stdlib`
-->
- [x] #54765 <!-- Handle no-postdominator case in finalizer pass -->
- [x] #54591 <!-- Don't expose guard pages to malloc_stack API consumers
-->
- [x] #54755 <!-- [TOML] remove Dates hack, replace with explicit usage
-->
- [x] #54721 <!-- add try/catch around scheduler to reset sleep state
-->
- [x] #54631 <!-- Avoid concatenating LazyString in setindex! for
triangular matrices -->
- [x] #54322 <!-- effects: add new `@consistent_overlay` macro -->
- [x] #54785
- [x] #54865
- [x] #54815
- [x] #54795
- [x] #54779
- [x] #54837 

Contains multiple commits, manual intervention needed:
- [ ] #52694 <!-- Reinstate similar for AbstractQ for backward
compatibility -->
- [ ] #54649 <!-- Less restrictive copyto! signature for triangular
matrices -->

Non-merged PRs with backport label:
- [ ] #54779 <!-- make recommendation clearer on manifest version
mismatch -->
- [ ] #54739 <!-- finish implementation of upgradable stdlibs -->
- [ ] #54738 <!-- serialization: fix relocatability bug -->
- [ ] #54574 <!-- Make ScopedValues public -->
- [ ] #54457 <!-- Make `String(::Memory)` copy -->
- [ ] #53957 <!-- tweak how filtering is done for what packages should
be precompiled -->
- [ ] #53452 <!-- RFC: allow Tuple{Union{}}, returning Union{} -->
- [ ] #53286 <!-- Raise an error when using `include_dependency` with
non-existent file or directory -->
- [ ] #51479 <!-- prevent code loading from lookin in the versioned
environment when building Julia -->
@KristofferC KristofferC mentioned this pull request Jun 25, 2024
42 tasks
KristofferC pushed a commit that referenced this pull request Jun 26, 2024
KristofferC pushed a commit that referenced this pull request Jul 23, 2024
… 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 <longqian95@gmail.com>
Co-authored-by: Jameson Nash <vtjnash@gmail.com>
(cherry picked from commit 71fa11f)
KristofferC added a commit that referenced this pull request Jul 26, 2024
Backported PRs:
- [x] #54201 <!-- Fix generic triangular solves with empty matrices -->
- [x] #54358 <!-- Create `jl_clear_coverage_data` to dynamically reset
coverage -->
- [x] #54908 <!-- LazyString in interpolated error messages in
threadingconstructs -->
- [x] #54952 <!-- LAPACK: Avoid repr call in `chkvalidparam` -->
- [x] #54898 <!-- fix concurrent module loading return value -->
- [x] #55082 <!-- Add fast method for copyto!(::Memory, ::Memory) -->
- [x] #55084 <!-- Use triple quotes in TOML.print when string contains
newline -->
- [x] #55141 <!-- Update the aarch64 devdocs to reflect the current
state of its support -->
- [x] #54955 <!-- don't throw EOFError from sleep -->
- [x] #54871 <!-- Make warn missed transformations pass optional -->
- [x] #55178 <!-- Compat for `Base.@nospecializeinfer` -->
- [x] #55197 <!-- compat notice for a[begin] indexing -->
- [x] #54917 <!-- Fix potential underrun with annotation merging -->
- [x] #55209 <!-- correction to compat notice for a[begin] -->
- [x] #55203 <!-- document mutable struct const fields -->
- [x] #54791 <!-- Bump libblastrampoline to v5.10.1 -->
- [x] #54950 <!-- SuiteSparse: Bump version -->
- [x] #54956 <!-- Fix accidental early evaluation of imported `using`
binding -->
- [x] #54996 <!-- inference: add missing `MustAlias` widening in
`_getfield_tfunc` -->
- [x] #55070 <!-- LinearAlgebra: LazyString in error messages for
Diagonal/Bidiagonal -->
- [x] #54574 <!-- Make ScopedValues public -->
- [x] #54739 <!-- finish implementation of upgradable stdlibs -->
- [x] #54965 <!-- RFC: Make `include_dependency(path;
track_content=true)` the default -->
- [x] #53286 <!-- Raise an error when using `include_dependency` with
non-existent file or directory -->
- [x] #55066 <!-- fix loading of repeated/concurrent modules -->
- [x] #52694 <!-- Reinstate similar for AbstractQ for backward
compatibility -->
- [x] #55218 <!-- Artifacts: use a different way of getting the UUID of
a module -->
- [x] #54891 <!-- #54739-related fixes for loading stdlibs -->
- [x] #55072 <!-- trace-compile: don't generate `precompile` statements
for OpaqueClosure methods -->
- [x] #55188 <!-- Make Core.TypeofUnion use the type method table -->

Need manual backport:
- [ ] #55052 <!-- Fix `(l/r)mul!` with `Diagonal`/`Bidiagonal` -->


Contains multiple commits, manual intervention needed:

Non-merged PRs with backport label:
- [ ] #55169 <!-- `propertynames` for SVD respects private argument -->
- [ ] #55148 <!-- Random: Mark unexported public symbols as public -->
- [ ] #55017 <!-- TOML: Make `Dates` a type parameter -->
- [ ] #55013 <!-- [docs] change docstring to match code -->
- [ ] #54919 <!-- Fix annotated join with non-concrete eltype iters -->
- [ ] #54457 <!-- Make `String(::Memory)` copy -->
- [ ] #53957 <!-- tweak how filtering is done for what packages should
be precompiled -->
- [ ] #51479 <!-- prevent code loading from lookin in the versioned
environment when building Julia -->
- [ ] #50813 <!-- More doctests for Sockets and capitalization fix -->
- [ ] #50157 <!-- improve docs for `@inbounds` and
`Base.@propagate_inbounds` -->
- [ ] #41244 <!-- Fix shell `cd` error when working dir has been deleted
-->
@KristofferC KristofferC removed the backport 1.11 Change should be backported to release-1.11 label Jul 26, 2024
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
compiler:precompilation Precompilation of modules
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Using include_dependency with a non-existent file triggers precompilation at every startup
7 participants