Remove dead code from stdenv check-meta license logic #47230
Conversation
The `unfree` and `unfreeRedistributable` licenses both have `free = false`, which will trigger the first portion of logic. This removes dead code to simplify the logic. As a follow-up, I plan to add an attribute `redistributable = [true|false]`, which can be used by Hydra to determine whether a given package with a given license can be included in the channel.
GrahamcOfBorg
added
6.topic: stdenv
| @@ -42,8 +42,7 @@ let | |||
| allowUnsupportedSystem = config.allowUnsupportedSystem or false | |||
| || builtins.getEnv "NIXPKGS_ALLOW_UNSUPPORTED_SYSTEM" == "1"; | |||
|
|
|||
| isUnfree = licenses: lib.lists.any (l: | |||
| !l.free or true || l == "unfree" || l == "unfree-redistributable") licenses; | |||
| isUnfree = licenses: lib.lists.any (l: !l.free or true) licenses; | |||
There was a problem hiding this comment.
I may be reading this incorrectly, but as far as I can tell both unfree and unfreeRedistributable have free = false, which means the first condition will always return true and we don't need the special casing:
https://github.com/NixOS/nixpkgs/blob/master/lib/licenses.nix#L600-L608
There was a problem hiding this comment.
I think this comes from a while back when licenses were just strings - not attrsets. Might be reasonable to just leave it though.
There was a problem hiding this comment.
It's somewhat reasonable, I suppose, but it's dead code -- and the stdenv package is already quite big and complicated. I'd argue we should take the opportunity to simplify our logic slightly.
There was a problem hiding this comment.
We can just flat up require they not be mere strings elsewhere too
There was a problem hiding this comment.
Seems like a sensible improvement. I did some initial searching with
git grep 'license = ".*'
and found a few thousand packages with string licenses, but only in ~160 files. Looks like we can fix the bulk of them by updating the Haskell and NodeJS generators to output attr licenses instead of string licenses.
As for the rest, there are quite a few that just say "GPL", but not which GPL version, so it'll take some more effort than just blasting with sed.
|
Yeah looks reasonable for now. |
The
unfreeandunfreeRedistributablelicenses both havefree = false,which will trigger the first portion of logic. This removes dead code to
simplify the logic.
As a follow-up, I plan to add an attribute
redistributable = [true|false],which can be used by Hydra to determine whether a given package with a given
license can be included in the channel.
Motivation for this change
Things done
sandboxinnix.confon non-NixOS)nix-shell -p nox --run "nox-review wip"./result/bin/)nix path-info -Sbefore and after)