Bug 224000 - www/nginx: Update brotli module
Summary: www/nginx: Update brotli module
Status: Closed FIXED
Alias: None
Product: Ports & Packages
Classification: Unclassified
Component: Individual Port(s) (show other bugs)
Version: Latest
Hardware: Any Any
: --- Affects Some People
Assignee: Jochen Neumeister
URL:
Keywords: needs-qa, patch
Depends on:
Blocks:
 
Reported: 2017-11-30 20:26 UTC by Bernard Spil
Modified: 2017-12-05 02:09 UTC (History)
4 users (show)

See Also:
bugzilla: maintainer-feedback? (joneum)


Attachments
svn diff for www/nginx (5.87 KB, patch)
2017-11-30 20:26 UTC, Bernard Spil
no flags Details | Diff
svn diff for www/nginx (4.62 KB, patch)
2017-11-30 20:47 UTC, Bernard Spil
brnrd: maintainer-approval?
Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Bernard Spil freebsd_committer 2017-11-30 20:26:01 UTC
Created attachment 188433 [details]
svn diff for www/nginx

We have an update bug #223966 for brotli pending  (required for cURL update) that breaks nginx brotli module.
The upstream brotli module hasn't been updated for over a year even though it's been broken for over a year with later brotli releases. There's a fork that does update which uses the new brotli abi. This patch is based on the fork. Requires some patching to the config file as it depends on an old upstream brotli snapshot to be in the src dir.

```
www/nginx: Update brotli module

 - Switch to fork that uses new brotli ABI

PR:
```
Comment 1 Bernard Spil freebsd_committer 2017-11-30 20:40:17 UTC
Upstream patch here https://github.com/eustas/ngx_brotli/pull/1
Comment 2 Bernard Spil freebsd_committer 2017-11-30 20:47:12 UTC
Created attachment 188434 [details]
svn diff for www/nginx

Updated with upstreamed patch. Cleaner.
Comment 3 Sergey A. Osokin freebsd_committer 2017-11-30 23:49:38 UTC
Hi Bernard,

thanks for the patch.

I think we can simplify this update with the 9891a98811a5d301041298a6507de8acc498e52a commit, committed as the
https://github.com/google/ngx_brotli/pull/63/commits pull request.

I'd recommend to add the patch as a third-party patch for the ${PATCHDIR} of the www/nginx and www/nginx-devel ports.
Comment 4 Bernard Spil freebsd_committer 2017-12-01 06:53:35 UTC
Hi Sergey,

That patch is indeed the same, comes from the same fork. That doesn't solve the issue with building against an external brotli.

Bernard.
Comment 5 Bernard Spil freebsd_committer 2017-12-01 13:43:36 UTC
(In reply to Sergey A. Osokin from comment #3)
The pull request was abandoned
https://github.com/google/ngx_brotli/pull/63#issuecomment-348463499
suggest we take the fork. Otherwise you'll have to provide the old brotli snapshot in-source in the tree for the module to use.
Comment 6 commit-hook freebsd_committer 2017-12-01 22:53:15 UTC
A commit references this bug:

Author: joneum
Date: Fri Dec  1 22:52:28 UTC 2017
New revision: 455319
URL: https://svnweb.freebsd.org/changeset/ports/455319

Log:
  www/nginx: Update brotli module

  - Switch to fork that uses new brotli ABI

  The upstream brotli module hasn't been updated for over a year even
  though it's been broken for over a year with later brotli releases.
  There's a fork that does update which uses the new brotli abi.
  This patch is based on the fork. Requires some patching to the config
  file as it depends on an old upstream brotli snapshot to be in the src dir.

  PR:		224000
  Reported by:	brnrd
  Approved by:	tcberner (mentor)
  Differential Revision:	https://reviews.freebsd.org/D13319

Changes:
  head/www/nginx/Makefile
  head/www/nginx/distinfo
  head/www/nginx/files/extra-patch-brotli_config
Comment 7 Philipp Engel 2017-12-03 08:13:12 UTC
Patching failes:

===>  Patching for nginx-1.12.2_2,2                                                                                                                                
===>  Applying extra patch /usr/ports/www/nginx/files/extra-patch-brotli_config                                                                                    
No file to patch.  Skipping...                                                                                                                                     
3 out of 3 hunks ignored--saving rejects to ../ngx_brotli-9891a98/config.rej                                                                                       
Can't create ../ngx_brotli-9891a98/config.rej, output is in /tmp/patchr8OdL4ys32T: No such file or directory                                                       
*** Error code 1                                                                                                                                                   
                                                                                                                                                                   
Stop.                                                                                                                                                              
make: stopped in /usr/ports/www/nginx                                                                                                                              
=>> Cleaning up wrkdir                                                                                                                                             
===>  Cleaning for nginx-1.12.2_2,2                                                                                                                                
build of www/nginx | nginx-1.12.2_2,2 ended at Sat Dec  2 19:17:00 CET 2017                                                                                        
build time: 00:00:05                                                                                                                                               
!!! build failure encountered !!!
Comment 8 Bernard Spil freebsd_committer 2017-12-03 09:40:30 UTC
The updated port uses a newer commit hash than the one that was in the patch here. The newer commit hash includes the fix that allows building with an external libbrotli.

Checking at the moment, but it seems safe to just remove files/extra-patch-brotli_config
Comment 9 commit-hook freebsd_committer 2017-12-03 09:48:37 UTC
A commit references this bug:

Author: brnrd
Date: Sun Dec  3 09:48:02 UTC 2017
New revision: 455397
URL: https://svnweb.freebsd.org/changeset/ports/455397

Log:
  www/nginx: Unbreak brotli option

   - The used commit hash includes the patch

  PR:		224000
  Reported by:	Philipp <kidon posteo de>

Changes:
  head/www/nginx/Makefile
  head/www/nginx/files/extra-patch-brotli_config
Comment 10 Bernard Spil freebsd_committer 2017-12-03 09:50:25 UTC
For the record.

After submitting the patch here, I discovered that the author of the fork (eustas) is the maintainer of the brotli module for google.
Comment 11 Philipp Engel 2017-12-03 09:51:11 UTC
(In reply to Bernard Spil from comment #8)
Removing the patch from the Makefile solves the issue.
Comment 12 commit-hook freebsd_committer 2017-12-05 02:09:38 UTC
A commit references this bug:

Author: osa
Date: Tue Dec  5 02:09:22 UTC 2017
New revision: 455560
URL: https://svnweb.freebsd.org/changeset/ports/455560

Log:
  Switch to fork that uses new brotli ABI.

  Thanks to:	Evgenii Kliuchnikov <eustas@google.com>

  PR:		224000

  While I'm here add CONFLICTS_INSTALL and PORTSCOUNT.
  Bump PORTREVISION.

Changes:
  head/www/nginx-devel/Makefile
  head/www/nginx-devel/distinfo