Skip to content

nginx-mainline: add brotli module#8771

Closed
vr wants to merge 1 commit intowolfi-dev:mainfrom
vr:nginx_brotli
Closed

nginx-mainline: add brotli module#8771
vr wants to merge 1 commit intowolfi-dev:mainfrom
vr:nginx_brotli

Conversation

@vr
Copy link
Copy Markdown

@vr vr commented Nov 17, 2023

copying what was done in nginx-ingress

copying what was done in nginx-ingress
Comment thread nginx-mainline.yaml
Comment on lines +58 to +63
- uses: git-checkout
with:
repository: https://github.com/google/ngx_brotli.git
branch: master
expected-commit: a71f9312c2deb28875acc7bacfdd5695a111aa53
destination: ngx_brotli
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I think this should probably change to be

   - runs: |
        git clone https://github.com/google/ngx_brotli.git
        git checkout a71f9312c2deb28875acc7bacfdd5695a111aa53

Otherwise the next time this build runs the expected commit sha will not match and the build will fail.

Unfortunately we don't have an automated way to handle keeping multiple git-checkout.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Thinking about it a bit more, a better solution would probably be to create a new ngx_brotli package and add a environment.contents.packages dependency for the new ngx_brotli which can then be used for --add-dynamic-module=./ngx_brotli

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

@vr did you want to try that? I'm happy to if you prefer, up to you. This way both nginx-mainline and nginx-ingress can share the same ngx_brotli package.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

i'm not sure you can build once for both nginx version 🤔

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

and i was not able to directly use git without having uses: git-checkout before because git command was not available before

@rawlingsj rawlingsj added the blocked indicates there are blocking issues that need to be addressed before progress can be made label Dec 11, 2023
@rawlingsj rawlingsj self-assigned this Dec 11, 2023
@rawlingsj rawlingsj removed the blocked indicates there are blocking issues that need to be addressed before progress can be made label Dec 20, 2023
@github-actions
Copy link
Copy Markdown
Contributor

This Pull Request is stale because it has been open for 90 days with
no activity. It will automatically close after 30 more days of
inactivity. Keep fresh with the 'lifecycle/frozen' label.

@github-actions github-actions Bot closed this Apr 19, 2024
@vr vr deleted the nginx_brotli branch May 24, 2024 11:37
Axel-KIRK pushed a commit to Axel-KIRK/os that referenced this pull request Apr 23, 2026
Add ngx_brotli as a build-time dependency and emit two new dynamic
module subpackages (nginx-{mainline,stable}-mod-http_brotli_filter
and -static) via the existing range: modules loop.

The ngx_brotli sources are consumed from /usr/src/ngx_brotli
(provided by the new ngx_brotli source-only package) rather than
fetched inline, matching the pattern used by njs consuming
nginx-src-main. This structure lets nginx-mainline, nginx-stable,
and downstream consumers (e.g. ingress-nginx-controller) share the
same ngx_brotli pin via a single package.

Revives wolfi-dev#8771, addressing @rawlingsj's review feedback that
suggested packaging ngx_brotli separately.

Refs: wolfi-dev#8771
Axel-KIRK pushed a commit to Axel-KIRK/os that referenced this pull request Apr 23, 2026
Add ngx_brotli as a build-time dependency and emit two new dynamic
module subpackages (nginx-{mainline,stable}-mod-http_brotli_filter
and -static) via the existing range: modules loop.

The ngx_brotli sources are consumed from /usr/src/ngx_brotli
(provided by the new ngx_brotli source-only package) rather than
fetched inline, matching the pattern used by njs consuming
nginx-src-main. This structure lets nginx-mainline, nginx-stable,
and downstream consumers (e.g. ingress-nginx-controller) share the
same ngx_brotli pin via a single package.

Revives wolfi-dev#8771, addressing @rawlingsj's review feedback that
suggested packaging ngx_brotli separately.

Refs: wolfi-dev#8771
Axel-KIRK pushed a commit to Axel-KIRK/os that referenced this pull request Apr 23, 2026
Add ngx_brotli as a build-time dependency and emit two new dynamic
module subpackages (nginx-{mainline,stable}-mod-http_brotli_filter
and -static) via the existing range: modules loop.

The ngx_brotli sources are consumed from /usr/src/ngx_brotli
(provided by the new ngx_brotli source-only package) rather than
fetched inline, matching the pattern used by njs consuming
nginx-src-main. This structure lets nginx-mainline, nginx-stable,
and downstream consumers (e.g. ingress-nginx-controller) share the
same ngx_brotli pin via a single package.

Revives wolfi-dev#8771, addressing @rawlingsj's review feedback that
suggested packaging ngx_brotli separately.

Refs: wolfi-dev#8771
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants