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

libcmark.pc: use CMAKE_INSTALL_LIBDIR #185

Merged
merged 1 commit into from
Feb 5, 2017
Merged

Conversation

juhp
Copy link
Contributor

@juhp juhp commented Feb 5, 2017

needed for multilib distros like Fedora

See https://bugzilla.redhat.com/show_bug.cgi?id=1416426

Particularly with pkgconf.

needed for multilib distros like Fedora
@jgm jgm merged commit 118ebb3 into commonmark:master Feb 5, 2017
@jgm
Copy link
Member

jgm commented Feb 5, 2017

Thanks

@juhp juhp deleted the patch-1 branch February 6, 2017 02:01
@jgm
Copy link
Member

jgm commented Oct 9, 2017

@juhp How is CMAKE_INSTALL_LIBDIR supposed to be set? It doesn't seem to be set to lib by default, and this has broken the default build (see #236).

jgm added a commit that referenced this pull request Oct 10, 2017
For some reason this wasn't getting set in processing
libcmark.pc.in, and we were getting the wrong entry in libcmark.pc.
(See #236)

The new approach sets an internal libdir variable to
lib${LIB_SUFFIX}.  This variable is used both to set the
install destination and in the libcmark.pc.in template.

Closes #236.

However, I'd welcome comments from @juhp who originally
added CMAKE_INSTALL_LIBDIR in #185.  I think that the new
system should work fine with Fedora, since LIB_SUFFIX will
be set appropriately, but some testing is in order.
@juhp
Copy link
Contributor Author

juhp commented Oct 11, 2017

Hmm, I am not sure to be honest - I had assumed that cmake would default it to lib.
I am not a cmake aficionado though - perhaps there is a better more correct way of doing this.

On Fedora at least (Linux?) it seems to default to the right thing at least, maybe not on MacOS?

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.

None yet

2 participants