-
Notifications
You must be signed in to change notification settings - Fork 22
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
Use Scratch.jl for GAP root #825
Conversation
This introduces a minor regression which I have not yet understood: upon |
Codecov Report
Additional details and impacted files@@ Coverage Diff @@
## master #825 +/- ##
==========================================
+ Coverage 78.05% 81.54% +3.49%
==========================================
Files 48 48
Lines 3686 3680 -6
==========================================
+ Hits 2877 3001 +124
+ Misses 809 679 -130
|
@fingolfin I cannot reproduce the problem with the GAP banner. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good. Thanks.
@ThomasBreuer which Julia version are you using? For me the problem of the missing banner only occurs in Julia 1.8 -- and indeed, the hack we use (I think perhaps invented by @thofma ?) checks for a function |
Yes, this was my doing. I can have a look. |
I would suggest you merge this here and I will fix it in a followup PR. |
This also forces a full regeneration of our custom GAP root dir each time the package loads, instead of only during precompilation. This adds ~130 milliseconds startup overhead for me, of which 90% are spent in finding working C/C++ compilers... Not great, but with Scratch.jl any of the directories could move, so we kinda have to do it this way, as far as I can tell (Polymake.jl also does it this way). Perhaps I could at least cache the C/C++ compiler values somehow, but for now I just want to have this working.
Resolves #822
Resolves #728