Page MenuHomeFreeBSD

archivers/brotli: Update to 0.6.0
ClosedPublic

Authored by brnrd on Jun 21 2017, 8:48 AM.

Details

Summary
archivers/brotli: Update to 0.6.0

 - Update brotli to 0.6.0
 - Move from devel/libbrotli
 - No longer requires the meta project
 - Switch from autoconf to cmake
 - Fix nginx brotli module

PR: 218851
Submitted by:  Markus Kohlmeyer <rootservice@gmail.com>
Reviewed_by:
Approved by:
Differential_Revision

Diff Detail

Repository
rP FreeBSD ports repository
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.

Event Timeline

archivers/brotli/Makefile
6 ↗(On Diff #29904)

PORTEPOCH.

15 ↗(On Diff #29904)

DISTVERSIONPREFIX.

24 ↗(On Diff #29904)

Having a static option enabling static libs, ok, why not, but disabling the shared libs, no. Unless you provide a slave port that installs the static libs.

@mat See inline comment

archivers/brotli/Makefile
24 ↗(On Diff #29904)

I think this is a problem of brotli itself - it's not possible to have both types of libraries. See:
https://github.com/google/brotli/issues/542
and
https://bugs.freebsd.org/bugzilla/show_bug.cgi?id=218851#c9

archivers/brotli/Makefile
24 ↗(On Diff #29904)

Then create a archivers/brotli-static with and use OPTIONS_EXCLUDE=STATIC/OPTIONS_SLAVE=STATIC

archivers/brotli/Makefile
24 ↗(On Diff #29904)

Or just do not provide an option to get static libs, nobody should ever be using them.

Set PORTEPOCH

Even though we're moving to a different name, we're going
back in version 1.0 -> 0.6.0

brnrd added inline comments.
archivers/brotli/Makefile
24 ↗(On Diff #29904)

Agree. This was on request of osa@ (see PR). Gone now.
Behavior of brotli cmake is also questionable, it is either or, not both. (if you only disable DYNAMIC, you don't get static either but still dynamic)

brnrd marked an inline comment as done.

Remove STATIC option as per portmgr request

Looks good, now all you have to do is get maintainer approval.

Successfully built and installed on FBSD103, together with Apache 2.4.26.

This revision is now accepted and ready to land.Jun 23 2017, 1:46 PM
brnrd edited edge metadata.

Add patch for nginx

  • Update nginx brotli module

As far as I can tell nginx is the only user of devel/libbrotli

This revision now requires review to proceed.Jun 23 2017, 7:32 PM

Add archivers/brotli-static port and www/nginx-devel update

  • Add static port on maintainers request
  • Update nginx-devel port brotli module

Really add archivers/brotli-static

Add archivers/brotli-static port and www/nginx-devel update

  • Add static port on maintainers request
  • Update nginx-devel port brotli module

The only other dependency I've found is in mail/cyrus-imapd30 where it is commented out, with the new archivers/brotli this builds fine

-# Our libbrotli is too old
-#HTTP_LIB_DEPENDS+=    libbrotlidec.so:devel/libbrotli
+HTTP_LIB_DEPENDS+=     libbrotlicommon.so:archivers/brotli
archivers/brotli-static/Makefile
10 ↗(On Diff #30026)

So, this conflicts with itself, nice.

archivers/brotli/Makefile
19 ↗(On Diff #30026)

So, mmm, this has OPTIONS_EXCLUDE=STATIC and OPTIONS_SLAVE=STATIC, I figure this is undefined behavior.

brnrd marked an inline comment as done.

Update STATIC options and conflicts

brnrd added inline comments.
archivers/brotli-static/Makefile
10 ↗(On Diff #30026)

That was pretty stupid indeed!

archivers/brotli/Makefile
19 ↗(On Diff #30026)

It was my impression that this was how it works, _EXCLUDE in master- and _SLAVE in slave-port.
Noticed there's no documentation in porter's handbook.

Could you not rewrite other port's Makefiles ? most of nginx-devel should not be in here.

brnrd marked an inline comment as done.

Remove nginx patches

Also, is there any need for the static stuff ? Using static libraries is a pain in the ports tree. Also ELF has been here for 20 years, nobody should be using or providing static libs, ever.

archivers/brotli-static/Makefile
10 ↗(On Diff #30576)

Simply write:

CONFLICTS_INSTALL= brotli

archivers/brotli/Makefile
15 ↗(On Diff #30576)

This should be ?=, and also only brotli-static.

20 ↗(On Diff #30576)

So one can ask the brotli port to build as STATIC, this is not really great. Maybe simply drop options and use a .if defined(STATIC_ONLY) / .else

This revision was automatically updated to reflect the committed changes.

Committed only the shared-lib version.
Maintainer of the port (who requested static option) didn't provide feedback (via mail)

Also, you forgot to commit fixes for ports that depend on this.