Page MenuHomeFreeBSD

devel/hyperscan: Install shared library, Modernize
AbandonedPublic

Authored by koobs on Aug 1 2016, 2:37 PM.
Tags
None
Referenced Files
Unknown Object (File)
Thu, Jan 23, 12:52 PM
Unknown Object (File)
Jan 3 2025, 6:13 AM
Unknown Object (File)
Jan 3 2025, 6:13 AM
Unknown Object (File)
Jan 3 2025, 5:18 AM
Unknown Object (File)
Dec 12 2024, 12:27 PM
Unknown Object (File)
Oct 4 2024, 5:59 AM
Unknown Object (File)
Oct 4 2024, 1:59 AM
Unknown Object (File)
Oct 1 2024, 5:45 PM

Details

Reviewers
vsevolod
Summary
devel/hyperscan: Install shared library, Modernize

suricata 3.1 added support for Hyperscan, but requires linking against
its shared library. The current version of this port has a SHARED option
which is not enabled by default.

This prevents users of the suricata (and other software from having
hyperscan support, which is desirable by default. In its default
configuration (not in OPTIONS_DEFAULT), it also prevents Ports users
from having hyperscan support, without hunting for the reason why,
creating a poor user experience.

It is a reasonable expectation that software that can provide a shared 
library for consumption & linking by other software, should install
that library by default, particularly in the context of FreeBSD Ports
and most other packaging systems, where linking against shared (vs
static) libraries is a de-facto, if not a required standard or
expectation.

- Remove the SHARED option accordingly.
- Ask (CMAKE) for both the static and shared libraries to be built.
- Add USE_LDCONFIG.
- Update pkg-plist accordingly.

While I'm here:

- Remove core2 from C{XX}FLAGS when NATIVE is off. 
  
  It is overly prescriptive for a global default, and can be overriden
  by user-supplied CFLAGS, defeating it's purpose. If the aim was to
  provide optimized builds, perhaps these flags would be better suited
  to sub-packages tailored to particular CPU's or Archs (core2, etc).
  
  Instead of the above, patch CmakeLists.txt to add an option
  (WITH_NATIVE) to build with -m{arch,tune}=native when enabled, and not
  fiddle with *FLAGS when it isn't enabled instead. This could be
  upstreamed. It shouldn't be the default. Remove NATIVE_C{XX}FLAGS
  options helpers accordingly, as it's already done by CmakeLists.txt.
  
  In addition:
  
  Add -mssse3 to C{XX}FLAGS instead and add a pre-install: conditional
  check for the CPU feature using MACHINE_CPU, setting BROKEN if the
  feature is not detected. (See Also: multimedia/ffmpeg)
  
  Remove ONLY_FOR_ARCHS_amd64 to allow building on other architectures.
  
- Use USES=pathfix instead of post-stage: foo for s/lib/libdata fix.
- Remove custom DEBUG check, USES=cmake handles build targets/stripping.
- Add patch to CMakeLists.txt adding WITH_EXAMPLES as an option.
- Use OPTIONS helpers for the NATIVE option.
- Update NATIVE_DESC and ONLY_FOR_ARCHS_REASON verbiage.
- Repatch patch files (makepatch compliant).
- Add LICENSE_FILE.
- Sort and group USE{S} entries.
- Sort pkg-plist.
- Add test target.

PR:		211002
Reviewed_by:    vsevolod (maintainer), mat
Approved_by:    vsevolod (maintainer)
DiffRev:        DXXXX
MFH:            2016Q3
Test Plan
  • portlint: OK (false positive WARN)
    • WARN: Makefile: NATIVE is listed in OPTIONS_DEFINE, but no PORT_OPTIONS:MNATIVE appears
    • portlint missing CMAKE_BOOL in list of option helper vars to check
  • porttest: OK (poudriere)
    • 93amd64 (gcc48)
    • 103amd64 (clang)
    • 103i386 (clang)
    • 11amd64 (clang)
  • unittest: OK
    • 2577 tests from 30 test cases ran. (910856 ms total)
    • [ PASSED ] 2577 tests.

Diff Detail

Repository
rP FreeBSD ports repository
Lint
No Lint Coverage
Unit
No Test Coverage
Build Status
Buildable 4674
Build 4728: arc lint + arc unit

Event Timeline

koobs retitled this revision from to devel/hyperscan: Install shared library, Modernize.
koobs updated this object.
koobs edited the test plan for this revision. (Show Details)
koobs added a reviewer: vsevolod.
devel/hyperscan/Makefile
32

I'll fix this

devel/hyperscan/Makefile
46–49

I don't understand why this is in a target, it should be by itself.

devel/hyperscan/Makefile
46–49

The aim was to prevent installation, the narrowest limitation i could think of, because runtime wont work on a machine without SSSE3 instructions, but one can built it without issue.

What's a better way?

devel/hyperscan/Makefile
46–49

What's the point of allowing building of something you can't use ?

devel/hyperscan/Makefile
46–49

Also, this does not do what you think it does.

This will create a pre-install target that does one thing, run a variable assignment in its shell, but it does not define BROKEN in the Makefile.

devel/hyperscan/Makefile
46–49

Cross building or building packages on the cluster (which we obviously dont want to impact) for a wider class of host CPU's (that do have the SSSE3 instructions)

What's the best way to achieve this?

devel/hyperscan/Makefile
46–49

So, you want to handle the convoluted case where the package building machine does not have SSE3 but the machine using the package has, while not allowing the building machine without SSE3 to install it.

Then echo the message in the pre-install target, and end it with ${FALSE}, also, enclose the pre-install: line in the .if so that you don't end up with an empty target. But let me say this, it is very very ugly.

Now, SSE3 arrived like 12 years ago, I'd say not doing that but just adding a pkg-message saying "you need SSE3 for this to work" would be better.

From what we can tell, especially the "illegal instruction trouble" (see below), we need a suricata-hs package that adds hyperscan without tainting the original package and make the new one amd64 only.

Or leave it as a non-default option for the brave ports builders out there.

I don't see how the hyperscan package can be improved without them stopping to require at least SSE3.

Also see: https://github.com/opnsense/core/issues/1244

Cheers,
Franco

@franco_opnsense.org The rest of these changes have maintainer approval and are pending commit (by me). I'll consider the idea of a separate port/package against the WIP port I have locally (which enables packages on supported archs by default)

I'm not doubting these changes, I just saw that enabling hyperscan for suricata is problematic for non SSE3 archs and there's no way around this :/

Speaking of which, I'll drop a patch for 3.1.3 into the bug tracker in a second. :)