move LinearAlgebra and Dates to package extensions

[oscardssmith]

This is a no-op for <1.9, but for 1.9 and above mean that you don't automatically bring in Dates and LinearAlgebra from bringing in Compat. This is mainly useful for people compiling custom sysimages.

[codecov]

Codecov Report

Merging #790 (f8f4fcb) into master (4d49a82) will not change coverage.
The diff coverage is n/a.

@@           Coverage Diff           @@
##           master     #790   +/-   ##
=======================================
  Coverage   91.92%   91.92%           
=======================================
  Files           2        2           
  Lines         260      260           
=======================================
  Hits          239      239           
  Misses         21       21           

Impacted Files	Coverage Δ
src/Compat.jl	92.46% <ø> (ø)

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

[martinholters]

I think this is fine for the Dates part, but the dependency on LinearAlgebra is to provide Compat.get_num_threads and Compat.set_num_threads (even if deprecated). It would be funny if these needed LinearAlgebra to be loaded to be present (though probably not too bad), but AFAIU, these would be hidden away in the CompatLinearAlgebraExt module?

[oscardssmith]

As I understand it, (and correct me if I'm wrong), on recent versions this functionality is LinearAlgebra.Blas.get_num_threads (or did I misread the PR?)

[martinholters]

As I understand it, (and correct me if I'm wrong), on recent versions this functionality is LinearAlgebra.Blas.get_num_threads (or did I misread the PR?)

Uh, yes, new code should be using LinearAlgebra.Blas.get_num_threads instead of Compat.get_num_threads() - that's why it's deprecated. But old code might still be using it, that's why we've kept it around.
With Compat master (on Julia master with depwarn=yes):

julia> using Compat

julia> Compat.get_num_threads()
WARNING: Compat.get_num_threads is deprecated, use LinearAlgebra.BLAS.get_num_threads instead.
  likely near REPL[1]:1
2

With this PR:

julia> using Compat, LinearAlgebra

julia> Compat.get_num_threads()
ERROR: UndefVarError: `get_num_threads` not defined

For that effect, we could just remove the LinearAlgebra stuff and avoid the extension - but it would be a breaking change and hence would require a major version bump.

[oscardssmith]

hmm that's supposed to work. I think I have a typo

[martinholters]

Thinking about this some more, CompatDatesExt seems rather pointless. It only includes code that is conditional on Julia versions prior to 1.9, so as a package extension, it's basically just empty. Couldn't we achieve the same by just making Dates a weakdep and only importing it for julia versions where it is needed (i.e. before 1.8.0-DEV.1109)?

How is the get_num_threads stuff supposed to work? Wouldn't that require CompatLinearAlgebraExt to eval code into Compat? EDIT: Which I do not believe to be a good idea.

[martinholters]

Even if you properly bind Compat before, this fails with

ERROR: LoadError: cannot assign variables in other modules

[martinholters]

I was considering something like

Compat.eval(quote
Base.@deprecate_binding set_num_threads $LinearAlgebra.BLAS.set_num_threads false
Base.@deprecate_binding get_num_threads $LinearAlgebra.BLAS.get_num_threads false
end)

but of course, that yields

ERROR: LoadError: Evaluation into the closed module `Compat` breaks incremental compilation because the side effects will not be permanent. This is likely due to some other module mutating `Compat` with `eval` during precompilation - don't do this.

Maybe we declare the functions in Compat without body and then add a method to them in CompatLinearAlgebraExt doing the deprecation?

[martinholters]

Actually, it looks to me as though package extensions don't work with stdlibs - I don't think CompatLinearAlgebraExt gets loaded for me at all. Shouldn't that load immediately after Compat as LinearAlgebra is already loaded as part of the sysimg? @KristofferC can you comment on that?

[KristofferC]

Actually, it looks to me as though package extensions don't work with stdlibs - I don't think CompatLinearAlgebraExt gets loaded for me at all. Shouldn't that load immediately after Compat as LinearAlgebra is already loaded as part of the sysimg?

It should yes. Could be a bug.

[martinholters]

Ah, I was confused because at the REPL, using LinearAlgebra works even if LinearAlgebra is not declared as a dependency, but the package extension is not loaded then - that requires LinearAlgebra to be declared as a dependency. Makes sense, I guess, but is not too intuitive.

[martinholters]

I've taken the liberty to

• merge master and resolve conflicts
• remove the Dates stuff (done in master by Make Dates a weakdep #791)
• rework the LinearAlgebra stuff as proposed above

The behavior now is slightly different depending on whether you use Julia version that support package extensions or not.
Julia 1.8:

julia> Compat.get_num_threads
WARNING: Compat.get_num_threads is deprecated, use LinearAlgebra.BLAS.get_num_threads instead.
  likely near REPL[2]:1
get_num_threads (generic function with 1 method)

julia> Compat.get_num_threads()
WARNING: Compat.get_num_threads is deprecated, use LinearAlgebra.BLAS.get_num_threads instead.
  likely near REPL[3]:1
2

Julia 1.10:

julia> Compat.get_num_threads
get_num_threads (generic function with 1 method)

julia> Compat.get_num_threads()
┌ Warning: `Compat.get_num_threads` is deprecated, use `LinearAlgebra.BLAS.get_num_threads` instead.
│   caller = top-level scope at REPL[6]:1
└ @ Core REPL[6]:1
2

They both give a deprecation warning when calling the deprecated functions, but on older Julia, the binding itself is deprecated. I don't think anyone will notice, so this should be ok.

[martinholters]

Oh, and with the new approach, in an environment without LinearAlgebra in its dependencies on Julia supporting package extensions:

julia> Compat.get_num_threads
get_num_threads (generic function with 0 methods)

julia> Compat.get_num_threads()
ERROR: MethodError: no method matching get_num_threads()

So this might technically be considered breaking, but who would be calling those methods if not wanting to use LinearAlgebra.BLAS in some way?

[martinholters]

Would be nice to have another pair of eyes on this one...

[martinholters]

Lacking further feedback, I think I'll merge this soonish and tag a new patch-version to see whether anyone comes complaining afterwards. Objections?