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

dev-cpp/blitz: 1.0.3_pre fix boost, doxygen dependencies #1065

Closed
wants to merge 2 commits into from
Closed

dev-cpp/blitz: 1.0.3_pre fix boost, doxygen dependencies #1065

wants to merge 2 commits into from

Conversation

band-a-prend
Copy link
Contributor

@band-a-prend band-a-prend commented Mar 14, 2021

  • The blitz-1.0.2_p20200524 provides libblitz.so.1.0.3 therefore
    the new ebuild is renamed to blitz-1.0.3_pre20200524 instead of revision bump while both use the same git-commit-sha-tarball.

  • Blitz++ optionally depends on dev-libs/boost[static-libs]
    to allow build with Boost::serialization support
    (note: cmake option is realy with typo).
    USE="boost" is used to handle this option.

  • Add USE="doc" app-doc/doxygen[dot] build dependency to build dev docs.

  • Add python build time dependency.

  • The testsuite, benchmark, examples targets now is only conditionally build
    in src_compile() and tests are ran in implicit src_test() phase.

  • Update live ebuild.

@band-a-prend
Copy link
Contributor Author

It's seems that using of boost serialization is optional, so it's need to define with use flag.

Copy link
Contributor

@epsilon-0 epsilon-0 left a comment

Choose a reason for hiding this comment

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

Thanks a lot.
Have left a few comments.

Squash both commits into one.

Drop the previous version of the dated ebuild, in a separate commit.

src_prepare() {
default
# provide "libblitz-1.0.2.so" instead of "libblitz-1.0.3.so"
sed -i -e "s/blitz_PATCH 3/blitz_PATCH 2/" CMakeLists.txt || die
Copy link
Contributor

Choose a reason for hiding this comment

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

lets not do this, there's no specific reason and does not provide any benefit,

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It slightly confuse me that package is 1.0.2 but installed library is 1.0.3.
Maybe then to rename ebuild to blitz-1.0.3_pre20200524, isn't?
And then to drop old dated ebuild.

Copy link
Contributor

Choose a reason for hiding this comment

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

That sounds good 👍

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done

BDEPEND="${PYTHON_DEPS}"
DEPEND="
dev-libs/boost:=
doc? ( app-doc/doxygen[dot] )
Copy link
Contributor

Choose a reason for hiding this comment

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

doxygen needs to be in BDEPEND.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done

}

src_compile() {
cmake_build
Copy link
Contributor

Choose a reason for hiding this comment

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

cmake_src_compile

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done. With addition of conditionally compiled doc and test targets.

}

src_test() {
cmake_build check-testsuite check-benchmarks check-examples
Copy link
Contributor

Choose a reason for hiding this comment

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

does it not need cmake_src_test?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

cmake_src_test tries to run precompiled testsuite, benchmark and examples tests. So the build of this targets is placed in src_compile() under appropriate condition.

@band-a-prend
Copy link
Contributor Author

Boost::serialization support requires dev-libs/boost[static-libs] as it is checked in src/CMakeLists.txt if it's static library is presented in system.

@Nowa-Ammerlaan
Copy link
Member

Nowa-Ammerlaan commented Mar 16, 2021

Boost::serialization support requires dev-libs/boost[static-libs] as it is checked in src/CMakeLists.txt if it's static library is presented in system.

Sometimes you can just sed s/.a/.so/g in those cases (and sometimes that will break everything)

@band-a-prend
Copy link
Contributor Author

Sometimes you can just sed s/.a/.so/g in those cases (and sometimes that will break everything)

This option is used and affected this

set(Boost_USE_STATIC_LIBS        ON) # only find static libs

The blitz unconditionally provides static library build that also build in boost::serialization in case under review.

In some cases static linking is usefull in scientific application then it build on platform with differ runtime and use just to run on other system. I'm not a fan of this approach but I encountered such case several times.

@band-a-prend band-a-prend changed the title dev-cpp/blitz: 1.0.2 fix boost, doxygen dependencies dev-cpp/blitz: 1.0.3_pre fix boost, doxygen dependencies Mar 16, 2021

EAPI=7

CMAKE_MAKEFILE_GENERATOR=emake
Copy link
Member

Choose a reason for hiding this comment

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

We really need this? :(

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Usually it requires if Fortran code is compiled. I.e. Sundials had compilation problems with ninja.

In blitz++ the Fortran compiler is required only optionally to build fortran benckmark that is used to compare speed of blitz++ with pure fortran array operations.

In this ebuild this benchmark isn't built so maybe blitz++ could use ninja. I need to recheck it.

DESCRIPTION="Multi-dimensional array library for C++"
HOMEPAGE="https://github.com/blitzpp/blitz"

COMMIT=39f885951a9b8b11f931f917935a16066a945056
Copy link
Member

Choose a reason for hiding this comment

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

Standard structure has DESCRIPTION/HOMEPAGE/SRC_URI/S together, then a blank newline, then LICENSE/SLOT/KEYWORDS/IUSE/RESTRICT.

Copy link
Member

Choose a reason for hiding this comment

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

I'd also quote COMMIT and put it above DESCRIPTION.

-DBUILD_TESTING=$(usex test)
-DBUILD_DOC=$(usex doc)
)
use boost && mycmakeargs+=( -DENABLE_SERIALISATION=ON )
Copy link
Contributor

Choose a reason for hiding this comment

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

add it to the original list with -DENABLE_SERIALISATION=$(usex boost)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ready

The blitz-1.0.2_p20200524 provides libblitz.so.1.0.3 therefore
the new ebuild is renamed to *-1.0.3_pre20200524 instead of revision bump
while both use the same git-commit-sha-tarball.

Blitz++ optionally depends on dev-libs/boost[static-libs]
to allow build with Boost::serialization support
(note: cmake option is realy with typo).
USE="boost" is used to handle this option.

Add USE="doc" app-doc/doxygen build dependency to build dev docs.

Add python build time dependency.

The testsuite, benchmark, examples targets now is only conditionally build
in src_compile() and tests are ran in implicit src_test() phase.

Update live ebuild.

Signed-off-by: band-a-prend <torokhov-s-a@yandex.ru>
Signed-off-by: Sergey Torokhov <torokhov-s-a@yandex.ru>
@epsilon-0
Copy link
Contributor

I have to fix my gentoo machine and will merge this when I get it fixed ❤️

@band-a-prend
Copy link
Contributor Author

@epsilon-0
Ping!

@epsilon-0
Copy link
Contributor

thanks! merged :D

@band-a-prend band-a-prend deleted the blitz branch April 21, 2021 15:46
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.

4 participants