github CI: run on forks

[catap]

Right now CI is runs only when someone is created PR.

It is quite simple to run CI inside fork's by simple allowing it.

Main repo contains only one master branch and I've excluded it from workflow.

Thus, you may see that this commit is started to build on my repository: https://github.com/catap/macports-ports/actions

Why? It allows to run CI without open a PR.

[catap]

@cjones051073 / @jmroot what do you think about this?

[mascguy]

Added @jmroot, @ryandesign, and @cjones051073 as reviewers, for their thoughts/comments.

[mascguy]

Kirill, are you also interested in tackling this as well?

63974 - GitHub CI: update PR workflow to cancel outdated jobs on push

[catap]

@mascguy sure :)

An example how it works you may see here: https://github.com/catap/macports-ports/actions/runs/1485511486

[ryandesign]

Added @jmroot, @ryandesign, and @cjones051073 as reviewers, for their thoughts/comments.

i have no idea.

[cjones051073]

I have to confess I do not really understand the description here well enough to get what exactly this PR is designed to do ?

[catap]

@cjones051073 right now when you fork macports repository to create PR, you can't run CI on github actions without creating of PR.

As result anyone who would like to create a PR can't test his changes on github CI without open a PR. Why? Because it is strictly forbidden.

With this trivial changes I can push to my fork of the repo, and run CI inside my fork. Without wasting main repo resources. This allows me to open PR only when a branch inside my fork had a green mark.

The second changes allow to cancel the useless build when someone pushed a new changes into the same branch.

[cjones051073]

Ok thanks, I get it now. How do you start a ci test build on a branch without a PR ?

[catap]

@cjones051073 by pushing a branch to my fork on github. For example a branch which I've used to create this PR https://github.com/catap/macports-ports/tree/ci-on-forks is starting to run CI inside my fork.

It also applied to branches that was created inside my fork and which hasn't got any PR. For example: https://github.com/catap/macports-ports/tree/kir-test

[cjones051073]

So its automatic ? I am not sure we want every time someone pushes a change to a private branch a ci test to be started. It should be optional and something that can be started on demand as required.

[catap]

@cjones051073 yes, it is automatic.

Anyway, it is outside of the repositoru or organization limit, it is consuming a limit for user who own this branch.

[cjones051073]

Thats fine, but I am still not convinced it should be purely automatic. It needs I think to be possible to enable/disable it as required.

[catap]

@cjones051073 base on my experience in software development the only CI that is working is one which is run automatically :)

[mascguy]

i have no idea.

@ryandesign and @jmroot, please take a look at the 2nd commit, re: cancelling outdated/defunct CI jobs. This is something that we really need, as multiple commits for a given PR clog up the CI queue. Auto-cancelling previous outdated jobs is a no-brainer, and would significantly improve the CI experience.

If there are no objections, I'd like to at least commit that change ASAP.

Thoughts?

[mascguy]

@ryandesign and @jmroot, please take a look at the 2nd commit, re: cancelling outdated/defunct CI jobs. This is something that we really need, as multiple commits for a given PR clog up the CI queue. Auto-cancelling previous outdated jobs is a no-brainer, and would significantly improve the CI experience.

Committed to master, with author set to Kirill so that he receives credit for it:
github ci: cancel outdated jobs on push

Kirill, you can remove the 2nd commit. And thanks so much for showing us how to do this!

[ryandesign]

@ryandesign and @jmroot, please take a look at the 2nd commit, re: cancelling outdated/defunct CI jobs. This is something that we really need, as multiple commits for a given PR clog up the CI queue. Auto-cancelling previous outdated jobs is a no-brainer, and would significantly improve the CI experience.

Again, I have no idea. It sounds like a good thing to do, but I don't know how.

[catap]

@ryandesign it is done at this commit. https://github.com/catap/macports-ports/actions/runs/1485511486 is example of such canceled build.

[catap]

@mascguy rebases to last master and removed the second commit.

[pmetzger]

Provided this is counted against the user's limits and not the macports organization limits it seems like a reasonable idea in principle.

[mascguy]

Provided this is counted against the user's limits and not the macports organization limits it seems like a reasonable idea in principle.

Agreed! Being able to finally pre-validate our own branches - without having to create a PR, AND without counting against MacPorts' org limits - is a revelation. Awesome stuff!

[mascguy]

Thats fine, but I am still not convinced it should be purely automatic. It needs I think to be possible to enable/disable it as required.

@cjones051073 We all have the ability to enable/disable automatic GitHub Actions, within our respective forks of a given repo. So that's already available.

https://github.com/<your_github_user>/macports-ports/settings/actions

Does that make sense?

[catap]

@mascguy from my point of view any branch is fine :)

Anyway, all senior contributors are pushing directly to repository, and this helps only to someone who opens PR to run CI before you open PR.

[reneeotten]

I apologize to be late to the party here... I don't think there is much benefit to this. It should certainly not be done automatically on users' forks and people are supposed to test things locally before submitting a PR anyway. I doubt that any occasional contributor will take the time to switch this on on their repository and people who are actively involved here will (or at least should) build stuff on their local machine before opening a PR. I'd conjecture that most contributors run any of the latest three macOS versions and if a port builds there it's very unlikely that it will fail on any of the systems covered by the CI.

[mascguy]

I apologize to be late to the party here... I don't think there is much benefit to this. It should certainly not be done automatically on users' forks and people are supposed to test things locally before submitting a PR anyway. I doubt that any occasional contributor will take the time to switch this on on their repository and people who are actively involved here will (or at least should) build stuff on their local machine before opening a PR. I'd conjecture that most contributors run any of the latest three macOS versions and if a port builds there it's very unlikely that it will fail on any of the systems covered by the CI.

Well, all of that has been discussed ad nauseam, per the prior comments. And this behavior will only occur when folks explicitly utilize the branch naming convention we ultimately agree upon.

Finally, and just as importantly, this will not count against MacPort's CI time.

Given all of that, do you still oppose this Renee...?

[catap]

@reneeotten right now I haven't any way to run CI on my fork on patches that I'm pushing => I need open PR to run CI and maybe to start digging.

This PR allows me to stop waste macports' CI resources to test :)

[mojca]

My suggestion would be to:

• prevent CI on pull requests if the branch name starts with no_ci (I don't care if all of no_ci/something, no_ci-something, no_ci_something work); we could optionally (this may be implemented later) still test whether portindex works etc., just not actually attempt to build the ports
• trigger CI on individual commits without PR if the branch name starts with run_ci (ci might also be ok, but we probably don't want it to trigger on something like cidr as that might be unexpected; alternatively ci/ or ci_ sounds ok as well)

The first feature to be able to easily avoid CI sounds super useful to me.
But I would also strongly prefer if pushes to branches would only be built "on request".

[Gcenx]

@mojca If only manual CI is desired then adding workflow_dispatch: would accomplish this.

[reneeotten]

@reneeotten right now I haven't any way to run CI on my fork on patches that I'm pushing => I need open PR to run CI and maybe to start digging.

that's why you have to build your changes in trace-mode locally before opening a PR. If that works then the chances that the CI will catch a failure that you didn't see locally is practically zero. So yes, I've read the discussion above and understand that it doesn't count to MacPorts' CI time; even considering all of that I still don't think this will be helpful.

[edit: in fact people could view this as an alternative of building it locally themselves, whereas it is no substitute for that. One should always build it locally, run the test phase if possible and make sure the binaries actually work; the latter two things cannot be done with the CI. So in my opinion there will be no time-saving or benefit.]

[catap]

@reneeotten in my case I'm testing everything on my laptop with macOS 12. CI allows to test the same port on 10.14, 10.15 and 11 :)

[pmetzger]

I doubt that any occasional contributor will take the time to switch this on on their repository and people who are actively involved here will (or at least should) build stuff on their local machine before opening a PR.

I would like to have this facility myself. I only test on one OS (the current release) but MacPorts supports much older releases.

[reneeotten]

I doubt that any occasional contributor will take the time to switch this on on their repository and people who are actively involved here will (or at least should) build stuff on their local machine before opening a PR.

I would like to have this facility myself. I only test on one OS (the current release) but MacPorts supports much older releases.

except that the CI will only test macOS 11 and 10.15; that will not likely result in any failures if it builds on the most current OS version.

[ryandesign]

that will unavoidably further steal resources available to our project.

That's a misunderstanding. The resources are taken from the CI allocation for the person with the fork, not our CI. It does not impact our CI pipeline.

It's not a misunderstanding. There are a finite number of CI machines made available by Microsoft or whoever provides them. If those machines are tied up doing work, regardless of whose CI allocation it counts against, then those machines are not available to do other work.

Since it's been explained that this change will not automatically cause unwanted extra CI usage for all forks, but will just enable users to opt in to it, for the couple users who want to be able to do it, sure, I guess we can merge this.

And now I'm again unsubscribing from this PR.

[pmetzger]

It's not a misunderstanding. There are a finite number of CI machines made available by Microsoft or whoever provides them. If those machines are tied up doing work, regardless of whose CI allocation it counts against, then those machines are not available to do other work.

That's true in the sense that in a theoretical sense Azure (which is now the cloud service backing Github IIRC) could run out of compute resources. If I am running a CI pipleline against my repository, it does not impact the fact that you are running one against your repository. They do not use the same resources.

[pmetzger]

Since it's been explained that this change will not automatically cause unwanted extra CI usage for all forks, but will just enable users to opt in to it, for the couple users who want to be able to do it, sure, I guess we can merge this.

Okay, cool then.

[neverpanic]

Ok all, thanks for your explanations. To summarise, as long as

1. Unless the user has actively enabled ci actions in their macports fork, so by default assuming they have not nothing will change for them, and
2. If they do enable ci actions any resources will never be accounted against macports itself, and only the user in question

then I am personally a lot more comfortable with this being merged.

For the record, I also agree with this.

[pmetzger]

Okay, so are we getting close to merging this?

[neverpanic]

I guess somebody should announce it on the developer mailing list, set a deadline, and then merge it. That announcement should probably also include a reminder that branches shouldn't be pushed to macports-ports – some developers have occasionally done that in the past, and it would trigger CI runs with this change that are probably not expected.

[catap]

ping here!

[pmetzger]

@catap Has it been announced to the dev list? If so I'll do a merge.

[catap]

@pmetzger I have no idea. I haven't got access to dev list

[mascguy]

Kirill, you can subscribe to the various lists here:

https://lists.macports.org/mailman/listinfo

You'll definitely want to be on the Dev list, along with Users.

If you're also interested in updates across all of our Trac issues, subscribe to Tickets.

[pmetzger]

Indeed, @catap, given how many merge requests you make, I'm quite surprised you're not on the list. Please do join!

[catap]

@pmetzger / @mascguy I just did.

I'll make an announce after I have some sleep ;)

[catap]

@mascguy may I ask you to merge it?

[mascguy]

@mascguy may I ask you to merge it?

Just to confirm, did you announce this on the list? (Please humor me, as I can't remember! LOL)

[catap]

@mascguy may I ask you to merge it?

Just to confirm, did you announce this on the list? (Please humor me, as I can't remember! LOL)

I did in February with deadline of merge as 1st March

[reneeotten]

it seems that this is now running by default on my fork when I push a branch, even though I haven't told it to do so.... that wasn't the consensus here in this PR I think. Can we somehow still make it happen that this only runs when a user has explicitly requested this - or am I missing something here?

[mascguy]

it seems that this is now running by default on my fork when I push a branch, even though I haven't told it to do so.... that wasn't the consensus here in this PR I think. Can we somehow still make it happen that this only runs when a user has explicitly requested this - or am I missing something here?

Yes, something seems a bit odd. For example, I noticed the following when reviewing CI actions.

Marius's open PR for Octave, for example, shows the following CI checks:

Along with the following GitHub action detail:

The earlier CI run is associated with a commit, while the latter is associated with the PR?

I haven't reviewed the GitHub changelog yet, but I'm wondering if there was a global Action-related behavior change...?