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

make install fail #11

Closed
videlec opened this issue Mar 13, 2018 · 19 comments
Closed

make install fail #11

videlec opened this issue Mar 13, 2018 · 19 comments
Assignees
Labels

Comments

@videlec
Copy link

videlec commented Mar 13, 2018

with

cmake . -DCMAKE_INSTALL_PREFIX=local -DBUILD_STATIC_LIBS=OFF

I obtain

[primecount-4.2] CMake Error at lib/primesieve/cmake_install.cmake:95 (file):
[primecount-4.2]   file INSTALL cannot find
[primecount-4.2]   "/opt/sage/local/var/tmp/sage/build/primecount-4.2/src/primesieve.pc".
[primecount-4.2] Call Stack (most recent call first):
[primecount-4.2]   cmake_install.cmake:90 (include)
@kimwalisch
Copy link
Owner

Hi,

I can confirm this bug is present in primecount <= 4.2. I actually found this bug myself about a month ago, it is a bug in primesieve's CMake script. Here is the fix kimwalisch/primesieve@48557d1 in primesieve.

After I fixed the bug in primesieve I also fixed the bug in primecount by updating to the latest primesieve version. Can you please test whether make installworks for you using the latest primecount version from the master branch?

If it works for you, I will release primecount-4.3 on the weekend.

@videlec
Copy link
Author

videlec commented Mar 13, 2018

It is even worse on the main git branch

$ cmake . -DCMAKE_INSTALL_PREFIX=local -DBUILD_STATIC_LIBS=OFF
$ LANG=en make
[ 25%] Building CXX object CMakeFiles/primecount.dir/src/app/cmdoptions.cpp.o
In file included from /tmp/primecount/src/app/cmdoptions.cpp:12:0:
/tmp/primecount/src/app/cmdoptions.hpp:14:10: fatal error: int128_t.hpp: No such file or directory
 #include <int128_t.hpp>
          ^~~~~~~~~~~~~~
compilation terminated.
make[2]: *** [CMakeFiles/primecount.dir/build.make:63: CMakeFiles/primecount.dir/src/app/cmdoptions.cpp.o] Error 1
make[1]: *** [CMakeFiles/Makefile2:68: CMakeFiles/primecount.dir/all] Error 2
make: *** [Makefile:130: all] Error 2

@kimwalisch
Copy link
Owner

OK, thanks for the detailed bug report. I'll have a look at it.

@kimwalisch kimwalisch self-assigned this Mar 13, 2018
@kimwalisch kimwalisch added the bug label Mar 13, 2018
@kimwalisch
Copy link
Owner

Hi,

I have found the issue. Here are default options in primecount's CMakeLists.txt:

option(BUILD_SHARED_LIBS "Build shared libprimecount"     OFF)
option(BUILD_STATIC_LIBS "Build static libprimecount"     ON)

By default BUILD_STATIC_LIBS=ON and BUILD_SHARED_LIBS=OFF.

Your cmake command:

cmake . -DCMAKE_INSTALL_PREFIX=local -DBUILD_STATIC_LIBS=OFF

turns off both BUILD_STATIC_LIBS=OFF and BUILD_SHARED_LIBS=OFF and hence primecount fails to compile. If you add enabling BUILD_SHARED_LIBS=ON to your cmake command it should work.

cmake . -DCMAKE_INSTALL_PREFIX=local -DBUILD_STATIC_LIBS=OFF -DBUILD_SHARED_LIBS=ON

@videlec
Copy link
Author

videlec commented Mar 13, 2018

Indeed I thought that shared would be turned on by default (I should have checked). Thanks! I will try again.

@videlec
Copy link
Author

videlec commented Mar 13, 2018

make install does work. Don't you have make check/make distcheck targets?

@kimwalisch
Copy link
Owner

Don't you have a make check/make distcheck target?

make check does not exist in CMake. In CMake the command is called make test. But in order to be able to run the test suite you have to enable building the tests using:

cmake . -DBUILD_TESTS=ON
make test

@videlec
Copy link
Author

videlec commented Mar 13, 2018

But then why running make compiles the tests? Souldn't it be a separated target? (It is standard GNU practice at least)

@videlec
Copy link
Author

videlec commented Mar 13, 2018

And: tests pass on my computer!

@kimwalisch
Copy link
Owner

kimwalisch commented Mar 13, 2018

And: tests pass on my computer!

Great!

I think I will add a check to CMakeLists.txt that BUILD_STATIC_LIBS and BUILD_SHARED_LIBS cannot both be set to OFF. If you ran into this issue then other users might run into the issue as well and from the current error messages it is impossible to understand why CMake fails.

But then why running make compiles the tests? Souldn't it be a separated target? (It is standard GNU practice at least)

GNU Autotools follows the standard GNU practice but CMake is different, it follows its own standard. In CMake when testing is enabled the tests will by default be built using make. That is just the CMake way of doing things, and it is best to stick to CMake coding conventions instead of trying to replicate Autotools coding conventions in CMake.

@videlec
Copy link
Author

videlec commented Mar 13, 2018

Good. I am waiting the new release to go further with SageMath integration (see https://trac.sagemath.org/ticket/24966).

@kimwalisch
Copy link
Owner

One more question:

Does SageMath still care about big-endian CPU architectures? Currently primecount only supports little-endian CPU architectures, hence it runs on x86, x64, ARM, ARM64, IBM POWER >= 8 and so forth but not on PowerPC and Sparc.

@videlec
Copy link
Author

videlec commented Mar 13, 2018

I think it does (but I am sure we are lacking computers to actually test it).

@dimpase
Copy link

dimpase commented Mar 15, 2018

I've just tested the latest release on Sparc Solaris 11. It appears to work. However, I'd like to run its own tests in test/, but there are no instructions as to how to actually do this.

@kimwalisch
Copy link
Owner

Hi Vincent,

There is another issue for integrating primecount into sagemath, by default primecount uses the POPCNT instruction on x86 and x64. When primecount is compiled using POPCNT it won't work on x86 and x64 CPUs that have been built before 2010. I already thought about this issue in the past so there is a build option to turn off POPCNT: -DWITH_POPCNT=OFF. The bad thing about turning off POPCNT is that primecount will only run half as fast.

Currently the portable code path (that does not use POPCNT) is not much optimized, I can easily achieve a 30% speed. Personally I think this is a good trade-off, the primecount version used in sagemath will run on any CPU (maximum portability) at the expense of running 30% slower on x86 and x64 (compared to POPCNT version).

I think this sounds reasonable. Any comments or questions?

@kimwalisch kimwalisch reopened this Mar 16, 2018
@dimpase
Copy link

dimpase commented Mar 16, 2018

I don't really understand the problem. We are mostly interested in building from source.
And then you're testing for POPCNT availability, right?

The option -DWITH_POPCNT=OFF might come handy if one is building a binary distribution to work on very old hardware, and I cannot imagine much demand for this.

@kimwalisch
Copy link
Owner

kimwalisch commented Mar 16, 2018

I don't really understand the problem. We are mostly interested in building from source.
And then you're testing for POPCNT availability, right?

Yes, the choice whether primecount will use POPCNT on x86 is done at configuration time i.e. when you run cmake. I don't fully understand yet how you want to use primecount in sagemath but let's suppose you build sagemath with primecount from source on x86 and then distribute this binary package to users via the Internet. Now primecount won't work for all users with x86 CPUs older than 2010 because these CPUs don't have the POPCNT instruction.

If you don't plan to distribute primecount to end users then this may not be an issue. E.g. if you plan to install primecount on a few x86 cloud servers and you know these servers support POPCNT then of course you should enable POPCNT.

@videlec
Copy link
Author

videlec commented Mar 16, 2018

We will turn popcount off for building binaries. Thanks for mentioning that point!

@kimwalisch
Copy link
Owner

I have released primecount-4.3.

The documentation has been improved considerably i.e. there is a new "C++ library" section in the main README.md which links to libprimecount.md which contains detailed information about the build options, C++ API, linking command... Most of the documentation updates are related to your questions & suggestions.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

3 participants