Prepare for v0.14.3 release

[JordanMartinez]

Description of the change

Updates the versions in preparation for the upcoming v0.14.3 release.

To clarify, was the purescript-cst package updated at all? I don't believe so, but I wasn't sure if that should be bumped to v2.0.0.1 or something.

A few issues that were found:

The RELEASE_GUIDE.md file doesn't instruct me to update the CHANGELOG.md file when submitting this PR.

When compiling the package set, this error was produced. See https://github.com/matthew-hilty/purescript-subcategory/blob/master/src/Functor/HasConst.purs

Error 1 of 2:

  in module Control.Subcategory.Endofunctor.HasConst
  at .spago/subcategory/v0.2.0/src/Endofunctor/HasConst.purs:25:26 - 25:52 (line 25, column 26 - line 25, column 52)

    No type class instance was found for

      Control.Subcategory.Functor.HasPure.HasPure c3
                                                  (c3 v14)

    The following instance partially overlaps the above constraint, which means the rest of its instance chain will not be considered:

      Control.Subcategory.Functor.HasPure.hasPureUnrestricted


  while applying a function pure'
    of type HasPure t0 t1 => ObjectOf t0 t2 => Proxy3 t0 -> t2 -> t1 t2
    to argument Proxy3
  while inferring the type of pure' Proxy3
  in value declaration constRestrictableFixedSourceArrow

  where c3 is a rigid type variable
          bound at (line 0, column 0 - line 0, column 0)
        v14 is a rigid type variable
          bound at (line 25, column 11 - line 25, column 54)
        t0 is an unknown type
        t1 is an unknown type
        t2 is an unknown type

  See https://github.com/purescript/documentation/blob/master/errors/NoInstanceFound.md for more information,
  or to contribute content related to this error.

Error 2 of 2:

  in module Control.Subcategory.Functor.HasConst
  at .spago/subcategory/v0.2.0/src/Functor/HasConst.purs:25:13 - 25:39 (line 25, column 13 - line 25, column 39)

    No type class instance was found for

      Control.Subcategory.Functor.HasPure.HasPure c3
                                                  (c3 v14)

    The following instance partially overlaps the above constraint, which means the rest of its instance chain will not be considered:

      Control.Subcategory.Functor.HasPure.hasPureUnrestricted


  while applying a function pure'
    of type HasPure t0 t1 => ObjectOf t0 t2 => Proxy3 t0 -> t2 -> t1 t2
    to argument Proxy3
  while inferring the type of pure' Proxy3
  in value declaration constRestrictableFixedSourceArrow

  where c3 is a rigid type variable
          bound at (line 0, column 0 - line 0, column 0)
        v14 is a rigid type variable
          bound at (line 0, column 0 - line 0, column 0)
        t0 is an unknown type
        t1 is an unknown type
        t2 is an unknown type

  See https://github.com/purescript/documentation/blob/master/errors/NoInstanceFound.md for more information,
  or to contribute content related to this error.


[error] Docs generation failed.

[JordanMartinez]

K, there have definitely been some changes in purescript-cst. I'll see what version it should be updated to based on PVP.

[thomashoneyman]

The new error in subcategory is unexpected -- is this related to #4118? If this isn't worrying, then when we go ahead with the release I'll drop the package from the package set and notify the author.

[hdgarrood]

This is worrying, I think we need to figure out what’s going on there before we can release. It’s a non-breaking release so it shouldn’t have breaking changes.

[MonoidMusician]

HasPure c (c v) is in scope, so it should be found … is something in the instance chain logic preventing it from being found because it is trying concrete instances?

[rhendric]

This is more likely to be a consequence of #4064, actually. We might need to consider reverting that if it would count as a breaking change (assuming this isn't just a bug in my code, which it might be).

[rhendric]

I believe that #4064 is working correctly and catching an error in purescript-subcategory.

The declaration

instance constRestrictableFixedSourceArrow
  :: ( HasPure c (c v)
     , Restrictable Function c
     )
  => HasConst c

introduces a type named v. The signature of const in HasConst is

  const
    :: forall v0 v1
     . ObjectOf c v0
    => ObjectOf c v1
    => v0
    -> c v1 v0

which introduces types named (conveniently, for my purposes) v0 and v1. The value of the expression

const v = pure' (Proxy3 :: Proxy3 c) v

is inferred to have type c v1 v0. The local context doesn't contain a solution for HasPure c (c v1); it contains a solution for HasPure c (c v), but that's not helpful.

The global context contains an instance chain for HasPure, and the first instance in that chain has ‘type’ forall f. HasPure Function f. c may or may not be Function, so this is an ambiguous match. The rule that predated #4064, but which was implemented imperfectly, is that an ambiguous match should prevent the rest of the instances in the chain from being considered.

Therefore, no instance found.

---

But now the question before the court is, is this a breaking change (was purescript-subcategory written correctly, given the understanding of the language that existed prior to this release) or a bug fix? I believe, given the way this HasConst instance is written, that purescript-subcategory is in error. The HasPure c (c v) constraint on the instance doesn't appear to serve any purpose whatsoever; it looks like an attempt to provide the instance that pure' will use, but it never succeeded in doing so, and now the compiler is no longer obscuring this fact.

What says the court?

[f-f]

Thank you for digging in this @rhendric! 👏

What says the court?

It doesn't matter what's correct, if it breaks users then it's a breaking change.
Since we'd like to release a patch release now I'd say we should revert #4064, get the release out, pull #4064 back in and expect the next release to be called 0.15

[hdgarrood]

That explanation makes sense to me. I agree with @f-f; I don’t think it would be acceptable to release this as v0.14.3 as-is, even though the problem is in the library. I expect there’s lots of places in practice where code is wrong but compiles anyway, and in many of those cases it’s probably not the end of the world that the code is wrong (maybe the paths which involve the bad code are rarely hit). From the point of view of a user who ran into one of the new bugs in 0.14.2, it’s a drag to be forced to fix an instance chain problem in code you don’t really care about to be able to benefit from the other fixes which led us to want to expedite this release.

It doesn't matter what's correct, if it breaks users then it's a breaking change.

I don’t think I agree with this; it’s often much less clear cut, because breaking changes are changes in the public API of a package, and so the question becomes “is the thing we are changing part of a public API”. I think breaking downstream code in a non-breaking release is acceptable if it’s clear that the downstream code is depending on a non-public API. For example, if we changed the representation of data types and there was downstream code which was depending on the old representation, that would be a case where it wouldn’t have to be a breaking release, because it’s clearly documented that the representation of data types is non-public.

In the case of instance chains, I don’t think they have ever been well documented enough for us to be able to claim that any part of them is “non-public”; in the absence of docs, the interface becomes the implementation.

[JordanMartinez]

it’s a drag to be forced to fix an instance chain problem in code you don’t really care about to be able to benefit from the other fixes which led us to want to expedite this release.

Sad to say it, but I think this should be the controlling factor here. Normally, we'd wait a few more weeks before making another release. We're releasing this one earlier than normal to fix a few bugs that arose in v0.14.2. In many ways, v0.14.3 would function more like a v0.14.2-2 release.

While it's possible that this error only affects that one person's code, it's possible that it will affect others' code. So, even if the library was fixed, we don't know who else would be affected.

Based on my reading of @f-f and @hdgarrood's points above, it sounds like you two both agree that this counts as a "breaking" change, even if your reasons for describing it as that differ. The immediate implication is that the breaking PR should be reverted (as much as I wish that wouldn't happen).

Since we'd like to release a patch release now I'd say we should revert #4064, get the release out, pull #4064 back in and expect the next release to be called 0.15

While #4064 should be in v0.15.0 if it's breaking, I don't think it follows that our next release should be v0.15.0. Are we all ready to do the work that that would entail? Have we agreed which other breaking changes (compiler and the foundational libraries) will also be included in that release besides ES modules, if any?

To clarify, I'm up for a v0.15.0 release, but only if everyone else is.

[f-f]

@JordanMartinez

it sounds like you two both agree that this counts as a "breaking" change, even if your reasons for describing it as that differ

I understood Harry's answer as a clarification on top of mine, I agree with what he said above.

Are we all ready to do the work that that would entail? Have we agreed which other breaking changes (compiler and the foundational libraries) will also be included in that release besides ES modules, if any?

I see no reason why we'd have to include in a 0.15 anything else than this breaking change, as far as I know we didn't promise anywhere "the next breaking release will absolutely contain ES modules".
In case it wasn't clear I'm firmly in the camp of "release small and often", because in my experience that's easier for users (more gradual breakage, easier to update) and for maintainers (as the release process is exercised more and the manually painful parts are automated away)

So I don't think releasing a small 0.15 in some weeks entails substantially more work than releasing a small 0.14.3 now.
(If we want to, of course. Rushing all the breaking changes in the "breaking release window" is also a possibility, but one that doesn't make sense to me and is in fact very risky, since we can cut a new release whenever we wish and spread smaller breakage over time)

[hdgarrood]

Note that a breaking compiler release doesn’t require updating every single core library. If the next release after 0.14.3 contains this fix but that’s the only breaking change, none of the libraries in the package set other than subcategory (and its dependents) will need updating, most likely. There’s no rule that says we have to address every planned breaking change in core libraries every time we make a breaking compiler release.

[hdgarrood]

I’m not sure I agree smaller and more frequent breaking changes are easier for users, though. I think there’s a very real risk of losing community goodwill if we are forcing people to handle breaking changes regularly in order to benefit from other non-breaking changes. This is actually one the most common reasons I’ve seen people express dissatisfaction for PureScript; even now, some people think our breaking changes are too arbitrary and frequent.

[f-f]

This is actually one the most common reasons I’ve seen people express dissatisfaction for PureScript; even now, some people think our breaking changes are too arbitrary and frequent

I heard a similar complaint, but I'd argue that most of the breaking releases we had are very breaking, so I understand (as a heavy user myself) how that is really annoying to deal with.
"Smaller releases" are actually the opposite of that! There's less going on, so it's much easier and less painful to go through the upgrades.

[rhendric]

In the case of instance chains, I don’t think they have ever been well documented enough for us to be able to claim that any part of them is “non-public”

Oh yeah, let's fix that.

[JordanMartinez]

Quick question. Should #4064 be reverted in a separate PR from this one (I assume yes)? If so, is it expected of me to do that? I implicitly thought @rhendric would do that, but wanted to ask directly since I opened this PR.

[rhendric]

Sorry, #4129 is now created.

[rhendric]

Although I have to say, I wish we used release branches for this sort of thing. One of my most frequent tools for understanding parts of the compiler is walking though the history of a line or function with git blame, and revert/revert-revert pairs in the commit history don't help with that.

[JordanMartinez]

#4129 has been merged. I just need an approval and then I can release v0.14.3

[hdgarrood]

Although I have to say, I wish we used release branches for this sort of thing.

I sympathise with this but I also really don't want to deal with backporting all the time, and I also think that because of how badly git's UI sucks, lots of potential contributors can't really handle a branching strategy any more complex than "everything goes into main in order". Backports in particular need to be really carefully reviewed, because people generally just don't know how to handle merge conflicts properly. I feel like I have spent too much of my time already helping people untangle themselves when they've gotten their git branches into a mess.