Skip to content

Conversation

elliedori
Copy link
Contributor

@elliedori elliedori commented Sep 2, 2020

Github Actions doesn't currently have an elegant way to prevent builds on forks. Following the suggestion outlined here, I added checks for each job to ensure it only runs on spring-projects/spring-session.

I opened a PR on my forked version as well as updated my forked master, and confirmed that no builds steps ran.

See https://github.com/elliedori/spring-session/actions/runs/236392775 and https://github.com/elliedori/spring-session/actions/runs/236394687 for more.

This PR resolves #1678

@spring-projects-issues spring-projects-issues added the status: waiting-for-triage An issue we've not yet triaged label Sep 2, 2020
@vpavic
Copy link
Contributor

vpavic commented Sep 2, 2020

Yeah, this seems like a very unfortunate limitation of GitHub Actions.

With this approach those builds still run, but end up being short-circuited due to the check. Also if you forget to add the check when adding a new job (which isn't unrealistic at all), the problem is there again.

Having a shortcoming like this one, to me GitHub Actions doesn't appear mature enough yet to be a primary CI/build runner.

@elliedori
Copy link
Contributor Author

You bring up a good point @vpavic. At this point the team has decided on moving forward with Github Actions, so we're currently finding the best ways to adapt it for our use case. If you'd prefer not to see the short-circuited run on your fork, you can also disable Github Actions in your fork of the project.

Screen Shot 2020-09-02 at 11 30 55 AM

Hope this helps, and thanks for bringing the original issue to our attention.

@vpavic
Copy link
Contributor

vpavic commented Sep 2, 2020

I did that for my fork already, but (as of time of writing) there are 909 other forks out there. 🙂

Keep in mind that this makes it very easy to spam people's inboxes, which is nice way to greet potential contributors.

@elliedori
Copy link
Contributor Author

elliedori commented Sep 3, 2020

Ah that's a good point @vpavic . I have email notifications disabled so I'm not feeling that pain. Are you getting emails even for skipped builds?

For context, the team's been wanting to port builds over the Github Actions for cleaner integration / having everything in one place given that the project lives on Github. If this is creating too much pain for contributors, then your point about not moving over to GA may be a valid one. That would be unfortunate, as we've already invested a lot of time getting GA pipelines up and running for this project and the spring security one.

I'll defer to @eleftherias in this case – what are your thoughts on how we should proceed?

@vpavic
Copy link
Contributor

vpavic commented Sep 3, 2020

Are you getting emails even for skipped builds?

That's fully configurable (see here), I personally have it enabled only for failed ones. But someone else might have it enabled for all.

Maybe it's just me, but I really wish Actions were an opt-in feature. Or at the very least if organization/repository owners had control whether Actions are enabled by default for forks. Having to workaround these issues by modifying jobs is not pretty.

@rwinch
Copy link
Member

rwinch commented Sep 8, 2020

I agree that working around this in the build is not pretty, but it should have very minimal impact to most users. In the long run, I still feel GitHub Actions is where we prefer to run our builds. It has a large community supporting it with lots of integrations. Additionally, as GitHub has become a standard for code management, I think that GitHub Actions will be a standard CI. Finally, it makes sense that users can run the CI on their side. After all, if you fork a project it is nice to know the health of your fork as well.

@vpavic
Copy link
Contributor

vpavic commented Sep 12, 2020

I disagree that it makes sense to run the builds on the fork. Fork is just a copy of a project typically created with the intention of contributing to upstream. Changes originating from the fork will get verified either way on the upstream once PR is opened. When that flow completes, vast majority of the forks will become stale (as most folks are not regular contributors) and running (scheduled) builds against those is just wasteful. Forks made with other purpose, such as creating custom builds or diverging the fork completely, will either way require you to set up the CI as if it was a new project.

The default GitHub Actions behaviour for forks is a major annoyance to me as it can quite easily end up being a spam machine, either the way like this project's builds do currently or using something else from the Actions ecosystem e.g. see the PRs Dependabot opened against my Testcontainers fork.

Don't get me wrong, I like GitHub Actions pretty much in general and agree that having CI in the sample place where the code is really nice. After all, that's one of the reasons many preferred Gitlab to GitHub over the years. However, as evident several examples, things are not there yet.

Also I'd be wary about one more thing when moving the CI completely to GitHub Actions - as adoption scales, I don't see it as unlikely that they imposing a limit of minutes to free tier at some point, similarly like Gitlab have recently.

@vpavic
Copy link
Contributor

vpavic commented Oct 3, 2020

Speaking of limitations, that didn't take too long. 🙂 This landed in my inbox the other day:

We’re making two changes to the usage policy for GitHub Actions. These changes will enable GitHub Actions to scale with the incredible adoption we’ve seen from the GitHub community. Here’s a quick overview:

  • Starting today, scheduled workflows will be disabled by default in new forks of public repositories.
  • Scheduled workflows will be disabled in public repos with no activity for 60 consecutive days.

First point is actually a good thing, but second one is a bit upsetting.

@eleftherias eleftherias self-assigned this Jul 6, 2021
@eleftherias eleftherias added in: build An issue in the build type: bug A general bug and removed status: waiting-for-triage An issue we've not yet triaged labels Jul 6, 2021
@eleftherias eleftherias merged commit 8b5b370 into spring-projects:main Jul 6, 2021
@eleftherias eleftherias added this to the 2.6.0-M1 milestone Jul 6, 2021
@eleftherias eleftherias added the status: duplicate A duplicate of another issue label Jul 6, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

in: build An issue in the build status: duplicate A duplicate of another issue type: bug A general bug

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Prevent GitHub Actions build from running on forks

5 participants