-
Notifications
You must be signed in to change notification settings - Fork 28
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
Executing commands at top level #1049
Comments
I fear this is not going to be easy. Any way that I can test that PkgEval will accept a future solution for this? |
Yes, PkgEval is easy to use, but the issue caused by the toplevel |
Hmm, upon closer inspection I think the PkgEval incompatibility is caused by a different issue. I definitely saw it crash once because of the top-level command, but that does not seem to be the reason for the consistent failures to test GMT/SeisPDF. So feel free to close this, although I would definitely recommend replacing the global commands by something more reliable/safe. |
Trying to address the Why is it hard to restructure how the library is determined? Honestly I don't remember how I ended up with this solution but I do know why. The big main issue is that I can't build GMT with the BinaryBuilder. Why? Because it forces that everything is buildable from Unix in a cross compiling way and GMT does not build with MinGW. Along the time (GMT is > 30 years old) we learned how to build GMT on Windows using the Microsoft compiler. We had to learn about all the cross dlls shits, on how text files mus be open in text mode and so on. And once we finally did it there was no more motivation to invest in MinGW/Cygwin, all with their own head-aches. So I can't build GMT with MinGW and I'm not interested either in a BinaryBuilder package that would use a GDAL that is not able to build with NetCDF/HDF libraries. Also here the problem seems to lie on trying to force a unix way only and Windows part not being satisfied. At the end, this harms both sides of this story. Not having a GMT_JLL what I do is:
I'll scratch my head again trying to come out with something more robust to store/find the names of those shared libs. You mention also a crash in the tests. Unfortunately I suspect what it was but I've tried a lot and can't find why there a random crash apparently caused by the module |
I'm not arguing for a JLL -- that would solve the problem too, but it's understandably more invasive -- but just for the fact that the whole discovery/building process ideally doesn't happen at toplevel, but either during But again, I think that the issue I was seeing on PkgEval was related to something else, so just treat the above as best practices. |
Thanks for the advises, I'll give it a try when some more urgent issues are finished. |
FYI, ran into this again in the context of another package, AnyMOD.jl. The problem that's common between that package and GMT.jl is the global invocation of Conda. |
Is it by any chance related to this. In particular an error like (even if caused by other shared libs)?
I thought the |
No, I encountered that error too, but only with Julia 1.8. Running under 1.9 (specifically, the latest nightly, but we should have an alpha soon) worked fine. |
That error wouldn't bother me in particular if it wasn't for the situation that it's stopping me to automatically register new versions since last July. |
Done in #1077 |
Thanks. Could you give another ping when this enters a release? I'll remove the PkgEval blacklist then. |
It's now in v0.44.0 |
GMT.jl currently executes a command (
gmt --show-library
) at top level:GMT.jl/src/GMT.jl
Line 63 in 466ca0d
This is strongly discouraged. Instead, you should use a
deps/build.jl
to perform discovery at package installation time, or during__init__
, but not during precompilation. Specifically, this is incompatible with PkgEval, so we can't currently include this package (and dependents like SeisPDF.jl) in our daily testing of the upcoming Julia version.The text was updated successfully, but these errors were encountered: