It is working well!

While this PR does work for building the gtk version of netsurf, the framebuffer version does not render pages for me.

I have made some changes over here to this PR which simplify (or maybe overly abstract*) the expression. I tested uilib = {gtk,gtk3,framebuffer} and all work for me.

It would be nice if you could test it too, if you want to test the framebuffer version build netsurf.browser without overriding and then run xterm -ti 340 -e "SDL_VIDEODRIVER=sixel ./result/bin/netsurf-fb"

Which brings me to the resulting binaries have the suffix {fb,gtk,gtk3}. It seems like other distributions symlink netsurf to the corresponding hyphenated binary. However, I am not sure if this is really a wanted thing since it causes a collision when installing the framebuffer and gtk version at the same time.

@samueldr do you want to incorporate my changes into this PR or prefer I make a PR after this gets merged?

* It would be nice to eventually make *Support options which correspond to Makefile.conf options.

Ooh, glossed over your commit and it does look good. It at least looks like it has better handling of the ca bundle. I will look at it more in details tomorrow or during the week-end.

Why is the ca bundle patch only applied for GTK builds?

   patches =
    if (gtk2Support || gtk3Support)
      then [ ./find-ca-bundle.patch ]
      else [ ];

Well, I see the patch is for the gtk frontend in the code; is there an equivalent patch for framebuffer (would it even be needed?). And thanks for the invocation for the sixel (framebuffer) variant, this'll end up being useful.

Why is the ca bundle patch only applied for GTK builds?

You guessed right. All though I am not sure why, framebuffer is able to connect to HTTPS sites without any patches applied.

And thanks for the invocation for the sixel (framebuffer) variant, this'll end up being useful.

No problem, happy to help.

Oh, one thing though I'm not sure about the ca-bundle patch; it assumes one wants the global /etc/ssl/certs/ca-bundle.crt path. It is likely to be present on non-NixOS distros, but I'm not sure it's ideal to rely on that existing.

I see some other derivations are built to embed the path of the ca-bundle for the current nixpgkgs release, which I think is likely better, at least better self-contained within nixpkgs. Will search a bit to see if there's a preferred stance for nixpkgs.

Hmmm

diff --git a/frontends/gtk/gui.c b/frontends/gtk/gui.c
index 3163be16d..143a1041b 100644
--- a/frontends/gtk/gui.c
+++ b/frontends/gtk/gui.c
@@ -200,15 +200,11 @@ static nserror set_defaults(struct nsoption_s *defaults)
 		nsoption_setnull_charp(downloads_directory, strdup(fname));
 	}
 
-	/* default path to certificates */
-	nsoption_setnull_charp(ca_path, strdup("/etc/ssl/certs"));
-
 	if ((nsoption_charp(cookie_file) == NULL) ||
 	    (nsoption_charp(cookie_jar) == NULL) ||
 	    (nsoption_charp(url_file) == NULL) ||
 	    (nsoption_charp(hotlist_path) == NULL) ||
-	    (nsoption_charp(downloads_directory) == NULL) ||
-	    (nsoption_charp(ca_path) == NULL)) {
+	    (nsoption_charp(downloads_directory) == NULL)) {
 		NSLOG(netsurf, INFO,
 		      "Failed initialising default resource paths");
 		return NSERROR_BAD_PARAMETER;

Testing with https://badssl.com/ seems to indicate that it works as expected that way. Still looking into it and will try to get in touch with upstream to see if it makes sense to drop the default, considering this is how framebuffer works.

EDIT: here's an strace showing it indeed looks to the ca-bundle with that patch.

~/tmp/nixpkgs/nixpkgs $ grep '/etc/ssl' netsurf.strace
21571 openat(AT_FDCWD, "/nix/store/029i3gc3a26wwddjiiszyzqc4kbxq9q0-openssl-1.0.2s/etc/ssl/openssl.cnf", O_RDONLY) = 5
21571 openat(AT_FDCWD, "/nix/store/mdzqf7m3kkzbq72ycnhlrg3pnj3x71gf-nss-cacert-3.44.1/etc/ssl/certs/ca-bundle.crt", O_RDONLY) = 14
21571 openat(AT_FDCWD, "/nix/store/mdzqf7m3kkzbq72ycnhlrg3pnj3x71gf-nss-cacert-3.44.1/etc/ssl/certs/ca-bundle.crt", O_RDONLY) = 19
21571 openat(AT_FDCWD, "/nix/store/mdzqf7m3kkzbq72ycnhlrg3pnj3x71gf-nss-cacert-3.44.1/etc/ssl/certs/ca-bundle.crt", O_RDONLY) = 19
21571 openat(AT_FDCWD, "/nix/store/mdzqf7m3kkzbq72ycnhlrg3pnj3x71gf-nss-cacert-3.44.1/etc/ssl/certs/ca-bundle.crt", O_RDONLY) = 19
21571 openat(AT_FDCWD, "/nix/store/mdzqf7m3kkzbq72ycnhlrg3pnj3x71gf-nss-cacert-3.44.1/etc/ssl/certs/ca-bundle.crt", O_RDONLY) = 19
21571 openat(AT_FDCWD, "/nix/store/mdzqf7m3kkzbq72ycnhlrg3pnj3x71gf-nss-cacert-3.44.1/etc/ssl/certs/ca-bundle.crt", O_RDONLY) = 19
21571 openat(AT_FDCWD, "/nix/store/mdzqf7m3kkzbq72ycnhlrg3pnj3x71gf-nss-cacert-3.44.1/etc/ssl/certs/ca-bundle.crt", O_RDONLY) = 19

For completion's sake, it looks like the framebuffer interface just stops loading resources on SSL failure. Additionally, only the beos GUI sets the value of ca_path at initialization.

Additional CAs added through security.pki.certificates cannot be found if we specify to use ${cacert}/etc/ssl/certs/ca-bundle.crt instead of the /etc/ssl/certs/ca-bundle.crt. At least that was the case about three years ago, it might have changed. If it has, then I am for the self-contained within nixpkgs option.

it looks like the framebuffer interface just stops loading resources on SSL failure

Considering the documentation for it states, "The framebuffer frontend is not a replacement for full native ports. It lacks functionality and flexibility compared to such implementations." How it acts versus how the GTK version acts might be purposeful design decisions though this is speculation on my part. Probably best to talk to upstream about it.

Additional CAs added through security.pki.certificates cannot be found [...]

Ah! Then I wouldn't think it would work with the self-contained option. It's likely more right to set it to the well-known configuration folder. Though, I further think that unsetting everything and relying on the intrinsic defaults in curl is the better solution. I am getting in touch with upstream to see what they think, and if I'm missing something.

Considering the documentation for it states, "The framebuffer frontend is not a replacement for full native ports. It lacks functionality and flexibility compared to such implementations." How it acts versus how the GTK version acts might be purposeful design decisions though this is speculation on my part. Probably best to talk to upstream about it.

That was purely informational :). I was basically stating that using the framebuffer version with https is not unsafe, as it will not load pages when the validation fails. In this case it's important since it has the better behaviour of using the intrinsic defaults of curl instead of setting options when initializing.

I will update with the accepted-by-upstream patch.

• http://source.netsurf-browser.org/netsurf.git/commit/?id=87177d8aa109206d131e0d80a2080ce55dab01c7

This means that we can decisively drop the nixos-specific wrapper with --ca_bundle (which I already didn't really like), and no need to add specifics for curl elsewhere.

Nice, thank you for talking to upstream about it. Have you had a chance to look at the other changes?

I started looking at your changes. There's a bunch of good, there's some reformatting, and some questionable bits.

Since Nix has lazy evaluation, using the following has no advantage.

{ stdenv, gtk2 ? null }:

let
  available = x: x != null;
  gtk2Support = uilib == "gtk";
in
assert gtk2Support -> all available [gtk2 /* ... */];
stdenv.mkDerivation rec {
  buildInputs = [
    /* ... */
  ]
  ++ optional (gtk2Support) gtk2
  ;
}

callPackage will dependency-inject gtk2 all the time, so it'll never end up being null. Such a pattern could be used outside Nixpkgs to, instead, select the UI library via the presence or absence of a package, which could be used like gtk2Support = gtk2 != null; or something like that. When gtk2 is not in buildInputs, from being optional, it will not be in the derivation, thus will not be in the dependency while building nor while using it.

Additionally, the asserts have no value. Nixpkgs is handled as a whole, gtk2 (for example) is always available, until it isn't, and when it isn't packages that were using it are fixed to not use it, or removed. The fact that gtk2 is used in the derivation would make it fail, so there's no need to assert first.

Similarly, switching between SDL without or with sixel support is better handled via passing a different SDL argument; dependency-injection again. Nothing else than SDL_sixel depends on the need for the sixel feature.

I'll import the definitely good stuff from your changes, but will not import those that change the behaviour just yet... instead I want to merge this one (with cleanups) faster, and then work a bit more on the netsurf infra in another PR, with you if you're interested.

Using with (edit) at a global scope is often frowned upon. At least with stdenv.lib has a relatively limited scope, but many contributors think that selecting specifics through let inherit ...; in generally ends up being better.

In the last changes, I have included some of the changes from S-NA@ff11007, though I also verified almost every assumptions about the build system. Things that were removed were simply not present in the source of netsurf, or not required for gtk and framebuffer to work.

The next steps, which I think should be handled in a new PR, is to rework the "public-facing" bits, the bits under the netsurf attrset.

Thank you for the information and looking through my changes. Indeed some of my changes were not optimal, it was mostly cobbled together at the last minute. Just noticed I had a typo in NETSURF_UA_FORMAT_STiRING too.

Sounds good with the off putting the public-facing bits for later. Hopefully this lands in master soon, currently what is in there is completely unusable for me.

@S-NA can you validate that the current state is good? I'm pretty sure it is, I verified with sixel, and with gtk(2). If it is, then I'll merge. Thanks for looking into it!

(A non-member can use the Review Changes feature of GitHub. In the NixOS organization, and especially the Nixpkgs project, it is encouraged that external reviewers leave a review when they are knowledgeable on the specifics. This helps other maintainers that do not have the domain specific knowledge in making a decision about the changes.

And, yeah, I saw NETSURF_UA_FORMAT_STiRING, even without the typo, I'm unsure about adding such a change. This only makes the already strong fingerprinting stronger, by making a really specific user agent string for Netsurf+NixOS. I think keeping the default is likely better in that respect. It's not something specific to Netsurf, User Agent strings should probably become much less specific overall.

Good point about the fingerprinting aspect. Considering that, best to leave it to the default. I can confirm the current state builds and everything is working nicely.