Skip to content

correct Requires usage to fix issues with PackageCompiler sysimages#387

Merged
ChrisRackauckas merged 4 commits intoJuliaArrays:masterfrom
KristofferC:kc/requires
Mar 6, 2023
Merged

correct Requires usage to fix issues with PackageCompiler sysimages#387
ChrisRackauckas merged 4 commits intoJuliaArrays:masterfrom
KristofferC:kc/requires

Conversation

@KristofferC
Copy link

Basically, the rule for Requires to work with sysimages is that it should load no package non-relatively.

Copy link
Member

@ChrisRackauckas ChrisRackauckas left a comment

Choose a reason for hiding this comment

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

Thanks!

@codecov
Copy link

codecov bot commented Mar 6, 2023

Codecov Report

Merging #387 (9bbde83) into master (0f7691c) will not change coverage.
The diff coverage is n/a.

@@           Coverage Diff           @@
##           master     #387   +/-   ##
=======================================
  Coverage   79.25%   79.25%           
=======================================
  Files           7        7           
  Lines         535      535           
=======================================
  Hits          424      424           
  Misses        111      111           
Impacted Files Coverage Δ
ext/ArrayInterfaceBandedMatricesExt.jl 85.36% <ø> (ø)
ext/ArrayInterfaceBlockBandedMatricesExt.jl 82.89% <ø> (ø)
ext/ArrayInterfaceCUDAExt.jl 0.00% <ø> (ø)
ext/ArrayInterfaceGPUArraysCoreExt.jl 0.00% <ø> (ø)
ext/ArrayInterfaceStaticArraysCoreExt.jl 93.33% <ø> (ø)
ext/ArrayInterfaceTrackerExt.jl 40.00% <ø> (ø)

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

@KristofferC
Copy link
Author

So Adapt is not loaded in ArrayInterface which means the using ..Adapt for Requires doesn't work. I don't see a way to be relocatable for Requries with that setup. I put it back how things were for that specific extension though.

@ChrisRackauckas
Copy link
Member

Adapt's stuff got upstreamed to Adapt.jl so that's fine.

@KristofferC
Copy link
Author

But there is a function below using it?

@ChrisRackauckas
Copy link
Member

🤦

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.

2 participants