<div dir="ltr"><div>Alright, I've updated the PR to use `Either SPDX.License License`:</div><div><br></div><div><a href="https://github.com/commercialhaskell/stack/pull/3878/commits/6679aafe99a087dd6aac341fce965f4067e1be77">https://github.com/commercialhaskell/stack/pull/3878/commits/6679aafe99a087dd6aac341fce965f4067e1be77</a></div><div><br></div><div>The only difference from your description that I ran into is that there's no Text instance for SPDX.License, meaning instead of:</div><div><br></div><div>    either display display</div><div><br></div><div>I ended up with</div><div><br></div><div>    display . either licenseFromSPDX id</div><div><br></div><div>Is it intentional that there is no Text instance for SPDX.License?<br></div></div><div class="gmail_extra"><br><div class="gmail_quote">On Tue, Feb 20, 2018 at 8:02 PM, Oleg Grenrus <span dir="ltr"><<a href="mailto:oleg.grenrus@iki.fi" target="_blank">oleg.grenrus@iki.fi</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">1. The InstalledPackageInfo license field is also `Either SPDX.License<br>
License` [1] and you can get a list of IPIs conviently with `HcPkg.dump` [2]<br>
2. It's up to you, if `Text` is enough, go for it. In the future you<br>
might want to provide some license reports, where you'll need structured<br>
license information, but then "reverting" `TextualLicense` will be a<br>
type-directed, so it's up to you.<br>
<br>
Oleg<br>
<br>
<br>
-<br>
<a href="https://github.com/haskell/cabal/blob/32fea06a1023a507d7dc16b29f542538c0b55e46/Cabal/Distribution/Types/InstalledPackageInfo.hs#L50" rel="noreferrer" target="_blank">https://github.com/haskell/<wbr>cabal/blob/<wbr>32fea06a1023a507d7dc16b29f5425<wbr>38c0b55e46/Cabal/Distribution/<wbr>Types/InstalledPackageInfo.hs#<wbr>L50</a><br>
-<br>
<a href="https://github.com/haskell/cabal/blob/32fea06a1023a507d7dc16b29f542538c0b55e46/Cabal/Distribution/Simple/Program/HcPkg.hs#L246" rel="noreferrer" target="_blank">https://github.com/haskell/<wbr>cabal/blob/<wbr>32fea06a1023a507d7dc16b29f5425<wbr>38c0b55e46/Cabal/Distribution/<wbr>Simple/Program/HcPkg.hs#L246</a><br>
<span class=""><br>
<br>
On 20.02.2018 19:45, Michael Snoyman wrote:<br>
> I'm sorry, I'm not quite sure I understand your recommendation. Are<br>
> you saying that I should ideally replace all usages of `License` in<br>
> the Stack codebase with `Either SPDX.License License`? That _should_<br>
> be possible, the only questions I'd have are:<br>
><br>
> 1. We additionally grab license info from the GHC package dump. Would<br>
> there be any problem parsing that license field into the old License<br>
> data type and storing as Right?<br>
> 2. If we're going to have to treat this as arbitrary text anyway, is<br>
> there any reason not to represent it as `newtype TextualLicense =<br>
> TextualLicense Text` or similar, and convert immediately with `display`?<br>
><br>
> On Tue, Feb 20, 2018 at 6:12 PM, Oleg Grenrus <<a href="mailto:oleg.grenrus@iki.fi">oleg.grenrus@iki.fi</a><br>
</span><span class="">> <mailto:<a href="mailto:oleg.grenrus@iki.fi">oleg.grenrus@iki.fi</a>>> wrote:<br>
><br>
>     Hi Michael,<br>
><br>
>     thanks for your comments!<br>
><br>
>     - The allBuildInfo change is<br>
>     <a href="https://github.com/haskell/cabal/commit/8fc10320a5dc4898927c84ad6a2dce7965ef30db" rel="noreferrer" target="_blank">https://github.com/haskell/<wbr>cabal/commit/<wbr>8fc10320a5dc4898927c84ad6a2dce<wbr>7965ef30db</a><br>
</span>>     <<a href="https://github.com/haskell/cabal/commit/8fc10320a5dc4898927c84ad6a2dce7965ef30db" rel="noreferrer" target="_blank">https://github.com/haskell/<wbr>cabal/commit/<wbr>8fc10320a5dc4898927c84ad6a2dce<wbr>7965ef30db</a>>,<br>
<div><div class="h5">>     I agree with Herbert on this. New `allBuildInfo` implementation is<br>
>     correct given the name. There was even a TODO to make that change.<br>
>     `reallyAllBuildInfo` would been silly. I also didn't felt ok to change<br>
>     the type to `Traversal` (we have lenses, please try out them too and<br>
>     tell if something is missing!). We'll highlight the change in the<br>
>     changelog, as it's easy to miss.<br>
><br>
>     - The lack of SPDX changelog entry is my bad. It was a series of<br>
>     patches, and the changelog + manual update is still not done. I try to<br>
>     summarise:<br>
>         - `cabal-version: 2.2` files use SPDX expression syntax for<br>
>     `license: ` field. PackageDescription's licenseRaw will be Left expr,<br>
>     expr :: Distribution.SPDX.License.<wbr>License<br>
>         - `cabal-version: 2.0` files still use old `License` syntax and<br>
>     type, licenseRaw is `Right l`, l :: Distribution.License.License<br>
>         - I recommend treating the field as opaque. You can `either<br>
>     display<br>
>     display` to show it<br>
>            - Next best choice is to use `SPDX.License` as in that<br>
>     direction<br>
>     conversion is lossless (licenseToSPDX), but have to workaround lack of<br>
>     e.g. `OtherLicense` as a concept (IIRC it's converted to<br>
>     LicenseRef:OtherLicense which is valid SPDX-License-Id). This might be<br>
>     necessary if you plan to read (human readable) text back.<br>
><br>
>     Oleg<br>
><br>
>     On 20.02.2018 15:47, Michael Snoyman wrote:<br>
>     > Hi all,<br>
>     ><br>
>     > Oleg mentioned to me on Reddit that a 2.2 branch has been cut for<br>
>     > Cabal, and recommended we try to upgrade Stack to it. I'm sharing<br>
>     > information here on that process in case it's useful to others,<br>
>     either<br>
>     > as feedback on the API changes, or help for others going through<br>
>     > similar upgrades. If anyone would rather that I do or do not<br>
>     post such<br>
>     > reports in the future, let me know.<br>
>     ><br>
>     > I've opened a PR for this change[1], which currently is a single<br>
>     > commit[2]. As you can see from the change to stack.yaml[3], I needed<br>
>     > to refer to the commit of Cabal in question, and turn on<br>
>     `allow-newer`<br>
>     > for some packages (cabal-doctest and http-api-data) with restrictive<br>
>     > upper bounds on Cabal. Both of these packages compiled without<br>
>     issue.<br>
>     ><br>
>     > No change was necessary to Stack's Setup.hs file.<br>
>     ><br>
>     > Most of the changes so far were compile-time errors, which is a Good<br>
>     > Thing. Changes I had to make:<br>
>     ><br>
>     > * The build type is no longer a Maybe field. (As a user: this is a<br>
>     > nice change, no longer a need to guess about what a Nothing<br>
>     value means.)<br>
>     > * There are now two License types, the original one and the SPDX<br>
>     > variant. I don't understand what this change is about, some further<br>
>     > explanation in the docs or the ChangeLog would definitely be<br>
>     > appreciated. But hacking my way through the types seems to have<br>
>     worked.<br>
>     > * Parsing a package description now works on a ByteString<br>
>     instead of a<br>
>     > String. This allowed me to remove some UTF8 conversion and<br>
>     > BOM-stripping code, which is very nice.<br>
>     > * The new parsing API exposes the cabal file version when that<br>
>     > prevented the parse (at least, that's my understanding). The changes<br>
>     > to accommodate the new API were relatively trivial, so another<br>
>     +1 here.<br>
>     > * Also: getting file position information for warnings and errors is<br>
>     > _very_ nice. +1<br>
>     ><br>
>     > I haven't done extensive testing yet, but I've so far only found one<br>
>     > bug due to runtime changes: the behavior of allBuildInfo has changed<br>
>     > to now include non-buildable components. This resulted in some<br>
>     > confusion until I reviewed the changelog more closely. If I<br>
>     could make<br>
>     > a request, it would be that, in the future, new runtime behavior<br>
>     come<br>
>     > with a new function name, to convert runtime errors into compile<br>
>     time<br>
>     > errors. This was _not_ a particularly difficult issue to work around<br>
>     > though, in large part thanks to the changelog.<br>
>     ><br>
>     > I'll continue testing this branch of Stack. Our plans are to merge<br>
>     > this to master, and release Stack 1.7.0 close to the Cabal 2.2<br>
>     release<br>
>     > date.<br>
>     ><br>
>     > Michael<br>
>     ><br>
>     > [1] <a href="https://github.com/commercialhaskell/stack/pull/3878/files" rel="noreferrer" target="_blank">https://github.com/<wbr>commercialhaskell/stack/pull/<wbr>3878/files</a><br>
>     <<a href="https://github.com/commercialhaskell/stack/pull/3878/files" rel="noreferrer" target="_blank">https://github.com/<wbr>commercialhaskell/stack/pull/<wbr>3878/files</a>><br>
>     > [2]<br>
>     ><br>
>     <a href="https://github.com/commercialhaskell/stack/pull/3878/commits/a101341d04213d6dd8e0cf16d2f2fef8e7ed5cd5" rel="noreferrer" target="_blank">https://github.com/<wbr>commercialhaskell/stack/pull/<wbr>3878/commits/<wbr>a101341d04213d6dd8e0cf16d2f2fe<wbr>f8e7ed5cd5</a><br>
>     <<a href="https://github.com/commercialhaskell/stack/pull/3878/commits/a101341d04213d6dd8e0cf16d2f2fef8e7ed5cd5" rel="noreferrer" target="_blank">https://github.com/<wbr>commercialhaskell/stack/pull/<wbr>3878/commits/<wbr>a101341d04213d6dd8e0cf16d2f2fe<wbr>f8e7ed5cd5</a>><br>
>     > [3]<br>
>     ><br>
>     <a href="https://github.com/commercialhaskell/stack/pull/3878/commits/a101341d04213d6dd8e0cf16d2f2fef8e7ed5cd5#diff-fafd0cdcd559a7b124cc61c29413fb54" rel="noreferrer" target="_blank">https://github.com/<wbr>commercialhaskell/stack/pull/<wbr>3878/commits/<wbr>a101341d04213d6dd8e0cf16d2f2fe<wbr>f8e7ed5cd5#diff-<wbr>fafd0cdcd559a7b124cc61c29413fb<wbr>54</a><br>
>     <<a href="https://github.com/commercialhaskell/stack/pull/3878/commits/a101341d04213d6dd8e0cf16d2f2fef8e7ed5cd5#diff-fafd0cdcd559a7b124cc61c29413fb54" rel="noreferrer" target="_blank">https://github.com/<wbr>commercialhaskell/stack/pull/<wbr>3878/commits/<wbr>a101341d04213d6dd8e0cf16d2f2fe<wbr>f8e7ed5cd5#diff-<wbr>fafd0cdcd559a7b124cc61c29413fb<wbr>54</a>><br>
>     ><br>
>     ><br>
>     > ______________________________<wbr>_________________<br>
>     > cabal-devel mailing list<br>
</div></div>>     > <a href="mailto:cabal-devel@haskell.org">cabal-devel@haskell.org</a> <mailto:<a href="mailto:cabal-devel@haskell.org">cabal-devel@haskell.<wbr>org</a>><br>
>     > <a href="http://mail.haskell.org/cgi-bin/mailman/listinfo/cabal-devel" rel="noreferrer" target="_blank">http://mail.haskell.org/cgi-<wbr>bin/mailman/listinfo/cabal-<wbr>devel</a><br>
<span class="">>     <<a href="http://mail.haskell.org/cgi-bin/mailman/listinfo/cabal-devel" rel="noreferrer" target="_blank">http://mail.haskell.org/cgi-<wbr>bin/mailman/listinfo/cabal-<wbr>devel</a>><br>
><br>
><br>
><br>
>     ______________________________<wbr>_________________<br>
>     cabal-devel mailing list<br>
</span>>     <a href="mailto:cabal-devel@haskell.org">cabal-devel@haskell.org</a> <mailto:<a href="mailto:cabal-devel@haskell.org">cabal-devel@haskell.<wbr>org</a>><br>
>     <a href="http://mail.haskell.org/cgi-bin/mailman/listinfo/cabal-devel" rel="noreferrer" target="_blank">http://mail.haskell.org/cgi-<wbr>bin/mailman/listinfo/cabal-<wbr>devel</a><br>
>     <<a href="http://mail.haskell.org/cgi-bin/mailman/listinfo/cabal-devel" rel="noreferrer" target="_blank">http://mail.haskell.org/cgi-<wbr>bin/mailman/listinfo/cabal-<wbr>devel</a>><br>
><br>
><br>
<br>
<br>
</blockquote></div><br></div>