-
Notifications
You must be signed in to change notification settings - Fork 772
{cae}[foss/2019b] OpenFOAM 7 #9390
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
{cae}[foss/2019b] OpenFOAM 7 #9390
Conversation
|
Test report by @boegel |
|
Test report by @lexming |
|
My failed test seems to be related to https://bugs.openfoam.org/view.php?id=3303 and the changes in OpenFOAM/OpenFOAM-7@9980357 |
|
@lexming it seems that OF7 already includes changes related to OF bug no. 3301. Nevertheless, I inclded also a patch related to OF bug 3303 (bashrc, wclean, wrmdep). |
|
Test report by @boegel |
|
@lexming Can you also re-test this PR? |
|
Test report by @boegel |
|
@furstj Any idea on this? Looks related to |
|
Test report by @boegel |
|
@boegel I have no idea why it failed due to GL/gl.h. It compiles on my system without problems and you were able to compile it to in your last test. Interesting is that Qt or ParaView compile well (and both use OpenGL too). Do you have any idea wht is the difference between your systems or build environments at node3169.skitty.os and generoso-2? My latest version uses several patches for OpenFOAM build system (wmake) which are connected to problems with compiling in directories with symlinks. Do you mean that there is any difference between generoso-2 and node3169 related to symlinks? |
|
@furstj yep, I also saw that some patches for OF bug 3303 are already merged in version 7 and it seems that the expansion of My changes are in lexming/easybuild-easyblocks@a6292b3 I would say that maintaining the code in the easyblock might be easier than having an additional patch, but honestly, I'm not familiar with the development of OpenFOAM, so I'm not sure which solution is best moving forward. Let me know what you think. I'll only open a PR in |
|
Test report by @lexming |
|
The previous test report is done with this PR for |
|
@furstj On all systems I tested except |
|
@furstj Can you open an issue on that problem? I don't think it should block this PR (since it's not specific to this particular OpenFOAM version, I think), but it would be nice to get it fixed (and not having an open issue for it means we'll forget about it). |
|
Why is OpenFOAM pulling in MESA in the first place? Did they add a GUI? |
|
@akesandgren As a dep for |
|
@boegel I will try to compile this EB on a machine without mesa-devel package and we will see. @akesandgren OF compiles also some plugins to ParaView (e.g. specific openfoam readers). I guess that it needs whole ParaView stack in order to compile these plugins (i.e. ParaView, Qt, Mesa, ...) @lexming I agree with you that corrected easyblock is much better solution than a mega-patch. However I think that the problem is in OpenFOAM build scripts (I mean in wmake) and it will hurt also all non-easybuild OpenFoamers. Moreover, patches for OpenFOAM 3303 bug are already included in OpenFOAM-dev version, so the current EB patch for 3303 is only temporary solution. I hope that the OpenFOAM-8 (when it will be released) will solve the problem. |
|
@furstj if this patchset is already part of OpenFOAM-dev, then having it included in OF7 is indeed the best solution. |
|
@boegel I tried a compilation on a system without mesa-devel and it failed with missing GL/gl.h too. After some analysis I found that the problem is quite complicated and it probably goes through Qt5 down to pkg-config. The "pkg-config --cflags gl" returns wrong include path! It then goes to Qt5 (it can be found in Qt5GuiConfigExtras.cmake) and finally breaks the OpenFOAM compilation. It is very interesting that the pkg-config from OpenSUSE Leap distribution reports also wrong path with libdrm and the Qt5GuiConfigExtras.cmake from standard distribution contains the wrong path too. There are several possibilities for dealing with this issue:
Under my opinion the least distracting is the option 1. It is sufficient to recompile Qt5 and that's all. |
|
Note that pkg-config --cflags gl returns the wrong include path in Ubuntu too, so this is not a EasyBuild specific problem. |
|
This looks more like a bug in how pkg-config works to me. |
|
The gl.pc pkgconfig file (from both Ubuntu and EasyBuild) is technically correct. But pkg-config is not returning what you'd expect. Try And look at what it really does... |
|
Actually, this is a bug in pkg-config 0.29.2... I'll try to chase it down |
|
I'll have to do this in our cleanroom setup to double check. |
|
Hmmm, my cleanroom install do fail on GL/gl.h |
|
@akesandgren Yes, exactly what happens to me and boegel. It fails when there is no /usr/include/GL/gl.h. I did try also OpenFOAM-v1906 and it compiles fine. Therefore it should be possible to patch OpenFOAM-7 too. The question is then if I should modify OpenFOAM-7 easyconfig letting Qt intact, or if (me someone else?) will modify Qt in order to replace librm path by path to mesa. Both options are feasible. What is your opinion? |
|
It's still not clear to me where the bug/breakage is actually taking place... Is the Also note that |
|
Something in either Qt5 or OpenFOAM has to be setting some variable that forces find_path to NOT look in CMAKE_INCLUDE_DIRS, or unsetting CMAKE_INCLUDE_DIRS. |
|
I'll be chasing this down (I hope) so hang in there... |
|
I'm able to patch OF7 compilation now. If we replace cmake ../.. by cmake -D_qt5gui_OPENGL_INCLUDE_DIR=$EBROOTMESA/include ../.. in applications/utilities/postProcessing/graphics/PVReaders/Allwmake, the OF compiles. |
|
Yeah, the reason it doesn't work as we expected it to, is that OpenFOAM is not using the cmakemake easyblock. That's where we are setting CMAKE_INCLUDE_PATH=paths(CPPFLAGS)+CPATH. So there is one fairly simple way out and that is to set CMAKE_INCLUDE_PATH the same way we do in cmakemake.py in the OpenFOAM easyblock. Or we could let openfoam.py inherit cmakemake. |
|
I have a potential fix involving cmakemake.py and openfoam.py that fixes this in a slightly more generic way. |
|
Test report by @boegel |
|
Test report on |
|
with or without the patch to work around the GL/gl.h problem? I.e. with or without 0efd968 |
Test report was with the last commit included (so not really testing easybuilders/easybuild-easyblocks#1869 I guess). But we get another opportunity for that in #9440... |
|
Test report by @boegel |
|
Test report by @boegel |
|
@furstj with the updated easyblock for cmakemake and openfoam in easybuilders/easybuild-easyblocks#1869 you can remove the patch that sets Although they did not make it into EB 4.1.0 |
|
@akesandgren I have just tested easybuilders/easybuild-easyblocks#1869 without -D_qt5.... and it works. Next commit removes the -D_qt5gui... Thanx a lot for more systematic solution. |
boegel
left a comment
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.
lgtm
|
Test report by @lexming |
|
Tested on top of easybuilders/easybuild-easyblocks#1869 |
|
Test report by @boegel |
|
Test report by @boegel |
|
Going in, thanks @furstj! |
(created using
eb --new-pr)