search

FreeCAD / homebrew-freecad

Homebrew recipes for FreeCAD
53 stars 34 forks source link

Salome MED file formula should be submitted to homebrew/science #27

Closed bblacey closed 7 years ago

bblacey commented 7 years ago

To reduce maintenance and give-back to the community, the MED file formula should be refined to meet the homebrew "acceptable formula" guidelines and submitted as a pull-request to homebrew/science. This should be relatively a straight-forward effort (e.g. add test cases and peer review) will ensure that bottles are available for all homebrew-support operating systems and will eliminate the need for the FreeCAD community.

luzpaz commented 7 years ago

Perhaps you want to ping the Salome folks to see if they want to assist ?

ianrrees commented 7 years ago

@luzpaz - are you aware of a Salome presence on github or Homebrew?

luzpaz commented 7 years ago

@ianrrees No. I don't. But there may be devs/volunteers that would like to help. Please see: http://www.salome-platform.org/forum/forum_9/344694157

Edit: ref: announcement on the above thread: http://www.salome-platform.org/forum/forum_9/344694157/455249875

luzpaz commented 7 years ago

Adding reference to the Salome MED homebrew formula: https://github.com/FreeCAD/homebrew-freecad/blob/master/med-file.rb

bvenkat commented 7 years ago

I am the author of the Salome forum post luzpaz refers to above. I am happy to assist though am not familiar with ruby / homebrew so will have some learning curve.

bblacey commented 7 years ago

@bvenkat - excellent and thanks for the offer. I can handle the homebrew/ruby machinations but perhaps you could help with the issues of building the Salome MED python bindings on macOS. I am wrestling with the proper config/compiler/linker settings to build both medC and the python bindings on macOS.

bvenkat commented 7 years ago

@bblacey : medC should build out of the box. I did get some errors with the python bindings (these are not included in the build for Salome) with swig 2.0.12 and the below patch is a work around for the errors. Can you kindly check and if you still see errors then post the details such swig version and the error messages.

diff -ruN med-3.2.0_orig/python/medenum_module.i med-3.2.0/python/medenum_module.i
--- med-3.2.0_orig/python/medenum_module.i  2016-01-14 21:07:48.000000000 +0530
+++ med-3.2.0/python/medenum_module.i   2017-02-14 01:53:02.000000000 +0530
@@ -6,6 +6,7 @@

 %{
 #include "med.h"
+#include <utility>
 %}

 %include "H5public_extract.h"
diff -ruN med-3.2.0_orig/python/medenumtest_module.i med-3.2.0/python/medenumtest_module.i
--- med-3.2.0_orig/python/medenumtest_module.i  2015-12-16 16:27:04.000000000 +0530
+++ med-3.2.0/python/medenumtest_module.i   2017-02-14 01:53:29.000000000 +0530
@@ -4,6 +4,7 @@

 %{
 #include "med.h"
+#include <utility>
 %}

 %include "H5public_extract.h"
bblacey commented 7 years ago

Thanks. Are you using cmake or configure to build MedC? I have found that I have to use GNUs' libstdc++, instead of the default LLVM libc++ regardless of whether I regenerate the SWIG headers or not. SWIG on my system is 3.0.12.

bvenkat commented 7 years ago

Would swig 2.0.12 do for you? I have not tested on swig 3.x because Salome itself uses 2.x. Am using cmake for the build system. medC should compile with clang/libc++ out of the box, can you post your errors. I am on El Capitan, not sure if that makes any difference.

bvenkat commented 7 years ago

It is possible that the errors with libc++ you are seeing are because your hdf5 is compiled using libstdc++. Can you check by running otool -L /path/to/libhdf5_cpp.dylib. If so, can you try compiling hdf5 using libc++.

ianrrees commented 7 years ago

@bvenkat - using SWIG 2 would likely be problematic, since we get SWIG from Homebrew core.

bblacey commented 7 years ago

I am away from my computer right now but using cmake to build on 10.10 with Xcode 6.4 (our continuous integration config), the linker fails with unresolved externals that I can get past if I use stdlib=libstdc++ but I eventually receive a warning, albeit non-fatal, that hdf5 was built using libc++.

bvenkat commented 7 years ago

unresolved externals - can you post the exact error. Output of otool -L /path/to/libhdf5_cpp.dylib will be helpful as well. I am able to build med with swig 3.0.12 as well but there is one change that I need to test / check further, will post a patch once done.

bblacey commented 7 years ago

Here you go - would have posted details earlier but I was responding with my phone....

cmake options:

cmake .. \
 -DCMAKE_C_FLAGS_RELEASE=-DNDEBUG \
 -DCMAKE_CXX_FLAGS_RELEASE=-DNDEBUG \
 -DCMAKE_INSTALL_PREFIX=/usr/local/Cellar/med-file/3.2.0_1 \
 -DCMAKE_BUILD_TYPE=Release \
 -DCMAKE_FIND_FRAMEWORK=LAST \
 -DCMAKE_VERBOSE_MAKEFILE=ON -Wno-dev \
 -DCMAKE_Fortran_COMPILER:BOOL=OFF \
 -DMEDFILE_BUILD_TESTS:BOOL=OFF \
 -DMEDFILE_INSTALL_DOC:BOOL=OFF \
 -DMEDFILE_BUILD_PYTHON:BOOL=ON \
 -DPYTHON_INCLUDE_DIR:PATH=/usr/local/Cellar/python/2.7.13/Frameworks/Python.framework/Versions/2.7/include/python2.7 \
 -DPYTHON_LIBRARY:FILEPATH=/usr/local/Cellar/python/2.7.13/Frameworks/Python.framework/Versions/2.7/lib/libpython2.7.dylib

Build errors (when not using stdlib=libstdc++

[ 98%] Building CXX object python/CMakeFiles/_medenum.dir/med/medenum_modulePYTHON_wrap.cxx.o
cd /tmp/med-file-20170213-53667-1pdhq3x/med-3.2.0/build/python && /usr/local/Homebrew/Library/Homebrew/shims/super/clang++   -DH5_USE_16_API -D_medenum_EXPORTS -I/usr/local/Cellar/python/2.7.13/Frameworks/Python.framework/Versions/2.7/include/python2.7 -I/tmp/med-file-20170213-53667-1pdhq3x/med-3.2.0/include -I/tmp/med-file-20170213-53667-1pdhq3x/med-3.2.0/build/include -I/usr/local/Cellar/hdf5/1.10.0-patch1/include -I/usr/local/opt/szip/include  -DNDEBUG -fPIC   -o CMakeFiles/_medenum.dir/med/medenum_modulePYTHON_wrap.cxx.o -c /tmp/med-file-20170213-53667-1pdhq3x/med-3.2.0/build/python/med/medenum_modulePYTHON_wrap.cxx
/tmp/med-file-20170213-53667-1pdhq3x/med-3.2.0/build/python/med/medenum_modulePYTHON_wrap.cxx:3656:16: error: no type named 'pair' in namespace 'std'
  typedef std::pair<int,const char * const> enum_;
          ~~~~~^
/tmp/med-file-20170213-53667-1pdhq3x/med-3.2.0/build/python/med/medenum_modulePYTHON_wrap.cxx:3656:20: error: expected unqualified-id
  typedef std::pair<int,const char * const> enum_;
                   ^
/tmp/med-file-20170213-53667-1pdhq3x/med-3.2.0/build/python/med/medenum_modulePYTHON_wrap.cxx:3659:9: error: unknown type name 'enum_'; did you mean 'enum'?
  const enum_ MED_MESH_TYPE_init[] = {
        ^~~~~
        enum
/tmp/med-file-20170213-53667-1pdhq3x/med-3.2.0/build/python/med/medenum_modulePYTHON_wrap.cxx:3659:15: error: ISO C++ forbids forward references to 'enum' types
  const enum_ MED_MESH_TYPE_init[] = {
              ^
/tmp/med-file-20170213-53667-1pdhq3x/med-3.2.0/build/python/med/medenum_modulePYTHON_wrap.cxx:3659:36: error: expected unqualified-id
  const enum_ MED_MESH_TYPE_init[] = {
                                   ^
/tmp/med-file-20170213-53667-1pdhq3x/med-3.2.0/build/python/med/medenum_modulePYTHON_wrap.cxx:3697:95: error: expected ')'
  MED_MESH_TYPE::Get__str__ MED_MESH_TYPE::_get__str__(MED_MESH_TYPE_init, MED_MESH_TYPE_init + sizeof(MED_MESH_TYPE_init)/ sizeof(*MED_MESH_TYPE_init));
                                                                                              ^
/tmp/med-file-20170213-53667-1pdhq3x/med-3.2.0/build/python/med/medenum_modulePYTHON_wrap.cxx:3697:55: note: to match this '('
  MED_MESH_TYPE::Get__str__ MED_MESH_TYPE::_get__str__(MED_MESH_TYPE_init, MED_MESH_TYPE_init + sizeof(MED_MESH_TYPE_init)/ sizeof(*MED_MESH_TYPE_init));
                                                      ^
/tmp/med-file-20170213-53667-1pdhq3x/med-3.2.0/build/python/med/medenum_modulePYTHON_wrap.cxx:3697:44: error: redefinition of '_get__str__' as different kind of symbol
  MED_MESH_TYPE::Get__str__ MED_MESH_TYPE::_get__str__(MED_MESH_TYPE_init, MED_MESH_TYPE_init + sizeof(MED_MESH_TYPE_init)/ sizeof(*MED_MESH_TYPE_init));
                                           ^
/tmp/med-file-20170213-53667-1pdhq3x/med-3.2.0/build/python/med/medenum_modulePYTHON_wrap.cxx:3676:24: note: previous definition is here
    static  Get__str__ _get__str__;
                       ^
/tmp/med-file-20170213-53667-1pdhq3x/med-3.2.0/build/python/med/medenum_modulePYTHON_wrap.cxx:3709:9: error: unknown type name 'enum_'; did you mean 'enum'?
  const enum_ MED_SORTING_TYPE_init[] = {
        ^~~~~
        enum
/tmp/med-file-20170213-53667-1pdhq3x/med-3.2.0/build/python/med/medenum_modulePYTHON_wrap.cxx:3709:15: error: ISO C++ forbids forward references to 'enum' types
  const enum_ MED_SORTING_TYPE_init[] = {
              ^
/tmp/med-file-20170213-53667-1pdhq3x/med-3.2.0/build/python/med/medenum_modulePYTHON_wrap.cxx:3709:39: error: expected unqualified-id
  const enum_ MED_SORTING_TYPE_init[] = {
                                      ^
/tmp/med-file-20170213-53667-1pdhq3x/med-3.2.0/build/python/med/medenum_modulePYTHON_wrap.cxx:3747:107: error: expected ')'
  MED_SORTING_TYPE::Get__str__ MED_SORTING_TYPE::_get__str__(MED_SORTING_TYPE_init, MED_SORTING_TYPE_init + sizeof(MED_SORTING_TYPE_init)/ sizeof(*MED_SORTING_TYPE_init));
                                                                                                          ^
/tmp/med-file-20170213-53667-1pdhq3x/med-3.2.0/build/python/med/medenum_modulePYTHON_wrap.cxx:3747:61: note: to match this '('
  MED_SORTING_TYPE::Get__str__ MED_SORTING_TYPE::_get__str__(MED_SORTING_TYPE_init, MED_SORTING_TYPE_init + sizeof(MED_SORTING_TYPE_init)/ sizeof(*MED_SORTING_TYPE_init));
                                                            ^
/tmp/med-file-20170213-53667-1pdhq3x/med-3.2.0/build/python/med/medenum_modulePYTHON_wrap.cxx:3747:50: error: redefinition of '_get__str__' as different kind of symbol
  MED_SORTING_TYPE::Get__str__ MED_SORTING_TYPE::_get__str__(MED_SORTING_TYPE_init, MED_SORTING_TYPE_init + sizeof(MED_SORTING_TYPE_init)/ sizeof(*MED_SORTING_TYPE_init));
                                                 ^
/tmp/med-file-20170213-53667-1pdhq3x/med-3.2.0/build/python/med/medenum_modulePYTHON_wrap.cxx:3726:24: note: previous definition is here
    static  Get__str__ _get__str__;
                       ^
/tmp/med-file-20170213-53667-1pdhq3x/med-3.2.0/build/python/med/medenum_modulePYTHON_wrap.cxx:3759:9: error: unknown type name 'enum_'; did you mean 'enum'?
  const enum_ MED_GRID_TYPE_init[] = {
        ^~~~~
        enum
/tmp/med-file-20170213-53667-1pdhq3x/med-3.2.0/build/python/med/medenum_modulePYTHON_wrap.cxx:3759:15: error: ISO C++ forbids forward references to 'enum' types
  const enum_ MED_GRID_TYPE_init[] = {
              ^
/tmp/med-file-20170213-53667-1pdhq3x/med-3.2.0/build/python/med/medenum_modulePYTHON_wrap.cxx:3759:36: error: expected unqualified-id
  const enum_ MED_GRID_TYPE_init[] = {
                                   ^
/tmp/med-file-20170213-53667-1pdhq3x/med-3.2.0/build/python/med/medenum_modulePYTHON_wrap.cxx:3798:95: error: expected ')'
  MED_GRID_TYPE::Get__str__ MED_GRID_TYPE::_get__str__(MED_GRID_TYPE_init, MED_GRID_TYPE_init + sizeof(MED_GRID_TYPE_init)/ sizeof(*MED_GRID_TYPE_init));
                                                                                              ^
/tmp/med-file-20170213-53667-1pdhq3x/med-3.2.0/build/python/med/medenum_modulePYTHON_wrap.cxx:3798:55: note: to match this '('
  MED_GRID_TYPE::Get__str__ MED_GRID_TYPE::_get__str__(MED_GRID_TYPE_init, MED_GRID_TYPE_init + sizeof(MED_GRID_TYPE_init)/ sizeof(*MED_GRID_TYPE_init));
                                                      ^
/tmp/med-file-20170213-53667-1pdhq3x/med-3.2.0/build/python/med/medenum_modulePYTHON_wrap.cxx:3798:44: error: redefinition of '_get__str__' as different kind of symbol
  MED_GRID_TYPE::Get__str__ MED_GRID_TYPE::_get__str__(MED_GRID_TYPE_init, MED_GRID_TYPE_init + sizeof(MED_GRID_TYPE_init)/ sizeof(*MED_GRID_TYPE_init));
                                           ^
/tmp/med-file-20170213-53667-1pdhq3x/med-3.2.0/build/python/med/medenum_modulePYTHON_wrap.cxx:3777:24: note: previous definition is here
    static  Get__str__ _get__str__;
                       ^
/tmp/med-file-20170213-53667-1pdhq3x/med-3.2.0/build/python/med/medenum_modulePYTHON_wrap.cxx:3810:9: error: unknown type name 'enum_'; did you mean 'enum'?
  const enum_ MED_ENTITY_TYPE_init[] = {
        ^~~~~
        enum
/tmp/med-file-20170213-53667-1pdhq3x/med-3.2.0/build/python/med/medenum_modulePYTHON_wrap.cxx:3810:15: error: ISO C++ forbids forward references to 'enum' types
  const enum_ MED_ENTITY_TYPE_init[] = {
              ^

Warning when building with libstdc++

Warning: freecad/freecad/med-file dependency homebrew/science/hdf5 was built with a different C++ standard
library (libc++ from clang). This may cause problems at runtime.

HDF5 libraries:

 otool -L /usr/local/opt/hdf5/lib/libhdf5_cpp.dylib 
/usr/local/opt/hdf5/lib/libhdf5_cpp.dylib:
    /usr/local/opt/hdf5/lib/libhdf5_cpp.100.dylib (compatibility version 101.0.0, current version 101.0.0)
    /usr/local/Cellar/hdf5/1.10.0-patch1/lib/libhdf5.100.dylib (compatibility version 101.0.0, current version 101.1.0)
    /usr/local/opt/szip/lib/libsz.2.dylib (compatibility version 3.0.0, current version 3.0.0)
    /usr/lib/libz.1.dylib (compatibility version 1.0.0, current version 1.2.5)
    /usr/lib/libSystem.B.dylib (compatibility version 1.0.0, current version 1226.10.1)
    /usr/lib/libc++.1.dylib (compatibility version 1.0.0, current version 120.1.0)
bvenkat commented 7 years ago

All the error messages appear to be in the python sub-directory and seem to be swig related. Can you kindly apply the patch below and try a clean reinstall. This should get medC & python bindings to compile with swig 3.x. The patch to the file H5public_extract.h.in - am not sure why this is compatible with swig 2.x but not swig 3.x and I could not find the info anywere. However the change seems to be harmless and it will be good to try & test the application to see if it is working correctly.

diff -ruN med-3.2.0_orig/include/H5public_extract.h.in med-3.2.0/include/H5public_extract.h.in
--- med-3.2.0_orig/include/H5public_extract.h.in    2016-01-14 21:25:27.000000000 +0530
+++ med-3.2.0/include/H5public_extract.h.in 2017-02-14 05:59:38.000000000 +0530
@@ -28,9 +28,6 @@
 @HDF5_TYPEDEF_HID_T@
 @HDF5_TYPEDEF_HSIZE_T@

-#typedef int herr_t;
-#typedef int hid_t;
-#typedef unsigned long long   hsize_t;

 #ifdef __cplusplus
 }
diff -ruN med-3.2.0_orig/python/medenum_module.i med-3.2.0/python/medenum_module.i
--- med-3.2.0_orig/python/medenum_module.i  2016-01-14 21:07:48.000000000 +0530
+++ med-3.2.0/python/medenum_module.i   2017-02-14 05:59:51.000000000 +0530
@@ -6,6 +6,7 @@

 %{
 #include "med.h"
+#include <utility>
 %}

 %include "H5public_extract.h"
diff -ruN med-3.2.0_orig/python/medenumtest_module.i med-3.2.0/python/medenumtest_module.i
--- med-3.2.0_orig/python/medenumtest_module.i  2015-12-16 16:27:04.000000000 +0530
+++ med-3.2.0/python/medenumtest_module.i   2017-02-14 05:59:51.000000000 +0530
@@ -4,6 +4,7 @@

 %{
 #include "med.h"
+#include <utility>
 %}

 %include "H5public_extract.h"
bblacey commented 7 years ago

patch -p1 <medfile.patch is complaining - most likely due to line endings not pasting/copying properly. I will patch by hand.

Also, I had already patched include/H5public_extract.h

bvenkat commented 7 years ago

Link to gist for the patch: https://gist.github.com/bvenkat/6185c7399f5b79dcd3d95eeb919defae

bblacey commented 7 years ago

Ok, I think we nailed it! The #include <utility> was the missing piece to the puzzle but also made two other patches:

diff --git a/python/CMakeLists.txt b/python/CMakeLists.txt
index 4016f7c..5d27f8f 100644
--- a/python/CMakeLists.txt
+++ b/python/CMakeLists.txt
@@ -27,7 +27,7 @@ SET(_swig_files
 )

 SET(_link_libs
-  med
+  medC
   ${PYTHON_LIBRARIES}
   )

diff --git a/src/2.3.6/ci/MEDequivInfo.c b/src/2.3.6/ci/MEDequivInfo.c
index 60a97e8..d157cb9 100644
--- a/src/2.3.6/ci/MEDequivInfo.c
+++ b/src/2.3.6/ci/MEDequivInfo.c
@@ -24,7 +24,7 @@
 #include <stdlib.h>

 int
-MEDequivInfo(int fid, char *maa, int ind, char *eq, char *des)
+MEDequivInfo(med_idt fid, char *maa, int ind, char *eq, char *des)
 {
   med_idt eqid;
   med_err ret;

And we see libc++ in the python bindings.

otool -L /usr/local/Cellar/med-file/3.2.0_1/lib/python2.7/site-packages/med/_medenum.so
/usr/local/Cellar/med-file/3.2.0_1/lib/python2.7/site-packages/med/_medenum.so:
    /usr/local/Cellar/med-file/3.2.0_1/lib/libmedC.dylib (compatibility version 0.0.0, current version 0.0.0)
    /usr/local/opt/python/Frameworks/Python.framework/Versions/2.7/Python (compatibility version 2.7.0, current version 2.7.0)
    /usr/local/opt/hdf5/lib/libhdf5.100.dylib (compatibility version 101.0.0, current version 101.1.0)
    /usr/local/opt/szip/lib/libsz.2.dylib (compatibility version 3.0.0, current version 3.0.0)
    /usr/lib/libz.1.dylib (compatibility version 1.0.0, current version 1.2.5)
    /usr/lib/libSystem.B.dylib (compatibility version 1.0.0, current version 1213.0.0)
    /usr/lib/libc++.1.dylib (compatibility version 1.0.0, current version 120.0.0)

So much for out-of-the box ;)

Thanks @bvenkat for your help!

bblacey commented 7 years ago

@bvenkat, it turns out we need on more patch to tighten this down. On macOS, it is recommended not to link directly to the python library but instead perform dynamic lookup.

git diff python/CMakeLists.txt 
diff --git a/python/CMakeLists.txt b/python/CMakeLists.txt
index 4016f7c..1e45797 100644
--- a/python/CMakeLists.txt
+++ b/python/CMakeLists.txt
@@ -26,8 +26,12 @@ SET(_swig_files
   medsubdomain_module.i
 )

+IF(APPLE)
+  SET(PYTHON_LIBRARIES "-undefined dynamic_lookup")
+ENDIF(APPLE)
+
 SET(_link_libs
-  med
+  medC
   ${PYTHON_LIBRARIES}
   )

Thanks again for your help.

bblacey commented 7 years ago

med-file formula submitted to Homebrew. Once it is merged into homebrew-science/master, I will remove med-file from this repo, refresh the ports-cache and close this issue.

bvenkat commented 7 years ago

Happy to see the progress and thanks for your good work. Cheers!

luzpaz commented 7 years ago

Thanks for all your work folks!

bblacey commented 7 years ago

Initial pursuit for patches to be merged upstream

luzpaz commented 7 years ago

Link to the submitted SALOME med file to homebrew-science: https://github.com/Homebrew/homebrew-science/pull/5138

luzpaz commented 7 years ago

SALOME forum post mentioning the patches: http://www.salome-platform.org/forum/forum_10/317092432

luzpaz commented 7 years ago

Uploaded clean patch to http://www.salome-platform.org/forum/forum_10/317092432#104751938

bblacey commented 7 years ago

Patch-set for reference if needed upstream.

med-file.patch.zip