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

Simplest change to change from pcre to pcre2 #54

Open
wants to merge 3 commits into
base: master
Choose a base branch
from
Open

Simplest change to change from pcre to pcre2 #54

wants to merge 3 commits into from

Conversation

paulrho
Copy link
Contributor

@paulrho paulrho commented Nov 22, 2021

No description provided.

@paulrho paulrho mentioned this pull request Nov 22, 2021
Open
gregoa
gregoa reviewed Nov 22, 2021
View reviewed changes
configure.ac Outdated
@@ -15,7 +16,8 @@ AC_PATH_PROG([XSLTPROC], [xsltproc])
PKG_PROG_PKG_CONFIG()

PKG_CHECK_MODULES(LIBOAUTH, oauth)
PKG_CHECK_MODULES(LIBPCRE, libpcre)
#PKG_CHECK_MODULES(LIBPCRE2, libpcre2)
PKG_CHECK_MODULES(PCRE2, [libpcre2-8 libpcre2-32])
Copy link
Contributor

Choose a reason for hiding this comment

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

Just curious: why -8 and -32?
On my Debian/unstable system I can see:

/usr/lib/x86_64-linux-gnu/pkgconfig/libpcre2-16.pc
/usr/lib/x86_64-linux-gnu/pkgconfig/libpcre2-32.pc
/usr/lib/x86_64-linux-gnu/pkgconfig/libpcre2-8.pc
/usr/lib/x86_64-linux-gnu/pkgconfig/libpcre2-posix.pc

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I've removed the -32

@gregoa
Copy link
Contributor

gregoa commented Nov 22, 2021

Alright, I added this PR as a patch to the Debian package, and unfortunately the build fails at the linker stage:

gcc -I/usr/include/x86_64-linux-gnu -I/usr/include/libxml2 -I/usr/include/json-c  -I/usr/include/x86_64-linux-gnu -I/usr/include/nss -I/usr/include/nspr -g -O2 -ffile-prefix-map=/build/bti-034=. -fstack-protector-strong -Wformat -Werror=format-security  -Wl,-z,relro -Wl,-z,now -o bti bti-config.o bti-bti.o -lcurl -lxml2 -ljson-c  -loauth -ldl 
/usr/bin/ld: bti-bti.o: in function `find_urls':
/tmp/buildd/bti-034/bti.c:1298: undefined reference to `pcre2_compile_8'
/usr/bin/ld: /tmp/buildd/bti-034/bti.c:1310: undefined reference to `pcre2_match_data_create_from_pattern_8'
/usr/bin/ld: /tmp/buildd/bti-034/bti.c:1312: undefined reference to `pcre2_match_8'
/usr/bin/ld: /tmp/buildd/bti-034/bti.c:1322: undefined reference to `pcre2_get_ovector_pointer_8'
/usr/bin/ld: /tmp/buildd/bti-034/bti.c:1339: undefined reference to `pcre2_code_free_8'
collect2: error: ld returned 1 exit status

If I manually add -lpcre2-8 at the end of the gcc call, it works.

Looks like there's some more autoconf/automake magic needed to get the needed flags in the needed places.

@paulrho
Copy link
Contributor Author

paulrho commented Nov 22, 2021

Okay, try again (pull request is automatically updated with this correction correction).
I'd modified the Makefile, but not the Makefile.am - now fixed.
I've checked that it builds on a fresh Fedora machine.

paulrho
paulrho commented Nov 22, 2021
View reviewed changes
Copy link
Contributor Author

@paulrho paulrho left a comment

Choose a reason for hiding this comment

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

Builds and runs without the -32

configure.ac Outdated
@@ -15,7 +16,8 @@ AC_PATH_PROG([XSLTPROC], [xsltproc])
PKG_PROG_PKG_CONFIG()

PKG_CHECK_MODULES(LIBOAUTH, oauth)
PKG_CHECK_MODULES(LIBPCRE, libpcre)
#PKG_CHECK_MODULES(LIBPCRE2, libpcre2)
PKG_CHECK_MODULES(PCRE2, [libpcre2-8 libpcre2-32])
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I've removed the -32

@gregoa
Copy link
Contributor

gregoa commented Nov 22, 2021

Thanks! This looks good indeed.
I'm able to build the package, and I can even send a tweet with it:
https://twitter.com/gregoa_/status/1462895918596820995
:)

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

@gregoa gregoa gregoa left review comments

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@paulrho @gregoa