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

pcre2test: really allow using libedit when enabled #96

Merged
merged 3 commits into from
Apr 8, 2022

Conversation

carenas
Copy link
Contributor

@carenas carenas commented Apr 7, 2022

This series fixes several issues found while testing PCRE2 in FreeBSD

The first one does the minimun possible change to expand the current
approach and could be simpified further, but those changes had been
punted for now, except for the one that is proposed in the last patch.

The second patch fixes the building issue for FreeBSD and is therefore
the more extensive, but the problem addressed is also affecting other
platforms, and also the configuration with autotools.

The last patch is the one with the highest potential for regressions, as
it would prevent building in systems that had libedit masquerading as
readline (ex: macOS) AND where the option to use libedit was requested
but in those cases enabling readline (which is the default with cmake)
instead works as expected, since in order to masquerade one library
with the other links for the header files and aliases for the library need
to be provided.

@carenas carenas marked this pull request as draft April 7, 2022 18:56
@carenas carenas marked this pull request as ready for review April 7, 2022 19:36
@carenas carenas changed the title pcre2test: really allow libedit with cmake pcre2test: really allow using libedit when enabled Apr 8, 2022
@carenas carenas marked this pull request as draft April 8, 2022 02:09
When `./configure --enable-pcre2test-libedit` is used in FreeBSD,
the resulting test will succeed but won't set the necessary flag
to distinguish between libedit and readline header files, therefore
using readline's at built time (if installed)

Consolidate all header tests into one and use instead the corresponding
autogenerated defines to check for each possibility.
carenas added 2 commits April 7, 2022 20:33
Using cmake to configure and enable linking pcre2test with libedit,
could result in a broken build, because the header used was instead
pointing to readline.

In cases were the build will succeed (because both libraries were
available), it would likely show warnings, because several history
functions were being used without declarations, since readline
requires including "history.h" for those.

Additionally, since PCRE2_SUPPORT_READLINE is ON by default (unlike
configure), turning PCRE2_SUPPORT_LIBEDIT=ON, would require setting
that other option to OFF explicitly (even if readline wasn't available)
or the setup would abort.

Lastly, in systems with no default sysroot (ex: macOS), the use of
absolute paths for searching for libedit's readline.h could fail so
use instead relative PATH_SUFFIXES.
When asked to enable libedit in a system that ALSO has readline,
the headers of the former would be found and used by the earlier.

While that would mostly work, some functions will be missing
definitions (which is forbidden in C99), so instead abort the
configuration and let the user provide for them.
@carenas carenas marked this pull request as ready for review April 8, 2022 03:58
@PhilipHazel PhilipHazel merged commit 9c8abdd into PCRE2Project:master Apr 8, 2022
PhilipHazel added a commit that referenced this pull request Apr 8, 2022
@PhilipHazel
Copy link
Collaborator

I don't really know much about Autotools, still less about CMake; these patches work for me so I have done the merge. Thanks for working in these areas.

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.

2 participants