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

Adjust julia gc to jl_gc_new_weakref no longer being exported #5824

Closed

Conversation

lgoettgens
Copy link
Contributor

As discussed in oscar-system/GAP.jl#1032 (comment). cc @fingolfin

Text for release notes

not needed

@fingolfin
Copy link
Member

You'll need this at the top of the file:

#include <julia_threads.h>    // for jl_get_ptls_states

#if JULIA_VERSION_MAJOR == 1 && JULIA_VERSION_MINOR == 7
// workaround issue with Julia 1.7 headers which "forgot" to export this
// function
JL_DLLEXPORT void * jl_get_ptls_states(void);
#endif

@fingolfin
Copy link
Member

Still crashes :-(

@fingolfin
Copy link
Member

Still lots of compiler warninga in the bundled GAP. And it compiles GAP twice, gotta look into that

@fingolfin
Copy link
Member

We really should ask the Julia folks to add jl_gc_new_weakref back to headers given that jl_gc_new_weakref_th existed in older version but wasn't in the headers (and was it exported?)

@lgoettgens
Copy link
Contributor Author

Still lots of compiler warninga in the bundled GAP. And it compiles GAP twice, gotta look into that

The double compilation is due to the way that setup_override_dir.jl works. It expects gap to already be compiled the usual way, then replaces two files and re-compiles but this time with julia gc.

@lgoettgens
Copy link
Contributor Author

We really should ask the Julia folks to add jl_gc_new_weakref back to headers given that jl_gc_new_weakref_th existed in older version but wasn't in the headers (and was it exported?)

See JuliaLang/julia#56319

@fingolfin
Copy link
Member

Julia PR was merged so we can close this. Thanks for the effort, @lgoettgens !

@fingolfin fingolfin closed this Oct 24, 2024
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