<div dir="ltr"><div>Got it, thanks.</div><div><br></div><div>Mikhail: thank you for the feedback too. If you want me to review any updates, let me know.<br></div></div><div class="gmail_extra"><br><div class="gmail_quote">On Tue, Feb 20, 2018 at 10:19 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">Good point. There aren't ReadP-parser for SPDX.License, so no Text<br>
instance either.<br>
But there is `Distribution.Pretty.Pretty` instance (`prettyShow`).<br>
<br>
- Oleg<br>
<span class=""><br>
<br>
On 20.02.2018 20:28, Michael Snoyman wrote:<br>
> Alright, I've updated the PR to use `Either SPDX.License License`:<br>
><br>
> <a href="https://github.com/commercialhaskell/stack/pull/3878/commits/6679aafe99a087dd6aac341fce965f4067e1be77" rel="noreferrer" target="_blank">https://github.com/<wbr>commercialhaskell/stack/pull/<wbr>3878/commits/<wbr>6679aafe99a087dd6aac341fce965f<wbr>4067e1be77</a><br>
><br>
> The only difference from your description that I ran into is that<br>
> there's no Text instance for SPDX.License, meaning instead of:<br>
><br>
> either display display<br>
><br>
> I ended up with<br>
><br>
> display . either licenseFromSPDX id<br>
><br>
> Is it intentional that there is no Text instance for SPDX.License?<br>
><br>
> On Tue, Feb 20, 2018 at 8:02 PM, Oleg Grenrus <<a href="mailto:oleg.grenrus@iki.fi">oleg.grenrus@iki.fi</a><br>
</span><div><div class="h5">> <mailto:<a href="mailto:oleg.grenrus@iki.fi">oleg.grenrus@iki.fi</a>>> wrote:<br>
><br>
> 1. The InstalledPackageInfo license field is also `Either SPDX.License<br>
> License` [1] and you can get a list of IPIs conviently with<br>
> `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<br>
> 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>
> <<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>
> <<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>
><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.<br>
> 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<br>
> `display`?<br>
> ><br>
> > On Tue, Feb 20, 2018 at 6:12 PM, Oleg Grenrus<br>
> <<a href="mailto:oleg.grenrus@iki.fi">oleg.grenrus@iki.fi</a> <mailto:<a href="mailto:oleg.grenrus@iki.fi">oleg.grenrus@iki.fi</a>><br>
</div></div><div><div class="h5">> > <mailto:<a href="mailto:oleg.grenrus@iki.fi">oleg.grenrus@iki.fi</a> <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>
> > <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>
> <<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>
> > <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>
> <<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>
> > I agree with Herbert on this. New `allBuildInfo`<br>
> implementation is<br>
> > correct given the name. There was even a TODO to make that<br>
> change.<br>
> > `reallyAllBuildInfo` would been silly. I also didn't felt ok<br>
> to change<br>
> > the type to `Traversal` (we have lenses, please try out them<br>
> too and<br>
> > tell if something is missing!). We'll highlight the change<br>
> 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<br>
> 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<br>
> Left expr,<br>
> > expr :: Distribution.SPDX.License.<wbr>License<br>
> > - `cabal-version: 2.0` files still use old `License`<br>
> 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<br>
> workaround lack of<br>
> > e.g. `OtherLicense` as a concept (IIRC it's converted to<br>
> > LicenseRef:OtherLicense which is valid SPDX-License-Id).<br>
> 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<br>
> cut for<br>
> > > Cabal, and recommended we try to upgrade Stack to it. I'm<br>
> sharing<br>
> > > information here on that process in case it's useful to<br>
> others,<br>
> > either<br>
> > > as feedback on the API changes, or help for others going<br>
> 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<br>
> single<br>
> > > commit[2]. As you can see from the change to<br>
> 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<br>
> 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<br>
> is a Good<br>
> > > Thing. Changes I had to make:<br>
> > ><br>
> > > * The build type is no longer a Maybe field. (As a user:<br>
> 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<br>
> the SPDX<br>
> > > variant. I don't understand what this change is about,<br>
> some further<br>
> > > explanation in the docs or the ChangeLog would definitely be<br>
> > > appreciated. But hacking my way through the types seems to<br>
> 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).<br>
> 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<br>
> errors is<br>
> > > _very_ nice. +1<br>
> > ><br>
> > > I haven't done extensive testing yet, but I've so far only<br>
> found one<br>
> > > bug due to runtime changes: the behavior of allBuildInfo<br>
> 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<br>
> behavior<br>
> > come<br>
> > > with a new function name, to convert runtime errors into<br>
> compile<br>
> > time<br>
> > > errors. This was _not_ a particularly difficult issue to<br>
> work around<br>
> > > though, in large part thanks to the changelog.<br>
> > ><br>
> > > I'll continue testing this branch of Stack. Our plans are<br>
> 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]<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>
> <<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>
> <<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>
> > <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>
> > <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>
> > <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>
> <<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>
> > > <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>
</div></div>> <mailto:<a href="mailto:cabal-devel@haskell.org">cabal-devel@haskell.<wbr>org</a> <mailto:<a href="mailto:cabal-devel@haskell.org">cabal-devel@haskell.<wbr>org</a>>><br>
<span class="">> > ><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>
> > <<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>
> > _____________________________<wbr>__________________<br>
> > cabal-devel mailing list<br>
> > <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>
</span>> <mailto:<a href="mailto:cabal-devel@haskell.org">cabal-devel@haskell.<wbr>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>
> <<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>
><br>
<br>
<br>
</blockquote></div><br></div>