Skip to content

Change license :: License to Either SPDX.License License #5050

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

Merged
merged 1 commit into from
Jan 21, 2018

Conversation

phadej
Copy link
Collaborator

@phadej phadej commented Jan 17, 2018

WIP: [ci skip] Have to fix cabal-tests and check that we don't break
older (i.e all) ghc-pkgs

Resolves #2547

I introduce SimpleLicenseExpression to make "OrAnyLater LicenseRef"
unrepresentable. That also simplifies types.

license field is parsed as old License when cabal-version <2.2,
and as SPDX expression otherwise. NONE is recognised.

There are also IPI changes, there both old and SPDX license expressions
are accepted, as both can occur in package database.

Please include the following checklist in your PR:

  • Patches conform to the coding conventions.
  • Any changes that could be relevant to users have been recorded in the changelog.
  • The documentation has been updated, if necessary.
  • If the change is docs-only, [ci skip] is used to avoid triggering the build bots.

Please also shortly describe how you tested your change. Bonus points for added tests!

@phadej
Copy link
Collaborator Author

phadej commented Jan 17, 2018

I probably end up introducing spdx field into IPI, so license field can be left intact, and new field used for SPDX stuff. I have to run cabal-tests to see how badly things are broken right now.

@phadej
Copy link
Collaborator Author

phadej commented Jan 17, 2018

I made Register write license as SPDX in IPI with GHC >= 8.4 only. That will do it. Simple.

-- If GHC >= 8.4 we register with SDPX, otherwise with legacy license
IPI.license =
if ghc84
then Left $ either id licenseToSPDX $ licenseRaw pkg
Copy link
Collaborator Author

@phadej phadej Jan 17, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This choice converts everything to SPDX, so in GHC-8.4 package databases will only contain SPDX expressions.
Alternatively we can let licenseRaw to go though unmodified, or convert only "sure" things (e.g. MIT, BSD3, GPL-3, not PublicDomain or OtherLicense)

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sounds good to me :-)

@phadej phadej requested review from 23Skidoo and hvr January 17, 2018 23:09
@phadej
Copy link
Collaborator Author

phadej commented Jan 18, 2018

How it's possible that in GHC-7.8.4 output https://travis-ci.org/haskell/cabal/jobs/330116951#L2091 I see

Linked component graph:
    unit my-0
        include base-4.11.0.0

ping @ezyang, any ideas?

@phadej
Copy link
Collaborator Author

phadej commented Jan 19, 2018

ping @ezyang , any ideas why tests fail on older GHC? For example https://travis-ci.org/haskell/cabal/jobs/330116951

@phadej
Copy link
Collaborator Author

phadej commented Jan 19, 2018

Ah I see, it actually tests with ghc-head! I have to skip that tests before GHC-head is updated :)

@23Skidoo
Copy link
Member

You can change the GHC version check to 8.4.0.2018MMDD: make this a month or two in the future now and update later once you know the correct snapshot date.

@phadej
Copy link
Collaborator Author

phadej commented Jan 19, 2018

@23Skidoo head is 8.5 so that won't work, I'll just temporarily skip the test for 8.4+ and unskip it when Cabal is updated in GHC-tree.

Copy link
Member

@23Skidoo 23Skidoo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks all right from a quick glance.

Resolves haskell#2547

I introduce SimpleLicenseExpression to make "OrAnyLater LicenseRef"
unrepresentable. That also simplifies types.

license field is parsed as old `License` when cabal-version <2.2,
and as SPDX expression otherwise. `NONE` is recognised.

There are best-effort functions to convert between `License` and
`SPDX.License`.

There are also IPI changes: parser accepts both `License` and
`SPDX.License`, as both can occur in package database. Cabal will
`register` a SDPX expression as `license` for GHC >= 8.4, and legacy
`License` for other (we are smart when converting `PackageDescription` +
LBI and other data to `InstalledPackageInfo`)

Also add NFData InstalledPackageInfo
@phadej
Copy link
Collaborator Author

phadej commented Jan 20, 2018

@23Skidoo do you want to take a closer look, or should I merge? (when travis is green, at least mostly)

@23Skidoo
Copy link
Member

I'll try to later today, feel free to merge if I won't.

@phadej phadej merged commit 8e3da02 into haskell:master Jan 21, 2018
@phadej phadej deleted the spdx-license-2 branch January 21, 2018 10:20
@phadej
Copy link
Collaborator Author

phadej commented Jan 21, 2018

I merged this now, any comments are still welcome

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Consider adopting/support SPDX licence identifiers
3 participants