Skip to content

nixos/nginx: remove recommendedZstdSettings, add experimental option - #381678

Merged
fpletz merged 1 commit into
NixOS:masterfrom
fpletz:nixos/nginx-experimental-zstd-settings
Jul 16, 2025
Merged

fpletz merged 1 commit into
NixOS:masterfrom
fpletz:nixos/nginx-experimental-zstd-settings

Conversation

@fpletz

@fpletz fpletz commented Feb 13, 2025

Copy link
Copy Markdown
Member

The zstd nginx module has known bugs and upstream is currently not maintained. We should not recommend a buggy module and configuration to our users since we are not maintaining the module either.

Things done

  • Built on platform(s)
    • x86_64-linux
    • aarch64-linux
    • x86_64-darwin
    • aarch64-darwin
  • For non-Linux: Is sandboxing enabled in nix.conf? (See Nix manual)
    • sandbox = relaxed
    • sandbox = true
  • Tested, as applicable:
  • Tested compilation of all packages that depend on this change using nix-shell -p nixpkgs-review --run "nixpkgs-review rev HEAD". Note: all changes have to be committed, also see nixpkgs-review usage
  • Tested basic functionality of all binary files (usually in ./result/bin/)
  • 25.05 Release Notes (or backporting 24.11 and 25.05 Release notes)
    • (Package updates) Added a release notes entry if the change is major or breaking
    • (Module updates) Added a release notes entry if the change is significant
    • (Module addition) Added a release notes entry if adding a new NixOS module
  • Fits CONTRIBUTING.md.

Add a 👍 reaction to pull requests you find important.

@github-actions github-actions Bot added 6.topic: nixos Issues or PRs affecting NixOS modules, or package usability issues specific to NixOS 8.has: module (update) This PR changes an existing module in `nixos/` labels Feb 13, 2025
@nix-owners
nix-owners Bot requested a review from RaitoBezarius February 13, 2025 08:28
@fpletz
fpletz force-pushed the nixos/nginx-experimental-zstd-settings branch from 8b1ca4f to f001242 Compare February 13, 2025 08:29
@github-actions github-actions Bot added 10.rebuild-darwin: 0 This PR does not cause any packages to rebuild on Darwin. 10.rebuild-linux: 1-10 This PR causes between 1 and 10 packages to rebuild on Linux. labels Feb 13, 2025
@fpletz
fpletz force-pushed the nixos/nginx-experimental-zstd-settings branch from f001242 to 77270af Compare February 13, 2025 09:56
ambroisie added a commit to ambroisie/nix-config that referenced this pull request Feb 13, 2025
The zstd module is unmaintained and buggy, remove it preventively.

The option itself will probably be removed soon [1].

[1]: NixOS/nixpkgs#381678
Comment thread nixos/modules/services/web-servers/nginx/default.nix Outdated
Comment thread nixos/modules/services/web-servers/nginx/default.nix Outdated
ambroisie added a commit to ambroisie/nix-config that referenced this pull request Feb 13, 2025
The zstd module is unmaintained and buggy, remove it preventively.

The option itself will probably be removed soon [1].

[1]: NixOS/nixpkgs#381678
@fpletz
fpletz force-pushed the nixos/nginx-experimental-zstd-settings branch from 77b60c7 to 18079b2 Compare February 13, 2025 22:55
Comment thread nixos/modules/services/web-servers/nginx/default.nix Outdated
@wegank wegank added the 2.status: merge conflict This PR has merge conflicts with the target branch label Apr 2, 2025
@ZeroEcks

Copy link
Copy Markdown
Contributor

I personally just got stung by this specific issue (pr 23 on the zstd-nginx-module), home assistant's favicon seems to trigger the infinite loop and is really annoying to debug!

I didn't understand that the 'recommended' zstd options could have major unfixed bugs like this, like causing my nginx reverse proxy to hang until i kill the worker process.

If we are going to keep this as an option, I would suggest we either include worker_shutdown_timeout or patch that bug, but it seems best to just adopt this PR.

@fpletz
fpletz force-pushed the nixos/nginx-experimental-zstd-settings branch from 18079b2 to 75db741 Compare July 3, 2025 15:37
@fpletz

fpletz commented Jul 3, 2025

Copy link
Copy Markdown
Member Author

Rebased and fixed the merge conflict. If there are no further objections, I intend to merge this in a few days.

The zstd nginx module has known bugs and upstream is currently not
maintained. We should not recommend a buggy module and configuration
to our users since we are not maintaining the module either.
@ofborg ofborg Bot removed the 2.status: merge conflict This PR has merge conflicts with the target branch label Jul 3, 2025
@fpletz
fpletz force-pushed the nixos/nginx-experimental-zstd-settings branch from 75db741 to db75f90 Compare July 3, 2025 15:41
@fpletz
fpletz merged commit 9771deb into NixOS:master Jul 16, 2025
3 of 4 checks passed
@fpletz
fpletz deleted the nixos/nginx-experimental-zstd-settings branch July 16, 2025 11:03
quic_bpf on;
''}

${optionalString cfg.experimentalZstdSettings ''

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.

In line 253 we still have recommendedZstdSettings which will probably break eval and I don't think that the move of the setting was intentional.

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.

pschmitt added a commit to pschmitt/nixos-config that referenced this pull request Jul 18, 2025
n0099 added a commit to n0099/nixos that referenced this pull request Dec 7, 2025
* replace hardcorded value for `home.stateVersion` with current NixOS version
* adopt changes of `programs.ssh`: nix-community/home-manager#7655
* adopt changes of `programs.git`: nix-community/home-manager#8006
@ home/n0099.nix

* adopt changes of NixOS/nixpkgs#381678 @ default.nix
* adopt changes of NixOS/nixpkgs#464333 @ http3.nix
@ nginx
n0099 added a commit to n0099/nixos that referenced this pull request Dec 8, 2025
* replace hardcorded value for `home.stateVersion` with current NixOS version
* adopt changes of `programs.ssh`: nix-community/home-manager#7655
* adopt changes of `programs.git`: nix-community/home-manager#8006
@ home/n0099.nix

* adopt changes of NixOS/nixpkgs#381678 @ default.nix
* adopt changes of NixOS/nixpkgs#464333 @ http3.nix
@ nginx
n0099 added a commit to n0099/nixos that referenced this pull request Dec 8, 2025
* replace hardcorded value for `home.stateVersion` with current NixOS version
* adopt changes of `programs.ssh`: nix-community/home-manager#7655
* adopt changes of `programs.git`: nix-community/home-manager#8006
@ home/n0099.nix

* adopt changes of NixOS/nixpkgs#381678 @ default.nix
* adopt changes of NixOS/nixpkgs#464333 @ http3.nix
@ nginx
pschmitt added a commit to pschmitt/nixos-config that referenced this pull request Jun 11, 2026
pschmitt added a commit to pschmitt/nixos-config that referenced this pull request Jul 21, 2026
pschmitt added a commit to pschmitt/nixos-config that referenced this pull request Jul 21, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

6.topic: nixos Issues or PRs affecting NixOS modules, or package usability issues specific to NixOS 8.has: module (update) This PR changes an existing module in `nixos/` 10.rebuild-darwin: 0 This PR does not cause any packages to rebuild on Darwin. 10.rebuild-linux: 1-10 This PR causes between 1 and 10 packages to rebuild on Linux.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants