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

Fix conda example packaging #1366

Merged
merged 4 commits into from
Aug 25, 2022
Merged

Fix conda example packaging #1366

merged 4 commits into from
Aug 25, 2022

Conversation

ischoegl
Copy link
Member

@ischoegl ischoegl commented Aug 13, 2022

Changes proposed in this pull request

  • Add SCons option package_build
  • If selected (i.e. package_build=y), exclude 'include' and 'library' paths outside of build and host enviroments in development sample Makefile's and SConstruct's, as well as include/cantera/Cantera.mak and /lib/pkgconfig/cantera.pc
  • Fix Fortran Makefiles

PR ensures that libcantera-devel packages all C++ and Fortran examples without referencing the build environment.

If applicable, fill in the issue number this pull request is fixing

Closes Cantera/conda-recipes#37

If applicable, provide an example illustrating new features this pull request is introducing

With an updated conda-recipes using the SCons flag package_build=y, the following will work

$ conda create -y -n ct-env make scons cmake
$ conda activate ct-env
$ conda install -y -c cantera libcantera-devel

C++ examples can be compiled with preconfigured build tools:

$ cd path/to/conda/envs/ct-env/shared/cantera/samples/cxx/demo
$ make
$ scons
$ cmake . && cmake --build .

Same for Fortran F90 (and F77):

$ cd path/to/conda/envs/ct-env/shared/cantera/samples/f90
$ make
$ scons
$ cmake . && cmake --build .

Checklist

  • The pull request includes a clear description of this code change
  • Commit messages have short titles and reference relevant issues
  • Build passes (scons build & scons test) and unit tests address code coverage
  • Style & formatting of contributed code follows contributing guidelines
  • The pull request is ready for review

Copy link
Member

@speth speth left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This makes sense to me, in general. I have just a few questions / suggestions related to a few of the implementation details.

samples/f77/Makefile.in Outdated Show resolved Hide resolved
site_scons/buildutils.py Show resolved Hide resolved
samples/cxx/SConscript Outdated Show resolved Hide resolved
platform/posix/SConscript Outdated Show resolved Hide resolved
@ischoegl
Copy link
Member Author

ischoegl commented Aug 22, 2022

@speth ... thanks for the review! I addressed all comments - overall, I believe this PR should fix most remaining issues for libcantera-devel built by conda-recipes. (once those pending PR's as well as the package_build option are added)

On a separate note, for the last action, all 18.04 ubuntu runners were cancelled.

@codecov
Copy link

codecov bot commented Aug 22, 2022

Codecov Report

Merging #1366 (d710204) into main (b9148b0) will decrease coverage by 0.03%.
The diff coverage is n/a.

@@            Coverage Diff             @@
##             main    #1366      +/-   ##
==========================================
- Coverage   70.93%   70.89%   -0.04%     
==========================================
  Files         356      357       +1     
  Lines       51471    51500      +29     
  Branches    17189    17198       +9     
==========================================
  Hits        36510    36510              
- Misses      12667    12696      +29     
  Partials     2294     2294              
Impacted Files Coverage Δ
src/clib/ct.cpp 7.56% <0.00%> (-0.13%) ⬇️
include/cantera/base/ExternalLogger.h 0.00% <0.00%> (ø)

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

Copy link
Member

@speth speth left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me.

@ischoegl ischoegl merged commit 60b3fb6 into Cantera:main Aug 25, 2022
@ischoegl ischoegl deleted the fix-packaging branch August 25, 2022 02:38
@ischoegl ischoegl mentioned this pull request Aug 27, 2022
5 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

conda-build mangles libcantera-devel build system files
3 participants