JLD2 extension take 2

[christiangnrd]

@lassepe I accidentally messed up your PR (#596) so I'm reopening this. I'll try to make it pass tests but if you get it fully working before then feel free to reopen and I'll close this one.

I apologize for the inconvenience.

[christiangnrd]

I got the tests to pass. However, I'm not sure this is the best way to deal with the new JLD2 requirement. Do we add a JLD2 dependency directly to this package's setup file so that the downstream packages require no changes, or do we add JLD2 to every downstream package's test Project.toml?

Also, @maleadt I removed the TestEnv stuff since TestEnv.activate() was causing issues, and it was actually calling Pkg.activate(), so it wasn't actually being used. That may not be what you actually want.

[maleadt]

Removing TestEnv is fine if the alternative works.

Do we add a JLD2 dependency directly to this package's setup file so that the downstream packages require no changes, or do we add JLD2 to every downstream package's test Project.toml?

In the past, we've opted for adding them to the test Project.toml, as running Pkg operations (mutating the active environment) during testing is not great, and may affect versions of other packages installed. It's an annoying way to handle this, but I don't think we can avoid it.

[lassepe]

@christiangnrd, can you elaborate on why it is even necessary to add JLD2 manually via Pkg.add? My understanding would be that if JLD2 is part of the test/Project.toml then it should be automatically available during Pkg.test?

Edit: I see now what is going on: the buildkite pipeline runs the tests of the downstream page (e.g. CUDA) and that package itself ends up including GPUArrays.jl:test/testsuite.jl

https://github.com/JuliaGPU/CUDA.jl/blob/8b6a2a06c8f9d247081d7c92d8f8a82ab68153d5/test/setup.jl#L12

which then triggers the "missing JLD2` dependency error.

I guess without fundamentally reworking the test infrastructure (e.g. by pulling out the testsuit stuff into its own package), merging dependencies manually (as @christiangnrd ) is unavoidable. Perhaps a minor improvement over the current approach: rather than manually adding only JLD2, I would suggest to do something like:

for (uuid, pkg_info) in gpuarray_test_deps
           Pkg.add(PackageSpec(name=pkg_info.name, uuid=uuid, version=pkg_info.version))
end

[maleadt]

This is for reverse-CI, executing e.g. CUDA.jl's tests, which import GPUArrays.TestSuite. If that contains JLD2-dependent tests, the CUDA.jl test project also needs to depend on JLD2.

[lassepe]

You beat me to it! :) See my edited comment above ☝️

[christiangnrd]

I opened JuliaGPU/CUDA.jl#2792, JuliaGPU/OpenCL.jl#329, JuliaGPU/oneAPI.jl#507, and merged JuliaGPU/Metal.jl#606.

Once merged we can rerun tests and then probably good to go.

[lassepe]

Friendly bump before this goes stale, @christiangnrd :)

[christiangnrd]

@lassepe I restarted the failing CUDA tests hopefully the runtime downloads this time