-
-
Couldn't load subscription status.
- Fork 159
make parsed license to be less like tobe a stringy license #677
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
base: master
Are you sure you want to change the base?
Conversation
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.
Please also add a changelog entry, ideally noting when nixpkgs added support for lib.licensesSpdx.
cabal2nix/src/Distribution/Nixpkgs/Haskell/FromCabal/License.hs
Outdated
Show resolved
Hide resolved
|
|
||
| fromCabalLicense :: Distribution.License.License -> Distribution.Nixpkgs.License.License | ||
| fromCabalLicense (GPL Nothing) = Unknown (Just "GPL") | ||
| fromCabalLicense (GPL Nothing) = Known "lib.licenses.gpl2Plus" |
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.
These cases are intentional as there is not enough information to conclude what variant of the license in question is used.
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.
I do realize they were intentional, but they can't stay in their current form. I'm currently working towards disallowing stringy licenses in nixpkgs, and those are pretty much the only ones left. Other options would be to make it even less informational with lib.licenses.free or to add a new custom license (not really a fan of this).
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.
I looked at a few packages in more detail and I think we can reasonably do two things:
- interpret
GPLasgpl2Plusas done here. That's because they could have usedGPL-2orGPL-3if they wanted to be more specific. - just use
freeas fallback for this.
For me, both are fine.
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.
free as a fallback seems reasonable for GPL without further specification, that is a good idea. (But not in general as a fallback for unknown, see #677 (comment)).
cabal2nix/src/Distribution/Nixpkgs/Haskell/FromCabal/License.hs
Outdated
Show resolved
Hide resolved
cabal2nix/src/Distribution/Nixpkgs/Haskell/FromCabal/License.hs
Outdated
Show resolved
Hide resolved
Hackage requires open source licenses so this should always be a free license
| -- Not handed: the '+' suffix and user-defined licences references. | ||
| -- Use the SPDX expression as a free-form license string. | ||
| Unknown (Just $ DPretty.prettyShow simpl) | ||
| fromSPDXLicenseExpression (SPDX.ELicense simpl (Just excep)) = | ||
| case simpl of | ||
| SPDX.ELicenseId lid -> Known ("lib.licensesSpdx.\"" ++ DPretty.prettyShow lid ++ "\" lib.licensesSpdx.\"" ++ DPretty.prettyShow excep ++ "\"") | ||
| _ -> | ||
| -- Not handed: the '+' suffix and user-defined licences references. |
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.
| -- Not handed: the '+' suffix and user-defined licences references. | |
| -- Use the SPDX expression as a free-form license string. | |
| Unknown (Just $ DPretty.prettyShow simpl) | |
| fromSPDXLicenseExpression (SPDX.ELicense simpl (Just excep)) = | |
| case simpl of | |
| SPDX.ELicenseId lid -> Known ("lib.licensesSpdx.\"" ++ DPretty.prettyShow lid ++ "\" lib.licensesSpdx.\"" ++ DPretty.prettyShow excep ++ "\"") | |
| _ -> | |
| -- Not handed: the '+' suffix and user-defined licences references. | |
| -- Not handled: the '+' suffix and user-defined licences references. | |
| -- Use the SPDX expression as a free-form license string. | |
| Unknown (Just $ DPretty.prettyShow simpl) | |
| fromSPDXLicenseExpression (SPDX.ELicense simpl (Just excep)) = | |
| case simpl of | |
| SPDX.ELicenseId lid -> Known ("lib.licensesSpdx.\"" ++ DPretty.prettyShow lid ++ "\" lib.licensesSpdx.\"" ++ DPretty.prettyShow excep ++ "\"") | |
| _ -> | |
| -- Not handled: the '+' suffix and user-defined licences references. |
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.
There is also a typo in the commit message.
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.
Why is this comment repeated?
| homepage = "https://github.com/blamario/incremental-parser"; | ||
| description = "Generic parser library capable of providing partial results from partial input"; | ||
| license = "GPL"; | ||
| license = lib.licenses.gpl2Plus; |
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.
Upstream has changed to GPL-3 by now.
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 is probably fine to update these test expressions to newer versions of the cabal file, but some of them may test something specific that has since been removed.
| homepage = "http://github.com/jgm/highlighting-kate"; | ||
| description = "Syntax highlighting"; | ||
| license = "GPL"; | ||
| license = lib.licenses.gpl2Plus; |
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.
Upstream still uses GPL, and has gpl2Only, I think: https://github.com/jgm/highlighting-kate/blob/master/LICENSE
| homepage = "https://github.com/skogsbaer/HTF/"; | ||
| description = "The Haskell Test Framework"; | ||
| license = "LGPL"; | ||
| license = lib.licenses.lgpl2Plus; |
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.
Upstream has now LGPL-2.1, which renders as LGPL-2.1-only on hackage.
| homepage = "https://wiki.haskell.org/Lambdabot"; | ||
| description = "Social plugins for Lambdabot"; | ||
| license = "GPL"; | ||
| license = lib.licenses.gpl2Plus; |
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.
lambdabot-social-plugins has GPL, but.. the license looks like BSD/MIT: https://github.com/lambdabot/lambdabot/blob/master/lambdabot-social-plugins/LICENSE
| homepage = "http://github.com/jgm/texmath"; | ||
| description = "Conversion between formats used to represent mathematics"; | ||
| license = "GPL"; | ||
| license = lib.licenses.gpl2Plus; |
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.
texmath has updated to GPL-2, which renders as GPL-2.0-only on hackage.
|
|
||
| fromCabalLicense :: Distribution.License.License -> Distribution.Nixpkgs.License.License | ||
| fromCabalLicense (GPL Nothing) = Unknown (Just "GPL") | ||
| fromCabalLicense (GPL Nothing) = Known "lib.licenses.gpl2Plus" |
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.
I looked at a few packages in more detail and I think we can reasonably do two things:
- interpret
GPLasgpl2Plusas done here. That's because they could have usedGPL-2orGPL-3if they wanted to be more specific. - just use
freeas fallback for this.
For me, both are fine.
|
For context, see also #520. One thing to keep in mind is that while cabal2nix mostly deals with Hackage, it may also process arbitrary cabal files. We can make some assumptions in Nixpkgs since additions of unfree packages would always have to be manual, but it does not seem wise to have such assumptions leak into In particular,
Apologies for the inconsistent messages. Some of my earlier comments were informed by an incorrect assumption how we decide on free/nonfree. (Note that prior to #520, cabal2nix would prevent “unfree” packages from being built on Hydra, but Nixpkgs seems to consider stringy licenses as free by default (?). So from |
| -- Not handed: the '+' suffix and user-defined licences references. | ||
| -- Use the SPDX expression as a free-form license string. | ||
| Unknown (Just $ DPretty.prettyShow simpl) | ||
| fromSPDXLicenseExpression (SPDX.ELicense simpl (Just excep)) = | ||
| case simpl of | ||
| SPDX.ELicenseId lid -> Known ("lib.licensesSpdx.\"" ++ DPretty.prettyShow lid ++ "\" lib.licensesSpdx.\"" ++ DPretty.prettyShow excep ++ "\"") | ||
| _ -> | ||
| -- Not handed: the '+' suffix and user-defined licences references. |
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.
There is also a typo in the commit message.
| Unknown (Just $ DPretty.prettyShow simpl) | ||
| fromSPDXLicenseExpression (SPDX.ELicense simpl (Just excep)) = | ||
| case simpl of | ||
| SPDX.ELicenseId lid -> Known ("lib.licensesSpdx.\"" ++ DPretty.prettyShow lid ++ "\" lib.licensesSpdx.\"" ++ DPretty.prettyShow excep ++ "\"") |
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.
In spirit, License holds a Nix expression, so we should not output fragments of one here, but instead make sure it is individually a valid Nix expression. As such the [ … ] should be added in this function and not fromSPDXLicense.
Or we don't return a License here to make clear what's going on. In general, it may be wise to move this into a where since it is not really something that should be called except from fromSPDXLicense.
| -- Not handed: the '+' suffix and user-defined licences references. | ||
| -- Use the SPDX expression as a free-form license string. | ||
| Unknown (Just $ DPretty.prettyShow simpl) | ||
| fromSPDXLicenseExpression (SPDX.ELicense simpl (Just excep)) = | ||
| case simpl of | ||
| SPDX.ELicenseId lid -> Known ("lib.licensesSpdx.\"" ++ DPretty.prettyShow lid ++ "\" lib.licensesSpdx.\"" ++ DPretty.prettyShow excep ++ "\"") | ||
| _ -> | ||
| -- Not handed: the '+' suffix and user-defined licences references. |
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.
Why is this comment repeated?
|
Cherry picked 7a5af01 onto master. |
|
We currently have about 250 packages with a string license we actually are building and distributing. In total, |
`stdenv.mkDerivation` does not require meta.license to be passed, so there is no reason `haskellPackages.mkDerivation` needs to enforce this. This would free up cabal2nix to not report a license if it is not sure. As I have argued in NixOS/cabal2nix#677 (comment), it is better not to report a license than reporting an inaccurate one. This would also allow to stop generating arbitrary strings as licenses in cabal2nix to remove string values to facilitate more cleanliness in the spirit of #445672, though the question is whether it is wise to remove the meta data altogether.
I'm working on removing all stringy licenses in nixpkgs, and currently the only ones left are the ones in haskellPackages and node2nix packages. This change fixes the ones for Haskell packages.