-
Notifications
You must be signed in to change notification settings - Fork 32
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
Migrate to SMESH 9.6.0 #31
Conversation
0739710
to
a852e52
Compare
@looooo why is that? |
I do not want to revert netgen to an older version. I guess this will lead to some difficulties with the dependency-solver. So this is a temporary workaround to simplify the packaging for me. The other option is to create a netgen 5.3 package. I guess there is no such package available for conda-forge right now. |
@looooo in this project, netgen 5.3 is now a submodule, built from sources, and the nglib and headers are part of SMESH. So the SMESH project now comes shipped with netgen as another library, no external dependency. put another, it's self-contained and easier to stay synced with upstream Salome. i think it could be adopted to built with the latest netgen. it's not too hard to apply patches to get the right interface in NETGENPlugin. But, Salome applies a bunch of patches to netgen 5.3 to make it work, and those can't be easily applied to later versions of netgen. |
i guess the library could be renamed to SMESHnglib or something to avoid conflicts in case you have other nglib library in your path |
What exactly does this mean? Does this repo now directly depend on master of the smesh sub-module? So a regression there would automatically affect this repo, too?
IMO, this is not necessarily a bad idea. With recent netgen releases I couldn't see much technical improvements but mainly code refactorings and (questionable) usage of the newer C++ standards (C++17 is mandatory now). PR #28 showed that netgen 6.2 technically even is a regression because the option MESHCONST_OPTSURFACE is not supported any more.
Are older versions still supported or is VTK 9.0 or higher mandatory? |
@wwmayer The submodules target a specific release of upstream sources, right now the v9.6.0 tag. They don't point straight to master, so if you go through the "build from sources" in the readme, it should show you how to clone the sources, prepare them with a python script locally, and then build like normal. As new upstream Salome sources are provided, the submodules in this repo can be updated (or not) as needed. So yes, a regression in the upstream sources can affect this project, but we can control when the upstream sources are used, and patches can be applied in this project as well. a good reason to write tests in this project for stuff that matters downstream, among other reasons. I haven't tried building v9.6.0 with older versions of VTK. Upstream Salome uses VTK 9 now, so was trying to stay in sync with that and also the latest OCCT 7.5.0 on conda-forge is also using VTK 9, so trying to keep them all the same. |
OK, this sounds good to me, then.
If older versions of third party libraries are not supported then this will be problematic for FreeCAD as there we try to support all still maintained Linux distributions (mainly Debian and derivatives) |
ok, thats a good point. so netgen from conda-forge and the netgen included within smesh shouldn't have any conflicing files?
ok yes, I understand. I will try to build it for conda-forge, but not yet sure how to handle the submodules. |
I see some issues with freecad compiied against this library and vtk9: |
It doesn't look like it's complicated to fix them. There is a change of the interface of some smesh classes (an int as argument has been dropped). |
FreeCAD/FreeCAD@135525a ports FreeCAD to smesh 9.6 |
thanks @wwmayer . Indeed everything compiled fine for linux. Also doing some basic tests looks promising. I will continue with the scripts for osx and mac. Once thats done I think it's best to create a kind of a release of this repo (@trelau). Once that happened and everything is packaged I will start the occt7.5 migration for conda-forge. |
Nice that you got it working. When building it on Linux I encountered two issues:
|
Not sure what you mean, but with conda (osx, and linux) I had an issue with wrong paths to opengl, I simply sed'ed them away: Regarding windows: I see a problem with finding lzma. Somehow this is related to vtk. But I am not sure if lzma is needed for FreeCAD. |
And yes for sure, a crash is happening when closing freecad.
maybe also related to python3.9... edit: yes I can't reproduce with python3.8 / vtk9 -> so it should be related to python3.9 to reproduce:
So it might be related to python features and py3.9 and for sure is not related to this topic at all ;) |
the win build now stops at finding netgen:
edit: fixed with: looooo/freecad-feedstock@b12b4cf#diff-54cf74e113dd3f6d11e092fdb1d888ec82c69bdafbb15cfb6570c83ecad28f33R24 |
vtk9 uses its own FindOpenGL.cmake module and there it fails to set the variable OPENGL_gl_LIBRARY, i.e. it's empty. Now, I found a way to fix it by changing the order of SetupOpenGL() and SetupSalomeSMESH()
I realized that this was already broken in SMESH 7. Fixed with FreeCAD/FreeCAD@f490852 |
great. I will test if this makes the sed-commands also obsolete. |
I only got the first three errors but not the last two. The three errors are related to an API change in SMESH 9. It's fixed with: |
@wwmayer thanks. I tested with an updated build and there are only two errors remaining. I will continue with reporting in the freecad forum as this is not related to smesh anymore. What is still remianing are some tests with osx as we saw some errors with the tests. |
Why was setting the SOVERSION for the libraries dropped in this patch? |
the tests for freecad are somehow failing on osx (illegal instruction). Not sure if this is related to smesh /netgen. |
@looooo it probably is. Remember we had to exclude the unit test on osx for feedstock. Hmm...probably worth opening up an issue in the SMESH repo. Will need to add osx support on GitHub actions to this repo I think. Probably will need someone with osx system to efficiently debug. |
@trelau it's related to meshing with netgen. I can remember having similar issues with netgen some time ago. Maybe some compile option are the reason for the crash, but I am not sure |
@trelau @wwmayer
this build is using the freecad 0.19 tag. So it can be related to freecad too, as I didn't see this issue some days ago with an older snapshot of freecad. |
This PR changes the nature of this project from containing sources files itself to using upstream repositories from Salome as much as possible. This is being done to make keeping up-to-date with the upstream sources easier and more robust. A few notable changes in this PR compared to the existing project: