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.

Sign up for GitHub

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

Jump to bottom

MDEV-29955: add ${PCRE_LIBRARY_PATH} to link directories #3225

Open
wants to merge 1 commit into
base: 10.6
Choose a base branch
from
Open

MDEV-29955: add ${PCRE_LIBRARY_PATH} to link directories #3225

wants to merge 1 commit into from

Conversation

keeper
Copy link
Contributor

@keeper keeper commented Apr 26, 2024

  • The Jira issue number for this PR is: MDEV-29955

Description

Commit 3a33ae8 uses pkg-config to find the location of PCRE library.
However when a user installs the library in non-standard location
(/workspace/mylib for example), it's also recommended to use the
'-Wl,-rpath -Wl,LIBDIR' linker flag to point the build system to where
the library installed.

How can this PR be tested?

  • Builds with no errors with PCRE installed in custom path

Manual checks:

  1. pkg-config:
$ pkg-config --variable=libdir libpcre2-8
/workspace/mylib/lib
$ pkg-config --variable=libdir libpcre2-posix
/workspace/mylib/lib
  1. Generated CMake build file
$ cmake -S . -B build -G Ninja --fresh -DWIHT_PCRE=system
...
...
$ grep LINK_LIBRARIES build/build.ninja
...
...
LINK_LIBRARIES = -Wl,-rpath,/workspace/mylib/lib: ...

Basing the PR against the correct MariaDB version

  • This is a bug fix and the PR is based against the earliest maintained branch in which the bug can be reproduced.

PR quality check

  • I checked the CODING_STANDARDS.md file and my PR conforms to this where appropriate.
  • For any trivial modifications to the PR, I am ok with the reviewer making the changes themselves.

@keeper
MDEV-29955: add ${PCRE_LIBRARY_PATH} to link directories
4ed9c71 
Commit 3a33ae8 uses pkg-config to find the location of PCRE library.
However when a user installs the library in non-standard location
(/workspace/mylib for example), it's also recommended to use the
'-Wl,-rpath -Wl,LIBDIR' linker flag to point the build system to where
the library installed.

All new code of the whole pull request, including one or several files
that are either new files or modified ones, are contributed under the
BSD-new license. I am contributing on behalf of my employer Amazon Web
Services.
@CLAassistant
Copy link

CLA assistant check
Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you sign our Contributor License Agreement before we can accept your contribution.
You have signed the CLA already but the status is still pending? Let us recheck it.

LinuxJedi
LinuxJedi approved these changes Apr 26, 2024
View reviewed changes
Copy link
Contributor

@LinuxJedi LinuxJedi left a comment

Choose a reason for hiding this comment

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

Looks like a good catch to me.

vuvova
vuvova reviewed Apr 26, 2024
View reviewed changes
cmake/pcre.cmake
@@ -85,6 +85,7 @@ MACRO (CHECK_PCRE)
PKG_CHECK_MODULES(PCRE libpcre2-8)
# in case pkg-config or libpcre2-8.pc is not installed:
CHECK_LIBRARY_EXISTS(pcre2-8 pcre2_match_8 "${PCRE_LIBRARY_DIRS}" HAVE_PCRE2_MATCH_8)
link_directories(${PCRE_LIBRARY_DIRS})
Copy link
Member

Choose a reason for hiding this comment

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

does it work "in case pkg-config or libpcre2-8.pc is not installed" ?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@vuvova vuvova vuvova left review comments

@LinuxJedi LinuxJedi LinuxJedi approved these changes

Assignees
No one assigned
Labels
None yet
Milestone
No milestone
4 participants
@keeper @CLAassistant @LinuxJedi @vuvova