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

finish implementation of upgradable stdlibs #54739

Merged
merged 1 commit into from
Jun 21, 2024
Merged

Conversation

vtjnash
Copy link
Member

@vtjnash vtjnash commented Jun 8, 2024

Fixes #53983

Unfortunately, TOML has a design problem with the way it is used by Pkg (because of the way they use Dates without depending on it correctly, they want to assume things that are false now), so this does not yet work correctly for Pkg. But maybe if we change some of the uses of loaded_packages to explicit_loaded_modules in loading, we can try to make the way those three packages interact to still kind of mostly work. (Fixed in Pkg now and removed from TOML in #54755)

Also currently one test is broken, as we need some more code rearrangement to make it legal again for an __init__ method to import code.

@vtjnash vtjnash added the backport 1.11 Change should be backported to release-1.11 label Jun 8, 2024
@vtjnash vtjnash mentioned this pull request Jun 10, 2024
14 tasks
vtjnash added a commit that referenced this pull request Jun 10, 2024
This hack will cease functioning soon (#54739), so it must be removed so
that packages can stop relying on it.

As an aid, now provide some basic conversion and printing support for
the Base.TOML variants of this as well such that users can round-trip
the contents (if they are well-formed as a date/time).
vtjnash added a commit that referenced this pull request Jun 13, 2024
This hack will cease functioning soon (#54739), so it must be removed so
that packages can stop relying on it.

As an aid, now provide some basic conversion and printing support for
the Base.TOML variants of this as well such that users can round-trip
the contents (if they are well-formed as a date/time).
@KristofferC KristofferC mentioned this pull request Jun 13, 2024
60 tasks
KristofferC pushed a commit that referenced this pull request Jun 13, 2024
This hack will cease functioning soon (#54739), so it must be removed so
that packages can stop relying on it.

As an aid, now provide some basic conversion and printing support for
the Base.TOML variants of this as well such that users can round-trip
the contents (if they are well-formed as a date/time).

(cherry picked from commit 2ce12e9)
Fixes #53983

This breaks one test, which relied on it being undefined behavior to
rely on the execution of `__init__` having occurred before `using` is
allowed to return. That usability issue is now (partially) eliminated.
@vtjnash vtjnash marked this pull request as ready for review June 20, 2024 17:46
@vtjnash vtjnash added the merge me PR is reviewed. Merge when all tests are passing label Jun 20, 2024
const explicit_loaded_modules = Dict{PkgId,Module}()
const explicit_loaded_modules = Dict{PkgId,Module}() # Emptied on Julia start
const loaded_modules = Dict{PkgId,Module}() # available to be explicitly loaded
const loaded_precompiles = Dict{Pair{PkgId,UInt128},Module}() # extended (complete) list of modules, available to be loaded
Copy link
Member

Choose a reason for hiding this comment

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

Can we be clearer about the differences between these three dicts? Is the first one modules loaded during this session? Is the second one currently visible modules, and the third one all modules? I'm not sure.

@vtjnash vtjnash merged commit a1a2ac6 into master Jun 21, 2024
5 of 8 checks passed
@vtjnash vtjnash deleted the jn/upgradable-stdlibs branch June 21, 2024 16:58
@matthias314
Copy link
Contributor

I'm sorry, but this doesn't fully fix #53983. There are still problems when using startup.jl. The setting is as follows: no ~/.julia directory, JULIA_DEPOT_PATH=/tmp/julia-depot containing the file /tmp/julia-depot/config/startup.jl with contents

using InteractiveUtils

Then:

/usr/local/git/julia$ ./julia
[ Info: Precompiling InteractiveUtils [b77e0a4c-d291-57a0-90e8-8db25a27a240] 
[ Info: Precompiling REPL [3fa0cd96-eef1-5676-8a61-b3b8758bbffb] 
               _
   _       _ _(_)_     |  Documentation: https://docs.julialang.org
  (_)     | (_) (_)    |
   _ _   _| |_  __ _   |  Type "?" for help, "]?" for Pkg help.
  | | | | | | |/ _` |  |
  | | |_| | | | (_| |  |  Version 1.12.0-DEV.766 (2024-06-21)
 _/ |\__'_|_|_|\__'_|  |  Commit 9d8ecaa899 (0 days old master)
|__/                   |

julia>

[ now I press the key "]" ]

(@v1.12) pkg> Unhandled Task ERROR: ArgumentError: Package REPLExt [e5eb5ef1-03cf-53a7-ae1d-5a66b08e832b] is required but does not seem to be installed:
 - Run `Pkg.instantiate()` to install all recorded dependencies.

Starting Julia with --startup-file=no works fine. Unsetting JULIA_DEPOT_PATH and moving startup.jl to ~/.julia/config instead also works fine.

Julia Version 1.12.0-DEV.766
Commit 9d8ecaa899 (2024-06-21 17:00 UTC)
Platform Info:
  OS: Linux (x86_64-linux-gnu)
  CPU: 4 × Intel(R) Core(TM) i3-10110U CPU @ 2.10GHz
  WORD_SIZE: 64
  LLVM: libLLVM-17.0.6 (ORCJIT, skylake)
Threads: 1 default, 0 interactive, 1 GC (on 4 virtual cores)
Environment:
  JULIA_DEPOT_PATH = /tmp/julia-depot

@vtjnash
Copy link
Member Author

vtjnash commented Jun 22, 2024

Much appreciated. I had fixed this earlier in the PR history, then forgot why I had the code to handle this and removed it again. Now it is clear again why I needed to handle this case with that example.

vtjnash added a commit that referenced this pull request Jun 22, 2024
This was an accidental late change in the PR, and I forgot why this
needed to be exactly written this way (and why this wasn't equivalent).

Followup to #54739 fixing #53983
@IanButterworth IanButterworth removed the merge me PR is reviewed. Merge when all tests are passing label Jun 23, 2024
@Keno
Copy link
Member

Keno commented Jun 23, 2024

I haven't fully bisected, but it seems like this is causing the following:

ERROR: InitError: KeyError: key Base.PkgId(Base.UUID("872c559c-99b0-510c-b3b7-b6c96a88d5cd"), "NNlib") not found
Stacktrace:
  [1] getindex
    @ ./dict.jl:477 [inlined]
  [2] macro expansion
    @ ./lock.jl:273 [inlined]
  [3] root_module(key::Base.PkgId)
    @ Base ./loading.jl:2358
  [4] parse_pkg_files(id::Base.PkgId)
    @ Revise ~/.julia/dev/Revise/src/loading.jl:38
  [5] watch_package(id::Base.PkgId)
    @ Revise ~/.julia/dev/Revise/src/pkgs.jl:349
  [6] add_require(sourcefile::String, modcaller::Module, idmod::String, modname::String, expr::Expr)
    @ Revise ~/.julia/dev/Revise/src/pkgs.jl:188
  [7] top-level scope
    @ ~/.julia/packages/Requires/Z8rfN/src/require.jl:70
  [8] (::NNlib.var"#599#603")()
    @ NNlib ~/.julia/packages/Requires/Z8rfN/src/require.jl:106
  [9] top-level scope
    @ ~/.julia/packages/Requires/Z8rfN/src/require.jl:20
 [10] macro expansion
    @ ~/.julia/packages/Requires/Z8rfN/src/require.jl:98 [inlined]
 [11] (::NNlib.var"#598#602")()
    @ NNlib ~/.julia/packages/Requires/Z8rfN/src/init.jl:11
 [12] __init__()
    @ NNlib ~/.julia/packages/Requires/Z8rfN/src/init.jl:18
 [13] run_module_init(mod::Module, i::Int64)
    @ Base ./loading.jl:1305
 [14] register_restored_modules(sv::Core.SimpleVector, pkg::Base.PkgId, path::String)
    @ Base ./loading.jl:1293

@KristofferC
Copy link
Member

KristofferC commented Jun 23, 2024

Should we use #53849 for this then? I guess it was designed for cases like this?

@Keno
Copy link
Member

Keno commented Jun 23, 2024

Should we use #53849 for this then?

That helps for pre-merge versioning. Now this can just use a regular VERSION check if intended or a fix if not.

@KristofferC
Copy link
Member

So this PR should have added an entry?

@Keno
Copy link
Member

Keno commented Jun 23, 2024

If the break here was intentional, then what should have happened would be:

  1. This PR adds an entry
  2. We make the package work both ways by querying the entry
  3. We merge/tag the package
  4. We merge the PR here

@KristofferC
Copy link
Member

KristofferC commented Jun 23, 2024

If you remove the internal/public barrier then any change is breaking. But so far it seems no new entry has been added so I guess no internals have been broken somehow? But Revise doesn't work now. So what should be changed in the workflow to make this thing actually do something?

vtjnash added a commit that referenced this pull request Jun 24, 2024
This fixes a couple unconventional issues people encountered and were
able to report as bugs against #54739

Note that due to several bugs in REPLExt itself
(#54889,
#54888), loading the extension
may still crash julia in some circumstances, but that is now a Pkg bug,
and no longer the fault of the loading code.
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
Copy link
Member

I am worried backporting this will cause #54940 on 1.11 but will have to check.

KristofferC pushed a commit that referenced this pull request Jul 23, 2024
This now allows the user to load any number of copies of a module, and
uses the combination of the environment, explicitly loaded modules, and
the requirements of the precompile caches to determine the meaning of a
name and which files need to be loaded. Note however that package
extensions continue to primarily only apply to the explicitly loaded
modules, although they may get loaded incidentally as the dependency of
another package, they won't get defined for every pair of combinations
of triggering modules.

Fixes #53983

(cherry picked from commit a1a2ac6)
KristofferC pushed a commit that referenced this pull request Jul 23, 2024
This fixes a couple unconventional issues people encountered and were
able to report as bugs against #54739

Note that due to several bugs in REPLExt itself
(#54889,
#54888), loading the extension
may still crash julia in some circumstances, but that is now a Pkg bug,
and no longer the fault of the loading code.

(cherry picked from commit a7fa1e7)
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
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1.11.0-alpha2: precompilation fails if JULIA_DEPOT_PATH is set
6 participants