Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Resolve #665: ghc-head works with GHA #671

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open

Resolve #665: ghc-head works with GHA #671

wants to merge 2 commits into from

Conversation

phadej
Copy link
Collaborator

@phadej phadej commented Aug 3, 2023

No description provided.

@phadej
Copy link
Collaborator Author

phadej commented Aug 3, 2023

Apparently allow-failure doesn't work on GHA. Why did I think it does. Well, I'm not going to add GHC nightly support then yet.

@RyanGlScott
Copy link
Contributor

RyanGlScott commented Aug 4, 2023

Apparently allow-failure doesn't work on GHA.

I think it does work, actually, but the UI is admittedly rather confusing. If you look at the Actions tab, you'll notice that the job for this PR is green. If you navigate to that job in particular, there is a scary red X next to the ghc-head job (and indeed, that job failed to build something), but the overall job status is still green due to the ghc-head job being marked as allow-failure: true.

@RyanGlScott
Copy link
Contributor

Ah, you noticed this already here. My apologies for the noise!

@phadej
Copy link
Collaborator Author

phadej commented Aug 7, 2023

Now I tried an alternative by marking individual steps as continue-on-error. That makes commit with a green checkbox, but it's impossible to have a quick glance whether the build were actual successful or not (i.e. you need to look through individual steps).

That's better (because of green checkmark) but somewhat useless.

What do you think? @RyanGlScott @Bodigrim?

EDIT: These are two points of bad UI in https://github.com/orgs/community/discussions/15452, if used on either job or step level continue-on-error UI is far from great. I'm not a fan of per-job behavior at all. I'm fine with having it per-step as I probably won't use GHC HEAD (or prerelease) jobs much. And for latter we can make a switch to actually make them fail properly (as prereleases are are lot less moving target, there is more stability, at least closer to an actual release).

@RyanGlScott
Copy link
Contributor

RyanGlScott commented Aug 7, 2023

Ideally, if a job fails for some reason, then it should be relatively straightforward to figure out the place in the job where it failed. I'm generally not a fan of continue-on-error for this reason, as it violates this property. Picking this job as a particular example, it's not at all obvious from a quick glance where this job fails (or even that it fails at all)—you have to go digging around to discover that it fails in the build step. I worry that this will make it easy to overlook potential issues in the GHC HEAD build, and if it's easy to overlook these issues, why both having a GHC HEAD configuration in the first place?

Using allow-newer has a different sort of suboptimal UI (in that things are given red X's that don't contribute to the overall success/failure of a workflow), but it is much easier to spot issues when they arise.

@phadej
Copy link
Collaborator Author

phadej commented Aug 7, 2023

Using allow-newer has a different sort of suboptimal UI

I don't understand. We talk about per job and per step continue-on-error.

As I said, per job continue-on-error is not an option as it gives the commit the red mark.

Screenshot from 2023-08-07 21-27-41

It would be simply no go to have a red mark on the above. I might know that it's fine (because I checked which job fails), but random contributor or just random code explorer will not.

EDIT: and I'd like to have ghc-head: True for this repository if it would rarely succeed, just to test that feature works. In fact the best feature would be to be able to mark steps which may fail, and will abort the job, but not fail the whole workflow. I.e. I wont to test that installing GHC etc. works, and the job may only fail after the "setup" part is done. (And I think travis worked that way).

@RyanGlScott
Copy link
Contributor

To be clear: the two modes of operation that I was comparing in my previous comment were:

  1. Marking each step in a job as continue-on-error (what this PR uses)
  2. Marking an entire job as allow-failure in the matrix, with no other changes to particular steps in the job (my "Using allow-newer has a different sort of suboptimal UI [...]" comment in Resolve #665: ghc-head works with GHA #671 (comment) is about this option)

Does that clarify things? I'm not sure which part of my comment you found confusing.

@phadej
Copy link
Collaborator Author

phadej commented Aug 7, 2023

@RyanGlScott I'm confused how allow-newer is related to this. Are you advocating of not using allow-newer in per step continue-on-error?

@RyanGlScott
Copy link
Contributor

Sorry, I meant to write "allow-failure" instead of "allow-newer". And specifically, I mean "using allow-failure without using continue-on-error in each step" for option (2).

@phadej phadej added the blocked label Aug 7, 2023
@phadej
Copy link
Collaborator Author

phadej commented Aug 7, 2023

Ok. We have

  • per step continue-on-error has essentially useless UI for our needs: we cannot spot failing builds.
  • per job continue-on-error shows "false negative" red mark on commit displays, which I cannot accept.

So this PR (and issue #665) will stay blocked until GitHub makes improvements in GHA or/and its UI so the use case of "testing against of unstable libraries/toolchains" is viable. (i.e. https://github.com/orgs/community/discussions/15452 is resolved).

@liskin
Copy link

liskin commented Sep 17, 2023

A possible solution to the useless UI of per-step continue-on-error would be to add an annotation (https://docs.github.com/en/actions/using-workflows/workflow-commands-for-github-actions#example-creating-an-annotation-for-an-error) so that it does show up in the pull-request UI. It's still mighty confusing but at least it's not completely silent and thus useless.

I think it's also possible to make a message appear in the workflow run UI using this: https://docs.github.com/en/actions/using-workflows/workflow-commands-for-github-actions#setting-an-error-message

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants