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

gmsh (gmsh_jll) -> wrong gmsh.dll path in artifacs folder for windows? #2502

Closed
c-schubert opened this issue Feb 7, 2021 · 17 comments
Closed

Comments

@c-schubert
Copy link

Hi,
sorry that I cannot exactly name the problems cause (I do not know BinaryBuilder very well and the whole build process seems to me to have a lot of dependencies, so that I find it difficult to follow it at least without some effort) , but I will do my best to describe the problem and I hope there are some people who can figure out the cause very quickly :)

In the artifact folder of the gmsh_jll.jl package, which is used i.e. by GridapGmsh the gmsh.dll is placed inside the "bin" folder of the gmsh_jll artifacts folder, but the gmsh.initialize() function of the gmsh.jl (which is placed in the lib folder of the gmsh_jll artifacts folder) is looking in the same folder (lib folder) for the gmsh.dll, but as said its located inside the bin folder (which should be wrong, i guess).
My guess it that it has something to do with the generated code of the windows built wrapper files (x86_64-w64-mingw32-cxx03.jl, x86_64-w64-mingw32-cxx11.jl) in https://github.com/JuliaBinaryWrappers/gmsh_jll.jl/tree/main/src/wrappers, because if I look at the build log of the windows binaries (i.e. https://github.com/JuliaBinaryWrappers/gmsh_jll.jl/releases/download/gmsh-v4.7.1+0/gmsh.v4.7.1.x86_64-w64-mingw32-cxx11.tar.gz) the gmsh.dll seems to be installed in the "right" (lib) directory.

I hope someone can help me with this, thanks in advance :)

@giordano
Copy link
Member

giordano commented Feb 7, 2021

It is correct that the shared library for Windows is under bin/. Why don't you use gmsh_jll.libgmsh_path instead of guessing its path? 🙂

@c-schubert
Copy link
Author

Hi, thanks for the quick answer 😊. Maybe it is correct (as in this is a standard? 🤷‍♂️), but as I mentioned, if you look at the gmsh.jl under the lib folder it looks for the gmsh.dll at the same directory in the initialize function.

const libdir = dirname(@__FILE__)
const libname = Sys.iswindows() ? "gmsh-4.8.dll" : "libgmsh"
import Libdl
const lib = Libdl.find_library([libname], [libdir])

and if you call the initialize function (gmsh.initialize()) (which you probably need to use the julia gmsh functions?) it will fail ...

Also if I download the SDK from the website the gmsh.dll is located in lib, so i suspect it is should really be there ?

@giordano
Copy link
Member

giordano commented Feb 7, 2021

Maybe it is correct (as in this is a standard? 🤷‍♂️)

Yes, libraries on Windows are installed in directories in the PATH, because Windows doesn't distinguish between PATH and something like LD_LIBRARY_PATH, so we must put everything in there.

if you look at the gmsh.jl under the lib folder it looks for the gmsh.dll at the same directory in the initialize function.

I think I miss the point of this file when you use the JLL package, since the JLL packages already does this for you: when you do

using gmsh_jll

the library is already automatically dlopened for you, on all operating systems

@c-schubert
Copy link
Author

c-schubert commented Feb 7, 2021

Ok, So I really should make an issue in the gmsh repo 🤔?
But I just realized another issue, the version number of the gmsh.dll, which is explicitly mentioned in gmsh.jl (with gmsh-4.8.dll) is lost within the gmsh_jll (Yggdrasil) build ...

Or is your point that I do (should) not need (use) the functionality of the gmsh.jl? Sorry if I do not understand it right now 😅 (I am not a package developer, I just wanted to make GridapGmsh work on windows ...) ?

@giordano
Copy link
Member

giordano commented Feb 7, 2021

Ok, So I really should make an issue in the gmsh repo thinking?

We could move the Julia script to bin/ as well, but again, once you're using the gmsh_jll package that script is useless.

But I just realized another issue, the version number of the gmsh.dll, which is explicitly mentioned in gmsh.jl (with gmsh-4.8.dll) is lost within the gmsh_jll (Yggdrasil) build ...

This is the build script:

cd ${WORKSPACE}/srcdir/gmsh
install_license LICENSE.txt
mkdir build
cd build
cmake -DCMAKE_INSTALL_PREFIX=${prefix} \
-DCMAKE_TOOLCHAIN_FILE=${CMAKE_TARGET_TOOLCHAIN} \
-DCMAKE_BUILD_TYPE=Release \
-DENABLE_BUILD_DYNAMIC=1 \
..
make -j${nproc}
make install

We don't do anything special, you should ask them why the version number isn't there when we build from source.

@c-schubert
Copy link
Author

I guess moving the gmsh.jl will make unix system fail with the gmsh.jl ... so that won't be a good option.

To conclude I think I will have to make some local tests and file an issue to the gmsh git, to maybe fix the gmsh.jl to look for the dll inside the bin folder and fix the version number thingy ... ? Than this issue could be closed at this place.

PS: Is there an easy way to get all parameter of build_tarballs(ARGS, name, version, sources, script, platforms, products, dependencies, preferred_gcc_version=v"7") to better reproduce the build process locally?

@giordano
Copy link
Member

giordano commented Feb 7, 2021

I guess moving the gmsh.jl will make unix system fail with the gmsh.jl ... so that won't be a good option.

I'd move it only for Windows of course. But once more, I think it's more reasonable to not use that script at all when using the JLL package: it's more reasonable, not redundant, it avoids the users downloading a new tarballs which if functionally identical to the previous one (and users always complain about the size of the artifacts directory).

PS: Is there an easy way to get all parameter of build_tarballs(ARGS, name, version, sources, script, platforms, products, dependencies, preferred_gcc_version=v"7") to better reproduce the build process locally?

What parameters? All variables are defined in the script

@c-schubert
Copy link
Author

I guess moving the gmsh.jl will make unix system fail with the gmsh.jl ... so that won't be a good option.

I'd move it only for Windows of course. But once more, I think it's more reasonable to not use that script at all when using the JLL package: it's more reasonable, not redundant, it avoids the users downloading a new tarballs which if functionally identical to the previous one (and users always complain about the size of the artifacts directory).

Ok, sorry if I'am just stupid here, but the gmsh.jl (I talk about) is included in the tarball of the gmsh_jll and is only a wrapper for the gmsh*.dll library functions and does not lead to any additional downloads (at least I do not find any download targets in that file or am I just wrong)? - Also if I understand you right the using gmsh_jll the gmsh.dll would be dlopened for me, but I would have to use ccall the library functions for using it? or am I wrong there either? The GridappGmsh.jl package seems to rely on the gmsh.jl functions, which does nothing more than exposing the ccall to julia functions, as I would say on a first glance.

PS: Is there an easy way to get all parameter of build_tarballs(ARGS, name, version, sources, script, platforms, products, dependencies, preferred_gcc_version=v"7") to better reproduce the build process locally?

What parameters? All variables are defined in the script

Ok, I was not sure if i.e. $prefix or ARGS may receive something from functions calling the build_tarballs.jl ...

Thank you very much for the help btw. 🙂

@giordano
Copy link
Member

giordano commented Feb 7, 2021

Ok, I think I may need some more context. Where is this script called?

@c-schubert
Copy link
Author

c-schubert commented Feb 7, 2021

It's basically called here:

https://github.com/gridap/GridapGmsh.jl/blob/7d64a9e9998a653a1c37ffc4aa3816b85b60f7c1/src/GridapGmsh.jl#L29

here gmsh_jl variable is defined at build time under 'deps/deps.jl' which in my case contains (after I somehow fixed the windows build gridap/GridapGmsh.jl#37):

const GMSH_FOUND = true
const gmsh_jl = "C:\\Users\\Schubert\\.julia\\artifacts\\4216fe50dba72ebe84b758bf98073f269af20f94\\lib\\gmsh.jl"

And then the package basically uses the exposed functionality of gmsh via gmsh.somefunction() etc...

PS: I have to go now so my next respond will probably be 2-3 h later ...

@giordano
Copy link
Member

giordano commented Feb 7, 2021

At https://github.com/gridap/GridapGmsh.jl/blob/7d64a9e9998a653a1c37ffc4aa3816b85b60f7c1/deps/build.jl#L9 the library is already dlopened and the variable to use for calling into the library is libgmsh, all the rest is completely useless when you use the JLL package. I believe this script should simply be reorganised in a better way. For example, in that else branch you can simply do write("deps.jl", "using gmsh_jll") and you're completely done.

@c-schubert
Copy link
Author

Ok , I get your point, but I believe the way the gmsh_jll is used here is kind of unusual, because also the fact that some library itself has a julia function wrapper module included is kind of unusual itself, I guess.
And what is done here is nothing more than using this julia module delivered with the source code of gmsh (even if there may be some possible comprehensions to be done in the source code)
But why do you think that is so bad, why writing all ccalls() yourself, if there are already some nice interfaces written for you included in gmsh(.jl) module that comes included together with the build files?
I get that this is not the usual procedure for _jll packages, but I also think usually there is no wrapper module for julia included within the library itself...

@c-schubert
Copy link
Author

Hi again,
the comments from my previous post aside, I have done some additional reading and I think I now better understand some of your considerations and and have summarized my thoughts in a few statements, but please correct me if I am wrong 🙂:

  1. For an .exe in Windows compiled with a shared library the .dll files must be indexed by PATH variable or be located in the same folder as the .exe.

  2. So letting the gmsh.dll reside inside the lib folder would be a bad decision for the gmsh_jll build, since it would break the functionality of the gmsh.exe

  3. Possible best way for everything to work fine, would be to convince the gmsh community to place the gmsh-*.dll inside the bin folder (as this seems to lead to some confusion anyhow https://gitlab.onelab.info/gmsh/gmsh/-/issues/1173) and to modify the gmsh.jl module file to look for the dll inside the bin folder in case of windows systems.

  4. Copying the gmsh.jl file to the bin folder specifically for windows only should also do the trick, as you mentioned above, and would be my favorite option atm. Since I do not know how hart it would be for me to convince anyone in gmsh community that the above mentioned fixes will not break anything for someone, while with the gmsh_jll that should no affect anyone (which has not already done some manually coping or modification of the gmsh.jl file) ...

@giordano
Copy link
Member

giordano commented Feb 7, 2021

I guess I was again missing some context. I thought the gmsh.jl script bundled (why?) in the tarball was only to load the library, that's why I was insisting on simply using using gmsh_jll to load the binary library.

Again, we could move the script to the bin/ directory for Windows, but then the binary has the wrong name. I have no idea why they generate a file with a name different from what they expect 🤷

@giordano
Copy link
Member

giordano commented Feb 7, 2021

Oh, we posted the message around the same time

  1. yes
  2. exactly
  3. this isn't entirely necessary, we can move the julia script ourselves
  4. but the name of the file library is gsmh.dll, which is different from what the the gmsh.jl script expects. Frankly, I believe that script shouldn't attempt to dlopen anything. Honestly, I'm not even sure why it isn't simply a separate normal Julia package

@c-schubert
Copy link
Author

Oh yes you are right 🤦‍♂️. I thought it worked, because it works with the downloaded SDK from their website, but the gmsh.dll has a version there ... Wonder if they do the numbering manually for their releases ... Probably the gold way would be to make the gmsh.jl a separate package to expose the functionality of the gmsh library on a higher level, which then uses/relies on the gmsh_jll build. I have to think about this, but I guess you are right, you can do nothing to make everything work together right now... Guess we than can close this issue for now, thank you for the help I learned a lot 😅🙂.

@giordano
Copy link
Member

giordano commented Feb 7, 2021

We can't even simply rename gmsh.dll to gmsh-4.8.dll, because the program gmsh.exe links to gmsh.dll and changing the link is less trivial than renaming a file. There is something inconsistent in their build. Ideally, we shouldn't do anything to fix this stuff, the build system should take care of everything.

Guess we than can close this issue for now

👍

@giordano giordano closed this as completed Feb 7, 2021
c-schubert added a commit to c-schubert/GridapGmsh.jl that referenced this issue Feb 8, 2021
Add some temporary note, about the installation of the gmsh dependency under Windows, regarding this discussing: JuliaPackaging/Yggdrasil#2502

This commit can be undone if the underlying issues will be fixed.
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

No branches or pull requests

2 participants