Skip to content

Fix cmake when custom CMAKE_INSTALL_LIBDIR is given #4235

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

Merged
merged 1 commit into from
Jun 6, 2018

Conversation

sifmelcara
Copy link
Contributor

@sifmelcara sifmelcara commented Jun 6, 2018

According to cmake doc, we cannot assume CMAKE_INSTALL_LIBDIR is a relative path. People often use cmake -DCMAKE_INSTALL_LIBDIR=/some/custome/library/path to specify where the libraries should install to.

Simply use lib as temporary libdir is probably a solution.

Inconsistent CMAKE_INSTALL_LIBDIR problem addressed by @aarlt and @chriseth ( #3467 (comment) ) is fixed by passing -DCMAKE_INSTALL_LIBDIR=lib to jsoncpp's cmake.

We don't need GNUInstallDirs here because jsoncpp's build is only temporary used anyway.

@chfast could you take a look? Thanks so much

According to cmake documents, we cannot assume CMAKE_INSTALL_LIBDIR is a
relative path. This commit fixes the "no rule to make libjsoncpp.a"
error by passing -DCMAKE_INSTALL_LIBDIR=lib to jsoncpp external project.
@chfast chfast requested review from chfast and chriseth June 6, 2018 07:58
@chfast
Copy link
Member

chfast commented Jun 6, 2018

Yes, this is good solution. Exactly the same I proposed 2 days ago in #4221.

@chriseth chriseth merged commit 59b35fa into ethereum:develop Jun 6, 2018
@sifmelcara
Copy link
Contributor Author

sifmelcara commented Jun 6, 2018

Wow.. What a coincidence. guess I will close this PR as your are more complete
Edit: seems merged, thanks.

resonant-riches added a commit to ChorusOne/azimuth-cli that referenced this pull request Mar 4, 2024
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.

3 participants