-
Notifications
You must be signed in to change notification settings - Fork 13
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
Custom installations of Gmsh via Preferences.jl #31
Conversation
Codecov ReportAttention: Patch coverage is
❗ Your organization needs to install the Codecov GitHub app to enable full functionality. Additional details and impacted files@@ Coverage Diff @@
## master #31 +/- ##
===========================================
- Coverage 92.30% 29.62% -62.68%
===========================================
Files 1 1
Lines 13 54 +41
===========================================
+ Hits 12 16 +4
- Misses 1 38 +37 ☔ View full report in Codecov by Sentry. |
Is there a reason not to use the regular override mechanism (https://pkgdocs.julialang.org/v1/artifacts/#Overriding-artifact-locations)? |
Hi @fredrikekre IMHO, understanding how artifacts work and setting up an override looks way more complicated than reading the documentation of a function, calling the function, and restarting Julia (proposed solution). In addition, the Override.toml seems to affect all the packages in the current depot:
Is it possible to have project A using gmsh_jll and project B using a system installed gmsh? I would say that this is not possible with overrides, but it is definitively possible with preferences. |
Should this be merged? |
@@ -1,10 +1,141 @@ | |||
module Gmsh | |||
|
|||
import gmsh_jll | |||
include(gmsh_jll.gmsh_api) | |||
@static if VERSION >= v"1.6" |
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.
You can remove these version checks and set julia compat to 1.6 instead.
I think the Overrides.toml is pretty much deprecated. However, using preferences I think jlls already have support for this, see e.g. https://docs.binarybuilder.org/stable/jll/#Overriding-specific-products. I don't think bespoke code should be added to binary wrappers when there is a generic solution to do exactly this. If that is faulting for some reason it should be improved. |
This looks promising. Do you have a concrete example showing how this would apply to Gmsh.jl? |
Something like this perhaps (I haven't used it very much myself): julia> using gmsh_jll
julia> gmsh_jll.gmsh_path
"/home/kc/.julia/artifacts/65888a08d8ab9e3bf5e28f26eabd2731f2a95ff3/bin/gmsh"
julia> using Preferences
julia> set_preferences!(
"LocalPreferences.toml",
"gmsh_jll",
"libgmsh_path" => "/lib/x86_64-linux-gnu/libgmsh.so.4.8"
)
# restart julia
julia> using gmsh_jll
Precompiling gmsh_jll
1 dependency successfully precompiled in 1 seconds. 55 already precompiled.
ERROR: InitError: could not load library "/lib/x86_64-linux-gnu/libgmsh.so.4.8"
/lib/x86_64-linux-gnu/libgmsh.so.4.8: undefined symbol: _ZN24BRepBuilderAPI_MakeShape5BuildEv
Stacktrace:
[1] dlopen(s::String, flags::UInt32; throw_error::Bool)
@ Base.Libc.Libdl ./libdl.jl:117
[2] dlopen(s::String, flags::UInt32)
@ Base.Libc.Libdl ./libdl.jl:116 |
This works:
but it requires adding Libdl as a dependency here in Gmsh because the downloaded gmsh.jl file from the sdk uses it, but the file is patched in the gmsh_jll (https://github.com/JuliaPackaging/Yggdrasil/blob/3e5207cb20f6438ff1ce6e502680b8c3593e5020/G/gmsh/build_tarballs.jl#L31-L36) to comment this out. |
What about hiding julia> set_preferences!(
"LocalPreferences.toml",
"gmsh_jll",
"libgmsh_path" => abspath("gmsh-4.12.2-Linux64-sdk/lib/libgmsh.so"),
"gmsh_api_path" => abspath("gmsh-4.12.2-Linux64-sdk/lib/gmsh.jl"),
) in a function |
To me, that feels like it will be annoying to get correct on all systems. For example, the one in the PR doesn't find my libgmsh even though:
There might also be multiple ones?
Just feels easier to document:
|
Agreed! I would add: And also test a custom installation in the CI tests. |
Could always be added, but in some sense we are not really testing anything Gmsh-specific (Gmsh only knows about |
I close the PR as the solution below by @fredrikekre is good enough. @fredrikekre @KristofferC Thanks for the feedback!
|
I still think we need to add Libl as a dependency right? Would you make a PR? The CI test can also be kept I think, this repo is so low traffic anyway so no it isn't ao wasteful IMO. Perhaps also add this to the README? |
See PR #34 |
Hi there!
This PR allows users to use their custom installation of Gmsh. The new logic is implemented using Preferences.jl
Two new functions are added
Gmsh.use_system_gmsh
. Calling this function will configure Gmsh to use a system installation. It will look for an installation in the system (LD_LIBRARY_PATH
), or the user can provide the path to the installation. Changes will be effective in the next Julia session.Gmsh.use_gmsh_jll
. Calling this function will configure Gmsh to use the binary provided bygmsh_jll
(e.g., to revert a previous call toGmsh.use_system_gmsh
). Changes will be effective in the next Julia session.The CI is updated to test Gmsh.jl with a custom installation of Gmsh.