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 yaml-cpp to the shared libraries if building with system yaml-cpp #671

Merged
merged 3 commits into from
Aug 1, 2019

Conversation

spinnau
Copy link
Contributor

@spinnau spinnau commented Jul 23, 2019

Fixes #668

Changes proposed in this pull request:

  • If building Cantera with system_yamlcpp=y the yaml-cpp library needs to be specified in the linker flags.

@codecov
Copy link

codecov bot commented Jul 23, 2019

Codecov Report

Merging #671 into master will increase coverage by <.01%.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #671      +/-   ##
==========================================
+ Coverage   70.78%   70.78%   +<.01%     
==========================================
  Files         373      373              
  Lines       43454    43454              
==========================================
+ Hits        30757    30758       +1     
+ Misses      12697    12696       -1
Impacted Files Coverage Δ
src/transport/GasTransport.cpp 90.58% <0%> (+0.2%) ⬆️

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 5f08b36...1fde140. Read the comment docs.

@codecov
Copy link

codecov bot commented Jul 23, 2019

Codecov Report

Merging #671 into master will increase coverage by 0.11%.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #671      +/-   ##
==========================================
+ Coverage   70.66%   70.78%   +0.11%     
==========================================
  Files         372      373       +1     
  Lines       43507    43454      -53     
==========================================
+ Hits        30745    30758      +13     
+ Misses      12762    12696      -66
Impacted Files Coverage Δ
test_problems/pureFluidTest/testPureWater.cpp 96.87% <0%> (-0.1%) ⬇️
include/cantera/zeroD/ReactorFactory.h 100% <0%> (ø) ⬆️
include/cantera/zeroD/WallFactory.h 64.28% <0%> (ø) ⬆️
include/cantera/zeroD/FlowDeviceFactory.h 64.28% <0%> (ø) ⬆️
include/cantera/zeroD/Wall.h 50% <0%> (ø) ⬆️
include/cantera/PureFluid.h 66.66% <0%> (ø)
test_problems/rankine_democxx/rankine.cpp 93.87% <0%> (+0.39%) ⬆️
src/clib/ctreactor.cpp 7.77% <0%> (+0.76%) ⬆️
src/kinetics/importKinetics.cpp 98.4% <0%> (+0.8%) ⬆️
src/clib/Cabinet.h 33.96% <0%> (+7.54%) ⬆️
... and 10 more

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 6852087...6ecd5ad. Read the comment docs.

@bryanwweber
Copy link
Member

I pushed a few more changes to fix the compiling issue I was having on macOS with system_yamlcpp and remove linking to libfmt, since we use the header-only version now to my understanding.

@speth Can you take a look and verify that these changes are correct?

@spinnau
Copy link
Contributor Author

spinnau commented Jul 23, 2019

@bryanwweber: Compiles and works fine for me on Linux with your changes and system_fmt=y.

spinnau and others added 3 commits August 1, 2019 17:30
…-cpp

If building Cantera with system_yamlcpp=y the yaml-cpp library needs to be
specified in the linker flags.

Fixes Cantera#668
Similar to the system_sundials option, libyaml-cpp must be added to the
linker line when the system libraries are used.
We use the header-only version of fmt now, so no need to link to the
compiled library.
@speth speth merged commit bfa5a66 into Cantera:master Aug 1, 2019
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.

Cantera import error in python when built with system yaml-cpp
3 participants