sct: fix segfault when DISPLAY is empty, clean up derivation #29801
Conversation
lukateras
force-pushed
the
sct/patch-segfault
branch
from
9664a8e
to
ebdefa1
Compare
lukateras
force-pushed
the
sct/patch-segfault
branch
from
ebdefa1
to
44beb56
Compare
| # failed builds as the checksum had changed. | ||
| # The checksum is updated for now, however, this is unpractical and potentially unsafe | ||
| # so any future changes might warrant a fork of the (feature complete) project. | ||
| # The code is under public domain. |
There was a problem hiding this comment.
There are several problems with this comment:
The code is in public domainis now in metadata.- The change that has been made is not unknown and basically incorporates
patchPhasethat I've also removed. - Digest change is not potentially unsafe, that's misleading. It can only result in failed builds.
Also I'd argue that it's overly verbose. Keep in mind that sct.c hasn't been updated for a year and a half and probably never will be. The discussion (#17163) is mentioned in 62ba6b0 and is easily retrievable in case if needed.
There was a problem hiding this comment.
Can we host this file somewhere else, where it won't be changed (gist)? Otherwise it will break nixpkgs.
There was a problem hiding this comment.
@Mic92 This was already decided in #17163. Author (one of OpenBSD commiters, tedu@) knows that several distros rely on this file being unchanged, and probably didn't anticipated this when he had changed the file previously. I could have made https://web.archive.org copy and link to it, but unfortunately author's website uses a custom TLS certificate and https://web.archive.org can't archive when certificate is not valid/not in cert store.
| }; | ||
| phases = ["patchPhase" "buildPhase" "installPhase"]; | ||
| patchPhase = '' | ||
| sed -re "/Xlibint/d" ${src} > sct.c |
There was a problem hiding this comment.
No longer necessary.
Incorporating this patch was the unknown change that the comment has mentioned.
There was a problem hiding this comment.
ok. I was not aware of this change.
|
Thanks! |
Motivation for this change
patchPhasethat is now redundant (libXintis not present in source code anymore: it was the unknown small change that was described in derivation comments)meta.license,meta.homepagenot setgccwas explicitly used instead ofccsctsegfaults whenDISPLAYenvironment variable is empty:This patch fixes that. Now it just exits with error code 1.
Things done
build-use-sandboxinnix.confon non-NixOS)nix-shell -p nox --run "nox-review wip"./result/bin/)