If this is a separate job we'd need to recompile the binary, which takes a while (since we cannot pass artifacts between jobs).

I think we could instead attach this step to the ubuntu build job, which is the shortest one so far.

We can share assets between jobs using upload-artifact

I'm looking into whether we can make this a workflow/job/step that runs only on PRs that don't have the breaking label, so we can avoid cluttering the history with commenting/uncommenting the job.

@rhendric we can do that by checking the labels on the pull request -- basically, adding this to the relevant job:

if: ${{ github.event.label.name != 'breaking' }}

Ah, nice. I think that's the thing to do then, at least for runs triggered by pull requests. I'm not sure what we should do about the builds that run on push to master; like I said, I'm not keen on having some sort of toggle being changed back and forth in the Git history, but I also don't want to lose the signal that those builds provide during the periods when breaking changes are expected.

We could potentially skip the check on pushes to master, since it will be checked in the PR already. That's not perfect -- things could interact strangely -- but it could get us very close to sure that we haven't introduced a breaking change when we're not supposed to and not require any other futzing around with settings during a period when we are introducing breaking changes.

We might want a label other than 'breaking', though, to indicate that we would like to skip the package set check. For example, if we've already introduced a breaking change then we'd like subsequent pull requests to also skip the check even if they aren't breaking.

Blast, you're right. I hadn't thought of that. Now I'm not sure what I think. 😁

I think the options are:

• being fine with disabling the check once breaking changes are in and the package set didn't catch up yet
• and/or point to an alternative package set that we keep patched (and/or require patching the set when the breaking change goes in)

Previously: #2837. (My position on this has softened since then though, I think this probably is a better tradeoff now). I think we won't want to have the check based on PR labels, though, because this test will start failing every time once we have merged one breaking change into master - which would mean that contributors would be encouraged to add a breaking label for every single PR once there is an unreleased breaking change in master, regardless of whether the PR in question is actually breaking.

Disabling the check entirely while there's an unreleased breaking change in master is probably not a bad option. Turning the check off entirely seems a bit crude, but we're probably not losing too much in the case where we do expect there to be breaking changes.

I also think requiring updating an alternative package set before merging would cause too much hassle, unless we are quite aggressive about removing things which no longer compile from that alternative set. We do occasionally need to be able to break things, and it's not feasible for the person who makes a contribution to the compiler to also be responsible for updating the entire ecosystem to handle that change. I could imagine a scenario where, in the periods where we have a breaking change which isn't shipped yet, we point to an alternative package set which, say, only contains the core libraries.

We do occasionally need to be able to break things, and it's not feasible for the person who makes a contribution to the compiler to also be responsible for updating the entire ecosystem to handle that change. I could imagine a scenario where, in the periods where we have a breaking change which isn't shipped yet, we point to an alternative package set which, say, only contains the core libraries.

Right, a requirement along the lines of "the whole package set should still compile" is unreasonable since there are so many packages outside of our control and a change might require forming a big chunk of them, but something like "the package set with core, node, web and contrib should still compile" feels more reasonable.

However, the latter requirement wouldn't catch failures in exotic type-level libraries such as the one triggered by #4064, so maybe reducing the package set size makes this check way less useful?

Requiring a successful compile against node, web, and contrib still feels like too much to me personally, especially since this change was partially motivated by us being worried about how long it takes to get compiler PRs merged.

I don't think compiling against a reduced set in the run-up to a breaking change release makes the check that much less useful; it's intended for avoiding accidental breaking changes during non-breaking releases, so it's less relevant in the run-up to a breaking change release. The failure triggered by #4064 would have been caught anyway because at that point we weren't expecting to have any breaking changes, so we would have been compiling against the full set. If we had released that change as part of a breaking release I wouldn't have considered it much of a problem - in that case, I think all we would have needed to do would have been to move the relevant entry from the "new features" section of the release notes into the "breaking changes" section.

Reading through the download-artifact readme again, I think my usage of path is telling the action to download purs into the ~/bin/purs directory, so that the full file name would be ~/bin/purs/purs.

Anyone got ideas for why spago can't find the purs binary? CI shows that running purs --version works and that it's on the PATH.

https://github.com/purescript/purescript/runs/3147055332#step:8:35

Welp! I've exhausted all ideas I can think of. Any ideas? Or should I just close this PR?

@JordanMartinez I'll have a look at this

Closing since this never was figured out.