Skip to content

Small packaging improvements #93

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 2 commits into from
Dec 2, 2014
Merged

Small packaging improvements #93

merged 2 commits into from
Dec 2, 2014

Conversation

akien-mga
Copy link
Contributor

About commit bf0312c, I assumed that undefined variables in CMake were already interpreted as an empty string, but that might be worth checking.
To make sure LIB_SUFFIX is empty when undefined, you might add something like:

IF(NOT DEFINED LIB_SUFFIX)
    SET(LIB_SUFFIX "")
ENDIF(NOT DEFINED LIB_SUFFIX)

@akien-mga
Copy link
Contributor Author

BTW those changes would also be relevant for the pre-C++11 branch.

@cdunn2001
Copy link
Contributor

I'm afraid I don't fully understand this change, having little experience using cmake for static/dynamic libraries as variant builds. In general, shared libs go into the same 'lib' directory as static archives. Does this change affect only the build, or the installation too? I mean, if someone wants to install the shared libs into /usr/local/lib, will that still be possible?

@akien-mga
Copy link
Contributor Author

The purpose is to install both static and shared libs in $(CMAKE_INSTALL_PREFIX)/lib$(LIB_SUFFIX).
I did not change the output directories for the build, only the ones for system wide installation.

On Debian-based distros, you will usually put static/shared libraries in /usr/local/lib (or /usr/lib for official packages), while on RPM-based distros the 64bit libraries should go to /usr/local/lib64, hence the need for a LIB_SUFFIX variable that RPM packagers will set to "64".

If LIB_SUFFIX is not defined, nothing will change compared to what you currently use.

RPM-based distros such as Fedora or Mageia put 64bit libraries in /usr/lib64
while 32bit libraries go to /usr/lib. This is usually taken into account
in CMake projects using a LIB_SUFFIX parameter that can be set to "" or "64".
BUILD_SHARED_LIBS is a standard CMake argument that serves the purpose
of the custom JSONCPP_LIB_BUILD_SHARED. For now we force JSONCPP_LIB_BUILD_SHARED
to true if BUILD_SHARED_LIBS was defined.
Workaround for #51.
@akien-mga
Copy link
Contributor Author

I rebased my pull request to add two small modifications:

  • The LIB_SUFFIX variable is now set to an empty string as a default value, and it will be cached, i.e. it will appear as a configurable variable in cmake-gui for example. It can also be overridden by the user thanks to the CACHE parameter.
  • Added a mention to issue Please use standard CMake variable to configure static vs. shared builds #51 in 27639ce. (Note that the solution mentioned in this issue might be better than mine, that's why I qualified mine as a "workaround". It doesn't hurt though).

cdunn2001 added a commit that referenced this pull request Dec 2, 2014
Small packaging improvements
@cdunn2001 cdunn2001 merged commit 9ca1aaa into open-source-parsers:master Dec 2, 2014
@cdunn2001
Copy link
Contributor

Oh! I thought LIB_SUFFIX was for dynamic versus static, but it's for 64-bit of course! Thanks.

@akien-mga
Copy link
Contributor Author

You're welcome. Do you want me to make a similar pull request for the pre-C++11 branch, or will you handle that?

@cdunn2001
Copy link
Contributor

We're only doing bug-fixes on that branch, unless you think this is really important.

@akien-mga
Copy link
Contributor Author

It's not primordial, I already patched it locally to update Mageia's jsoncpp package.

@cdunn2001
Copy link
Contributor

Ok, if you can validate it for us, then we might as well include it on that branch too.

@akien-mga
Copy link
Contributor Author

I can confirm that it works fine on that branch too, it just needs manual merging since the pkgconfig file is slightly different there. Do you want me to prepare a pull request against the pre-C++11 branch?

@cdunn2001
Copy link
Contributor

Yes, go ahead. If you want, rebase this branch and you can re-use this pull-request, I think.

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