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

disable autoprecompile by default #26

Merged

Conversation

IanButterworth
Copy link
Member

With native code caching incoming, doing (auto)precompilation at this stage of CI is suboptimal because the native caches will be generated for a julia setup that doesn't match the test julia setup i.e. bounds checking. which would cause precompilation in this step and in a following julia-runtest step.

This disables auto-precompilation here, but exposes it as an option to re-enable.

My hope here is that we can release this as a non-breaking change so we can avoid a load more unnecessary precompilation once native caching lands.

@codecov-commenter
Copy link

Codecov Report

Base: 100.00% // Head: 100.00% // No change to project coverage 👍

Coverage data is based on head (64b7e04) compared to base (f995fa4).
Patch has no changes to coverable lines.

Additional details and impacted files
@@            Coverage Diff            @@
##            master       #26   +/-   ##
=========================================
  Coverage   100.00%   100.00%           
=========================================
  Files            1         1           
  Lines            2         2           
=========================================
  Hits             2         2           

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

☔ View full report at Codecov.
📢 Do you have feedback about the report comment? Let us know in this issue.

@IanButterworth
Copy link
Member Author

@SaschaMann if this requires your sign off, the thread here has more context JuliaLang/Pkg.jl#3281

@SaschaMann
Copy link
Member

My hope here is that we can release this as a non-breaking change so we can avoid a load more unnecessary precompilation once native caching lands.

Do you happen to know a scenario where this could be breaking?

I'm fine with merging it as it is though.

@IanButterworth
Copy link
Member Author

I don't think so. This step promises to build the package, the fact it autoprecompiled wasn't really within the spec I think

@SaschaMann SaschaMann merged commit 139ec78 into julia-actions:master Dec 17, 2022
@IanButterworth IanButterworth deleted the ib/disable_autoprecompile branch December 17, 2022 17:47
@kleinschmidt
Copy link

FWIW this did actually cause breakage in some internal code for us but in a VERY specific way: during CI, a different project is instantiated (with test dependencies), and that gets precompiled during Pkg.test(). But we have some code that uses Distributed.LocalManager to add additional workers with addprocs(; exeflags="--project") which in production is The Right Thing to do but during tests results in a precompilation race condition which causes mysterious segfaults/compilecache misses in dependencies; before, the "extra" precompilation that julia-buildpkg was doing meant that the project was appropriately precompiled and there was no race condition. The fix was to just activate the actual project that is active on teh manager when addproces is called, instead of relying on exeflags="--project". Again, I don't think this is really the FAULT of this change, but wanted to raise it just in case it's worth knowing for anyone else :)

@kleinschmidt
Copy link

All of which is to say https://xkcd.com/1172/

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.

4 participants