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

split "validate old ghcs" into a separate workflow #10274

Closed
wants to merge 1 commit into from

Conversation

geekosaur
Copy link
Collaborator

@geekosaur geekosaur commented Aug 24, 2024

I am not convinced this is a good idea, for the same reason that Artem wants it: complexity. I'm very worried that this will break things, although as far as I can tell it should be okay as long as nobody commits anything overriding branch protection.

All branch protection rules will need to be updated to check for "Validate old ghcs post job" the same way they check for "Bootstrap post job" and "Validate post job".

Template B: This PR does not modify behaviour or interface

E.g. the PR only touches documentation or tests, does refactorings, etc.

Include the following checklist in your PR:

  • Patches conform to the coding conventions.
  • Is this a PR that fixes CI? If so, it will need to be backported to older cabal release branches (ask maintainers for directions).

NOTE re backporting: branch protection rules need to be updated, but branch protections are currently applied to a rather widely-matching pattern and I don't think we want to backport this to every 3.x branch. It would be possible to restrict this rule to 3.1* because we have no 3.1 release, but then we need to backport to 3.10 branch in case a security problem is found that forces us to make another 3.10 release or something similar.

Copy link
Collaborator

@ulysses4ever ulysses4ever left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Perfect! A clear win, to my eye

.github/workflows/old-ghcs.skip.yml Show resolved Hide resolved
.github/workflows/validate.yml Show resolved Hide resolved
.github/workflows/validate.yml Show resolved Hide resolved
@geekosaur
Copy link
Collaborator Author

geekosaur commented Aug 24, 2024

Does anyone know how to make the RTD job rerun? It's not available in the GHA jobs queue, and it looks like the commit hadn't propagated fully or something when RTD's builder tried to pull it. (ETA: now irrelevant as rebasing on master reran it.)

@geekosaur
Copy link
Collaborator Author

Also, strictly speaking, the post job isn't required because there's only one job defined otherwise. Keeping it makes it consistent with the bootstrap and validate jobs, and allows for adding things to the new workflow later (not that I can think of anything we'd want to add).

@ulysses4ever
Copy link
Collaborator

Would it be possible to un-tag me in the commit message? I'm getting a standalone (not chained) email notification on every update of the commit, which is a little annoying.

@geekosaur
Copy link
Collaborator Author

Should be untagged now.

@geekosaur
Copy link
Collaborator Author

geekosaur commented Aug 24, 2024

One possible remaining issue: the cache rule's comment suggests that it is expecting the cache from the regular validate rules, which may mean that it needs a cache to be created first. I'm not sure when or how this would manifest, though.

Actually, looking at it more, I think we're expecting dist-newstyle and dist-newstyle-validate-ghc-9.4.8 to have been created by validate in validate.yml, but this now runs before those have been created and cached.

@ulysses4ever
Copy link
Collaborator

ulysses4ever commented Aug 25, 2024

Should be untagged now.

You changed it in the PR description but not in the commit message, so it had no effect 🥲

Cache is a black magic to me, sorry. If it's too much of a pain, perhaps, we should reconsider the whole thing...

@geekosaur geekosaur force-pushed the split-out-old-ghcs branch 3 times, most recently from 456963a to abdf69b Compare August 28, 2024 01:49
@ulysses4ever
Copy link
Collaborator

Is it possible to not use cache at all in this job? It runs on Ubuntu only, so it will still, perhaps, be faster than windows and mac jobs on the main validate job, and that's all that we should care for: the new workflow doesn't slow down the overall CI time.

@geekosaur
Copy link
Collaborator Author

It's definitely fast enough; that's part of how I realized cache wasn't doing what was intended. What worries me is wasting (what I consider to be) precious CI resources.

I am not convinced this is a good idea, for the same reason that
Artem wants it: complexity. I'm very worried that this will
break things, although as far as I can tell it should be okay as
long as nobody commits anything overriding branch protection.

All branch protection rules will need to be updated to check for
"Validate old ghcs post job" the same way they check for
"Bootstrap post job" and "Validate post job".
@geekosaur
Copy link
Collaborator Author

I'm parking this. The waste of CI resources is one thing, but the real problem is we can't check that this workflow completed successfully in Validate post job (may not matter much, since it's by definition independent if given its own workflow) or the cabal-head prerelease (essential, must know that this workflow succeeded somehow). The best that can be done is to make a reusable action out of it, then turn validate.yml into a collection of such actions.

But we may want that anyway, depending on what we want to do with #10372.

@geekosaur geekosaur closed this Oct 4, 2024
@geekosaur
Copy link
Collaborator Author

Especially considering #10379, we need to keep these in the main validate run and improve our testing thereof.

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

Successfully merging this pull request may close these issues.

2 participants