stevenblack-blocklist: rev bump and improvements of module + package

[Frontear]

Description of changes

• Wrapped the package with stdenvNoCC.mkDerivation
• Added a passthru.updateScript for @r-ryantm (tested with nix-shell maintainers/scripts/update.nix --argstr package stevenblack-blocklist
• Moved from pkgs/tools/networking/stevenblack-blocklist to pkgs/by-name/st/stevenblack-blocklist
• Changed $out output to contain the various hosts only, removing all else
• Added 5 distinct outputs [ "ads" "fakenews" "gambling" "porn" "social" ] that expose isolated outputs as host files.
• Changed the nixos module to use these new updated outputs (tested in a nixos-generate-config config using nixos-rebuild build-vm)

New outputs help reduce closure size by having significantly smaller outputs depending on isolated usecases. Before, the closure size was roughly 100MB, but with these changes even the full combination of closures will be roughly 20MB, a significantly better size. The original directory is left as a fallback with all the extraneous stuff removed, as those are likely not of interest to a user who simply wants hosts.

Functionality remains exactly the same as before, with the added bonus of smaller closure sizes overall.

Things done

• Built on platform(s)
  • [x] 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:
  • NixOS test(s) (look inside nixos/tests)
  • and/or package tests
  • or, for functions and "core" functionality, tests in lib/tests or pkgs/test
  • made sure NixOS tests are linked to the relevant packages
• [x] 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/)
• 24.11 Release Notes (or backporting 23.11 and 24.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
• [x] Fits CONTRIBUTING.md.

---

Add a :+1: reaction to pull requests you find important.

[Frontear]

Result of nixpkgs-review run on x86_64-linux 1

1 package built:

• stevenblack-blocklist

[Frontear]

Great ideas! Will commit and adjust PR description accordingly.

[Frontear]

Result of nixpkgs-review pr 321649 run on x86_64-linux 1

1 package built:

• stevenblack-blocklist

[Frontear]

Squash stdenvNoCC and updateScript commit

Done, and added dont{Configure,Build} = true in this commit as well.

[JohnRTitor]

I believe, these readme.md's can be removed before copying?

[Frontear]

I've got a new suggestion for this package.

Currently, closure size is quite high overall. A big part of this is due to the fact that unrelated hosts files are still brought into the closure but never used (see: https://github.com/NixOS/nixpkgs/blob/master/nixos/modules/config/stevenblack.nix).

My suggestion for this was simple, include applicable overrides that can reduce closure size massively. This would look something like this

{
  lib,
  stdenvNoCC,
  fetchFromGitHub,
  nix-update-script,

  blockList ? [] # can add "fakenews" "gambling" "porn" "social", much like the module
}:
let
  inherit (lib) concatStringsSep optionals;

  # ripped from nixos module
  activatedHosts = []
    ++ optionals (elem "fakenews" blockList) [ "fakenews" ]
    ++ optionals (elem "gambling" blockList) [ "gambling" ]
    ++ optionals (elem "porn" blockList) [ "porn" ]
    ++ optionals (elem "social" blockList) [ "social" ];
  hostsPath = (if activatedHosts != [] then "alternates/" + concatStringsSep "-" activatedHosts else "") + "/hosts"
in stdenvNoCC.mkDerivation {
  # ...

  installPhase = ''
    mkdir -p $out
    cp ${hostsPath} $out
  '';
  
  # ...
}

This will then require a change in the nixos module, which I propose can be done somewhat like this:

{ config, lib, pkgs, ... }:
let
  inherit (lib) mkEnableOption mkIf mkOption types; 
  cfg = config.networking.stevenblack;
  package = pkgs.stevenblack-blocklist.override { blockList = cfg.block };
in {
  options.networking.stevenblack = {
    enable = mkEnableOption "the stevenblack hosts file blocklist.";

    block = mkOption {
      type = types.listOf (types.enum [ "fakenews" "gambling" "porn" "social" ]);
      default = [ ];
      description = "Additional blocklist extensions.";
    };
  };

  config = mkIf cfg.enable {
    networking.hostFiles = [ "${package}/hosts" ];
  };
}

These are big changes, and I'm very open to ideas of syntax or design details, however I think this change will be greatly beneficial overall.

[itslychee]

I think that's a good idea!

[Frontear]

One small change, because theres no sane way to sanitize blockList as an override value, I instead think ill have four values

fakenews ? false
gambling ? false
...

Course this changes implementation slightly, but end result of closure size reduction will still be achieved, with the added bonus of no possible breakage from a bad override

[Frontear]

EDIT: Backtracking on this after some criticisms.

A fair critique I recieved on the unofficial NixOS discord is that this doesn't reduce the amount of "work" done (still needs to clone full repo, so network usage and storage space remains high still, this ONLY benefits closure size).

A solution I thought up for this (though it may be very unconventional), was to pull only the specific hosts file via a fetchurl. This would look something like this

src = fetchurl {
  url = https://raw.githubusercontent.com/StevenBlack/hosts/${finalAttrs.version}/path/to/hosts;
  hash = ""; # fill in
};

I believe this is quite unconventional, and it would break the auto update script. On the other hand, it could solve both the unnecessary work, and reduce closure size all in one. This alongside with the module changes could result in a significantly improved package + module.

Again, open to critiques and suggestions!

[itslychee]

All the users of this package are going to use all parts of this package at some point so I wouldn't concern yourself with the download sizes for Hydra, but rather worry about splitting the package up into outputs so users can use relevant parts for their usecases instead of downloading the entire src, and the host files seems to be where the big file sizes are coming from anyways, seemingly averaging around 3MBs per host file.

[Frontear]

All the users of this package are going to use all parts of this package at some point so I wouldn't concern yourself with the download sizes for Hydra, but rather worry about splitting the package up into outputs so users can use relevant parts for their usecases instead of downloading the entire src, and the host files seems to be where the big file sizes are coming from anyways, seemingly averaging around 3MBs per host file.

Honestly good point, I also don't particularly like the fetchurl thing. Backtracking on that idea, instead only focusing on simply opening the ability for overrides then

[Frontear]

Different change has been made, idea courtesy of @itslychee. There are now 5 distinct outputs that are joined in the module system to make it work identically. Normal source repo is still given, and technically the module could've worked fine without any changes, but I opted to update the module as well to use smaller outputs for a reduced closure size.

[itslychee]

Probably should split the package and module changes of the last commit into its own logically separated commits.

Something like

stevenblack-blocklist: separate outputs, fix module ->

• stevenblack-blocklist: split host files into their own outputs
• nixos/stevenblack: allow usage of specific outputs

[Frontear]

Probably should split the package and module changes of the last commit into its own logically separated commits.

Something like

stevenblack-blocklist: separate outputs, fix module ->

• stevenblack-blocklist: split host files into their own outputs
• nixos/stevenblack: allow usage of specific outputs

Done!

[Frontear]

force push because upstream rev bump

[Frontear]

Switching to draft because smaller fixes are incubating.

[Frontear]

Done some testing, things all look a lot better overall, so I will go ahead and un-draft this

[Frontear]

Another upstream bump happened, so another rebase has been pushed

[JohnRTitor]

Result of nixpkgs-review pr 321649 run on x86_64-linux 1

2 packages blacklisted:

• nixos-install-tools
• tests.nixos-functions.nixos-test

6 packages built:

• stevenblack-blocklist
• stevenblack-blocklist.ads
• stevenblack-blocklist.fakenews
• stevenblack-blocklist.gambling
• stevenblack-blocklist.porn
• stevenblack-blocklist.social

[JohnRTitor]

Before I merge this:

┃ error: builder for '/nix/store/dmr7w0cqxdj0xbz1l5fm5wm124c2jc09-review-shell.drv' failed with exit code 127;
┃        last 3 log lines:
┃        > /nix/store/1jik86ikk79rlkhrnwmyksfzcnhgaa6l-stevenblack-blocklist-3.14.84-ads: line 15: 127.0.0.1: command not found
┃        > /nix/store/d3dzfy4amjl826fb8j00qp1d9887h7hm-stdenv-linux/setup: line 692: pop_var_context: head of shell_variables not a function context
┃        > /nix/store/d3dzfy4amjl826fb8j00qp1d9887h7hm-stdenv-linux/setup: line 699: pop_var_context: head of shell_variables not a function context
┃        For full logs, run 'nix log /nix/store/dmr7w0cqxdj0xbz1l5fm5wm124c2jc09-review-shell.drv'.

looks like it's related to the ads output.

[Frontear]

Before I merge this:

┃ error: builder for '/nix/store/dmr7w0cqxdj0xbz1l5fm5wm124c2jc09-review-shell.drv' failed with exit code 127;
┃        last 3 log lines:
┃        > /nix/store/1jik86ikk79rlkhrnwmyksfzcnhgaa6l-stevenblack-blocklist-3.14.84-ads: line 15: 127.0.0.1: command not found
┃        > /nix/store/d3dzfy4amjl826fb8j00qp1d9887h7hm-stdenv-linux/setup: line 692: pop_var_context: head of shell_variables not a function context
┃        > /nix/store/d3dzfy4amjl826fb8j00qp1d9887h7hm-stdenv-linux/setup: line 699: pop_var_context: head of shell_variables not a function context
┃        For full logs, run 'nix log /nix/store/dmr7w0cqxdj0xbz1l5fm5wm124c2jc09-review-shell.drv'.

looks like it's related to the ads output.

Error is quite strange because I don't quite understand why its trying to process the hosts file as a shell command. I don't believe this error is exclusive to the ads output, its simply because ads builds first (probably due to alphabetical ordering).

[Frontear]

@JohnRTitor follow-up on issue, its related to a assumption in nixpkgs trying to source the output: https://github.com/Mic92/nixpkgs-review/issues/407.

I think i'll end up moving the outputs into a $out/hosts pattern. Suggestions?

[Frontear]

Fixed issue with stdenv sourcing the output as a shell file.

[JohnRTitor]

Result of nixpkgs-review pr 321649 run on x86_64-linux 1

1 package blacklisted:

• nixos-install-tools

6 packages built:

• stevenblack-blocklist
• stevenblack-blocklist.ads
• stevenblack-blocklist.fakenews
• stevenblack-blocklist.gambling
• stevenblack-blocklist.porn
• stevenblack-blocklist.social