New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Fix handling of lists in whitelistedLicenses and blacklistedLicenses #72070
Conversation
A package's meta.license can either be a single license or a list. The code to check config.whitelistedLicenses and config.blackListedLicenses wasn't handling this, nor was the showLicense function.
I would like to write a test for this but I'm not really sure how to test internal functionality of nixpkgs like this. If anyone has any suggestions let me know! |
|
||
hasBlacklistedLicense = assert areLicenseListsValid; attrs: | ||
hasLicense attrs && builtins.elem attrs.meta.license blacklist; | ||
hasLicense attrs && lib.lists.any (l: builtins.elem l blacklist) (lib.lists.toList attrs.meta.license); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's debatable whether this should be any
or all
- if a package has multiple licenses and only one is on the blacklist I suppose it might be ok to use the non-blacklisted one, but I guess which one takes priority is probably a question for lawyers (https://en.wikipedia.org/wiki/Multi-licensing). It felt safer to use any
here.
The same logic could lead to using all
for the whitelist above. The logic here is more confusing though, since hasDeniedUnfreeLicense
already supports lists and apparently will consider something unfree if any of the licenses is unfree (and providing it doesn't match allowUnfreePredicate
). So if you have a package with two licenses, one free and one unfree, and you want to use the whitelist to say that the unfree license is ok with you, you would have to add both the free and unfree license to the whitelist if we used all
, even though the whitelist is only really relevant for unfree licenses. Because of that I'm currently leaving it as using any
, but open to suggestions.
Overall the asymmetry between the blacklist and the whitelist, and the ambiguity when multiple licenses are present makes this all quite confusing. I tried to at least clarify the docs a little in #72075, but I think this could use a refactor. It might be hard to clean it up properly without breaking existing configurations though.
Note that this change could break existing configurations: for example if a user has blacklisted a license but some packages they use have a list of licenses, some packages may be silently getting through. This is probably a good breakage. If they are using whitelisting and have an unfree package with a list of licenses, they have probably already noticed that whitelisting doesn't work and have done something else (e.g. used |
Yeah the multi license case is kind of awkward. I think it's better to handle it this way than to ignore it though. Luckily almost all multi-license cases I'm aware of are multiple free licenses. Combining free and unfree should be very rare., |
Motivation for this change
A package's meta.license can either be a single license or a list. The
code to check config.whitelistedLicenses and config.blackListedLicenses
wasn't handling this, nor was the showLicense function.
Tested as per below:
Things done
sandbox
innix.conf
on non-NixOS)nix-shell -p nix-review --run "nix-review wip"
./result/bin/
)nix path-info -S
before and after)Notify maintainers
cc @