search

rakhimov / scram

Probabilistic Risk Analysis Tool (fault tree analysis, event tree analysis, etc.)
https://scram-pra.org
GNU General Public License v3.0
131 stars 54 forks source link

Cmake not detect catch include files #263

Closed Vascom closed 6 years ago

Vascom commented 6 years ago

Please add detecting presense and directory with catch include files when build with GUI. Sources of scram contain include but in Fedora this file in /include/catch/ and need manually add -I/include/catch to CXX parameters.

P.S. Also it will be good if you solve some simple build warnings like comparsion of signed and unsigned variables. If need I can write new issue for it.

rakhimov commented 6 years ago

Catch is provided as git submodule (may have custom patches). It is currently not meant to work with the system catch. You are circumventing it with sed. The pristine version should have failed with BUILD_TESTING=ON

rakhimov commented 6 years ago

Wsign-compare warning is because of the pesky 'size_t' being unsigned. I don't bother dealing with this language annoyance. http://www.aristeia.com/Papers/C++ReportColumns/sep95.pdf

Vascom commented 6 years ago

It is not simple for fedora packaging to use git submodules. So I disable tests. And will wait 0.17.0 release.

rakhimov commented 6 years ago

It is not simple for fedora packaging to use git submodules. So I disable tests.

I have been also disabling tests in packaging for this reason (Fedora, Debian, Arch, mac, win).

I am not very familiar with Fedora packaging conventions, but IMHO unit/integration/etc. tests could be omitted in distro packaging anyway. Tests that verify the installation/setup should suffice (smoke tests mostly). For example,

scram --help
scram-gui --help
scram --version
scram /path/to/share/scram/input/TwoTrain/two_train.xml

Can this be acceptable for Fedora?

Vascom commented 6 years ago

I am enable tests because I not use scram and this is only way for me to test builded binary. It is normal not enable tests during package build.