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