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

[chimera] Add new port #10132

Closed
wants to merge 32 commits into from
Closed

Conversation

ZMZ91
Copy link

@ZMZ91 ZMZ91 commented Feb 19, 2020

Chimera is a software regular expression matching engine that is a hybrid of Hyperscan and PCRE.
https://github.com/intel/hyperscan/tree/master/chimera

@msftclas
Copy link

msftclas commented Feb 19, 2020

CLA assistant check
All CLA requirements met.

@ZMZ91
Copy link
Author

ZMZ91 commented Feb 20, 2020

hi there, one question that how to handle the conflicts if hyperscan installed as well?

ports/chimera/portfile.cmake Show resolved Hide resolved
ports/chimera/portfile.cmake Outdated Show resolved Hide resolved
@NancyLi1013 NancyLi1013 changed the title add a port for chimera [chimera] Add new port Feb 20, 2020
@NancyLi1013
Copy link
Contributor

I noticed it failed on Windows platform.
Could you please help confirm if it supports Windows?

If not, please add vcpkg_fail_port_install(ON_TARGET "Windows") and also update the related triplets to ci.baseline.txt.
If yes, please look into the regressions and try to fix them.

Thanks.

scripts/ci.baseline.txt Outdated Show resolved Hide resolved
ports/chimera/CONTROL Outdated Show resolved Hide resolved
chimera:x64-uwp=fail
chimera:arm-uwp=fail
@NancyLi1013 NancyLi1013 added the depends:upstream-changes Waiting on a change to the upstream project label Mar 4, 2020
@NancyLi1013
Copy link
Contributor

Hi @ZMZ91

Let's wait for the PR intel/hyperscan#223 merged.
Note: If chimera requires pcre to build, please use pcre that vcpkg provided.

Thanks.

@ZMZ91
Copy link
Author

ZMZ91 commented Apr 16, 2020

Hi @NancyLi1013,
Got echo from intel/hyperscan#223 which said Chimera needs to use an internal UTF8 function _pcre_valid_utf from PCRE, which requires PCRE source to been compiled with --enable-unicode-properties option, so PCRE source is required.
Referring the reply above, I'm afraid we can't cut pcre source from the installation.

@NancyLi1013
Copy link
Contributor

NancyLi1013 commented Apr 17, 2020

Hi @ZMZ91
Thanks for your above info.
We will have a discussion about this.

@NancyLi1013
Copy link
Contributor

/azp run

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@NancyLi1013
Copy link
Contributor

@ZMZ91
Sorry for the delay.

If we add --enable-unicode-properties option for pcre provided by vcpkg, can this port work?

@NancyLi1013 NancyLi1013 added the category:new-port The issue is requesting a new library to be added; consider making a PR! label May 22, 2020
@NancyLi1013
Copy link
Contributor

@ZMZ91
Is work still being done for this PR?

@ZMZ91
Copy link
Author

ZMZ91 commented Jun 22, 2020

Thanks @NancyLi1013 , sorry for delayed reply. But the option --enable-unicode-properties is not fixing it.

ports/chimera/CONTROL Show resolved Hide resolved
@JackBoosY JackBoosY added the info:reviewed Pull Request changes follow basic guidelines label Aug 5, 2020
@ZMZ91
Copy link
Author

ZMZ91 commented Aug 6, 2020

@ZMZ91 It seems that chimera is not compatible with macosx at all. I have encountered many syntaxes that clang does not support. So I am going to disable macosx build.

emmm, I don't know why, but my application runs well with chimera on my macbook.

@JackBoosY
Copy link
Contributor

For example:

/Users/vagrant/Data/buildtrees/chimera/src/b5c379159c-88a7a080a9.clean/chimera/../src/util/hash.h:60:12: error: 'auto' not allowed in function return type
    static auto has_begin_function(const C &obj) -> decltype(std::begin(obj)) {
           ^~~~

@ZMZ91
Copy link
Author

ZMZ91 commented Aug 6, 2020

For example:

/Users/vagrant/Data/buildtrees/chimera/src/b5c379159c-88a7a080a9.clean/chimera/../src/util/hash.h:60:12: error: 'auto' not allowed in function return type
    static auto has_begin_function(const C &obj) -> decltype(std::begin(obj)) {
           ^~~~

Is that something about the compiler version? I never got such issue on my macbook. I'm using CMAKE_CXX_STANDARD 17.

@JackBoosY
Copy link
Contributor

@ZMZ91
Copy link
Author

ZMZ91 commented Aug 6, 2020

@ZMZ91 Not working, see https://stackoverflow.com/questions/30665506/auto-not-allowed-in-function-prototype-with-clang

Checked on my env, the clang version is

Apple clang version 11.0.0 (clang-1100.0.33.16)
Target: x86_64-apple-darwin19.5.0
Thread model: posix
InstalledDir: /Library/Developer/CommandLineTools/usr/bin

Could you get such an env and try?

@JackBoosY
Copy link
Contributor

@ZMZ91

vcpkg@vcpkg % clang --version
Apple clang version 11.0.3 (clang-1103.0.32.62)
Target: x86_64-apple-darwin19.5.0
Thread model: posix
InstalledDir: /Library/Developer/CommandLineTools/usr/bin
vcpkg@vcpkg %

@ZMZ91
Copy link
Author

ZMZ91 commented Aug 6, 2020

ha, that's weird. With my previous package, I can install chimera successfully on both mac and linux and my app runs without problem as well.

@JackBoosY
Copy link
Contributor

@ZMZ91 Therefore, I think this issue can be left to other contributors.

@strega-nil
Copy link
Contributor

I've fixed the problem, hopefully.

@JackBoosY JackBoosY removed the info:reviewed Pull Request changes follow basic guidelines label Aug 11, 2020
@strega-nil
Copy link
Contributor

Is there a reason that this isn't just a feature of hyperscan? @ZMZ91

@ZMZ91
Copy link
Author

ZMZ91 commented Aug 17, 2020

Is there a reason that this isn't just a feature of hyperscan? @ZMZ91

Chimera is indeed a feature from hyperscan. But it requires pcre which makes it quite heavier. Ideally I think it could be built in hyperscan package and add a switch to control chimera feature on/off.

@strega-nil
Copy link
Contributor

strega-nil commented Aug 17, 2020

@ZMZ91 yeah, that's possible; you'd have, in the hyperscan CONTROL file:

Feature: chimera
Description: Add the chimera regular expression engine

and then you can check for the feature in hyperscan's portfile.cmake with "chimera" IN_LIST FEATURES.

(if you have more questions about doing this, please ping me on discord)

Copy link
Contributor

@strega-nil strega-nil left a comment

Choose a reason for hiding this comment

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

Please make this port a feature of the hyperscan port.

@NancyLi1013
Copy link
Contributor

@ZMZ91
Could you please add chimera as a feature for hyperscan?

@NancyLi1013
Copy link
Contributor

@ZMZ91
Could you please handle the conflicts and address the review suggestions?

@NancyLi1013
Copy link
Contributor

Pinging @ZMZ91 for response. Is work still being done for this PR?

@NancyLi1013
Copy link
Contributor

Thanks for the PR; we're closing this for now since there's been no response. If you'd like to continue working on it, please reopen and ping us!

@NancyLi1013 NancyLi1013 closed this Dec 4, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
category:new-port The issue is requesting a new library to be added; consider making a PR!
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants