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

Remove dependency on libpcre++ #56

Merged
merged 1 commit into from
Nov 9, 2018
Merged

Conversation

jtsylve
Copy link
Contributor

@jtsylve jtsylve commented Nov 9, 2018

C++ has had support for simple regex matching since C++11. This PR removes the dependencies on both libpcre and libpcre++ by using the standard library implementation.

@scudette
Copy link
Collaborator

scudette commented Nov 9, 2018

It would be great to get rid of this dependency. I think we had it in there because some older compilers do not actually support regex but in a terrible way (as in they just return false for regex_match).

https://stackoverflow.com/questions/12530406/is-gcc-4-8-or-earlier-buggy-about-regular-expressions

This leads to runtime failures and it is harder to detrect at build time. It seems this has been fixed since gcc4.9 - is it possible to add a check in autoconf for it? It is fine to totally break building on older compilers.

@jtsylve
Copy link
Contributor Author

jtsylve commented Nov 9, 2018

That is crazy, but yeah, that's a very old version of gcc. I'm sure it's possible to add some sort of compile-time check, but I'm not sure how to go about that. Also the check is only actually needed when compiling for Windows, but the current configuration enforces the dependency on all systems.

I bet you can just throw an error during config if compiling with mingw with gcc version < 4.9.

Since all of this effort is for this single check, I wonder if regex is overkill here. Maybe a simpler check can just be used?

@scudette
Copy link
Collaborator

scudette commented Nov 9, 2018

Can you check if your change makes a working windows binary? I wonder if the mingw dependency in the docker file works. If the docker file works out the box then I vote to just go with it - we can just say the docker file is the supported build env and not worry too much about checking for old tools.

@jtsylve
Copy link
Contributor Author

jtsylve commented Nov 9, 2018

It makes a working windows binary with mingw-64 with gcc version 8.2.0, but I haven't tried the docker version.

I'm all in favor for not supporting ancient compilers.

@scudette
Copy link
Collaborator

scudette commented Nov 9, 2018

Awesome! We can always up the version in docker if we need to (although I think it is already current).

@scudette scudette merged commit fcbaf85 into Velocidex:master Nov 9, 2018
@jtsylve jtsylve deleted the std_regex branch February 8, 2019 03:50
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