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

Add the Accelerate framework to the CMake builds of the samples #612

Merged
merged 2 commits into from
Mar 15, 2019

Conversation

bryanwweber
Copy link
Member

Adds the Accelerate framework to the CMake builds of the C++ samples on macOS.

Does the CI run the samples? If not, I can try to set that up.

@bryanwweber
Copy link
Member Author

bryanwweber commented Mar 8, 2019

So I can't build the samples locally using CMake although everything works just fine with SCons. CMake complains that fmt/format.h can't be found. I can't see any difference between the explicitly included directories for the two methods. Any hints appreciated.

$ cd ~/cantera-test/.../rankine && mkdir build && cd build
$ cmake ..
-- Configuring done
-- Generating done
-- Build files have been written to: /Users/bryan/cantera-test/share/cantera/samples/cxx/rankine/build
$ make
/usr/local/Cellar/cmake/3.13.4/bin/cmake -S/Users/bryan/cantera-test/share/cantera/samples/cxx/rankine -B/Users/bryan/cantera-test/share/cantera/samples/cxx/rankine/build --check-build-system CMakeFiles/Makefile.cmake 0
/usr/local/Cellar/cmake/3.13.4/bin/cmake -E cmake_progress_start /Users/bryan/cantera-test/share/cantera/samples/cxx/rankine/build/CMakeFiles /Users/bryan/cantera-test/share/cantera/samples/cxx/rankine/build/CMakeFiles/progress.marks
/Library/Developer/CommandLineTools/usr/bin/make -f CMakeFiles/Makefile2 all
/Library/Developer/CommandLineTools/usr/bin/make -f CMakeFiles/rankine.dir/build.make CMakeFiles/rankine.dir/depend
cd /Users/bryan/cantera-test/share/cantera/samples/cxx/rankine/build && /usr/local/Cellar/cmake/3.13.4/bin/cmake -E cmake_depends "Unix Makefiles" /Users/bryan/cantera-test/share/cantera/samples/cxx/rankine /Users/bryan/cantera-test/share/cantera/samples/cxx/rankine /Users/bryan/cantera-test/share/cantera/samples/cxx/rankine/build /Users/bryan/cantera-test/share/cantera/samples/cxx/rankine/build /Users/bryan/cantera-test/share/cantera/samples/cxx/rankine/build/CMakeFiles/rankine.dir/DependInfo.cmake --color=
Scanning dependencies of target rankine
/Library/Developer/CommandLineTools/usr/bin/make -f CMakeFiles/rankine.dir/build.make CMakeFiles/rankine.dir/build
[ 50%] Building CXX object CMakeFiles/rankine.dir/rankine.cpp.o
/Library/Developer/CommandLineTools/usr/bin/c++   -I/Users/bryan/cantera-test/include -I/Users/bryan/miniconda3/envs/cantera-dev/include  -isysroot /Library/Developer/CommandLineTools/SDKs/MacOSX10.14.sdk   -std=gnu++11 -o CMakeFiles/rankine.dir/rankine.cpp.o -c /Users/bryan/cantera-test/share/cantera/samples/cxx/rankine/rankine.cpp
In file included from /Users/bryan/cantera-test/share/cantera/samples/cxx/rankine/rankine.cpp:3:
In file included from /Users/bryan/cantera-test/include/cantera/PureFluid.h:9:
In file included from /Users/bryan/cantera-test/include/cantera/thermo/PureFluidPhase.h:17:
In file included from /Users/bryan/cantera-test/include/cantera/thermo/ThermoPhase.h:14:
In file included from /Users/bryan/cantera-test/include/cantera/thermo/Phase.h:12:
In file included from /Users/bryan/cantera-test/include/cantera/base/ctexceptions.h:14:
/Users/bryan/cantera-test/include/cantera/base/fmt.h:15:12: fatal error: 'fmt/format.h' file not found
  #include "fmt/format.h"
           ^~~~~~~~~~~~~~
1 error generated.
make[2]: *** [CMakeFiles/rankine.dir/rankine.cpp.o] Error 1
make[1]: *** [CMakeFiles/rankine.dir/all] Error 2
make: *** [all] Error 2

Here is cantera.conf:

prefix = '~/cantera-test'
python_package = 'minimal'
python_cmd = 'python3'
f90_interface = 'n'
system_sundials = 'n'
boost_inc_dir = '${CONDA_PREFIX}/include'

@codecov
Copy link

codecov bot commented Mar 8, 2019

Codecov Report

Merging #612 into master will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##           master     #612   +/-   ##
=======================================
  Coverage   68.52%   68.52%           
=======================================
  Files         363      363           
  Lines       39953    39953           
=======================================
  Hits        27378    27378           
  Misses      12575    12575
Impacted Files Coverage Δ
src/transport/GasTransport.cpp 90.98% <0%> (ø) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update d6d91f4...81b08d2. Read the comment docs.

@speth
Copy link
Member

speth commented Mar 8, 2019

The CI does not currently build or run the samples. You can do the former with scons samples, but that's not the same as actually using the generated SConstruct and CMakeLists.txt.

By default, CMake uses the system SDK on macOS as the system root by
setting the isysroot flag to clang. This setting removes /usr/local from
the include search path.
@bryanwweber
Copy link
Member Author

OK, I fixed the build problems. I didn't realize that SCons had picked up the system version of fmt. I just pushed a change that should fix that CMake doesn't pick up /usr/local/include when -isysroot is set (which it is by default, at least on my computer).

I think this work came out of me trying to build the samples to test the install of the Conda package of libcantera. So now I need to go back to that and figure out where I was... 😄

@bryanwweber
Copy link
Member Author

I also don't think its worthwhile to build the samples at this time. If we look at improving the CI system in the future, I think it can be something we integrate then.

@speth speth merged commit 4026c17 into Cantera:master Mar 15, 2019
@bryanwweber bryanwweber deleted the add_accelerate_cmake_samples branch March 15, 2019 19:42
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

Successfully merging this pull request may close these issues.

2 participants