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

ci: split PR build and release build to manually trigger builds of artifacts #838

Closed
wants to merge 18 commits into from

Conversation

simonsan
Copy link
Contributor

@simonsan simonsan commented Aug 21, 2023

This changes the CI release workflow to be split into release builds of release/ and main branch and PR builds (which are triggered manually) for PRs with the label S-build.

I think this is beneficial, because we run the full build pipeline for each commit, which is a waste of resources. When we can just trigger it manually once we think a PR is in a state we want to create artifacts (e.g. for testing)

The flow is as follows:

  1. run checks, tests, security audit, and lints automatically
  2. wait for label S-build
  3. set label S-build
  4. build artifacts and push them to actions artifact storage
  5. remove label S-build (for retriggering the build workflow)

This needs to be merged after #843 as the release workflow needs to be changed WRT to uploading binaries which was not working before.

@simonsan simonsan added C-enhancement Category: New feature or request A-ci Area: Issues/Pull requests related to CI labels Aug 21, 2023
@simonsan simonsan changed the title ci: split CI build and release build to manually trigger builds of artifacts ci: split PR build and release build to manually trigger builds of artifacts Aug 21, 2023
@simonsan simonsan added the S-build Status: Command to trigger a build of platform artifacts label Aug 21, 2023
…rsions

Signed-off-by: simonsan <14062932+simonsan@users.noreply.github.com>
@simonsan simonsan added S-build Status: Command to trigger a build of platform artifacts and removed S-build Status: Command to trigger a build of platform artifacts labels Aug 21, 2023
Signed-off-by: simonsan <14062932+simonsan@users.noreply.github.com>
@simonsan simonsan added S-build Status: Command to trigger a build of platform artifacts and removed S-build Status: Command to trigger a build of platform artifacts labels Aug 21, 2023
Signed-off-by: simonsan <14062932+simonsan@users.noreply.github.com>
@simonsan simonsan added S-build Status: Command to trigger a build of platform artifacts and removed S-build Status: Command to trigger a build of platform artifacts labels Aug 21, 2023
Signed-off-by: simonsan <14062932+simonsan@users.noreply.github.com>
@simonsan simonsan removed the S-build Status: Command to trigger a build of platform artifacts label Aug 21, 2023
Signed-off-by: simonsan <14062932+simonsan@users.noreply.github.com>
@simonsan simonsan added the S-build Status: Command to trigger a build of platform artifacts label Aug 21, 2023
Signed-off-by: simonsan <14062932+simonsan@users.noreply.github.com>
@simonsan simonsan removed the S-build Status: Command to trigger a build of platform artifacts label Aug 21, 2023
@simonsan simonsan added the S-build Status: Command to trigger a build of platform artifacts label Aug 21, 2023
Signed-off-by: simonsan <14062932+simonsan@users.noreply.github.com>
@simonsan simonsan added S-build Status: Command to trigger a build of platform artifacts and removed S-build Status: Command to trigger a build of platform artifacts labels Aug 21, 2023
Signed-off-by: simonsan <14062932+simonsan@users.noreply.github.com>
@simonsan simonsan added S-build Status: Command to trigger a build of platform artifacts and removed S-build Status: Command to trigger a build of platform artifacts labels Aug 21, 2023
…roach

The problem could lay somewhere else, the reason for it not working, could be, that from a PR we don't have access to the owned repository. It might just magically work, when it is merged.

Signed-off-by: simonsan <14062932+simonsan@users.noreply.github.com>
@simonsan
Copy link
Contributor Author

simonsan commented Aug 21, 2023

2023-08-22 02_10_35-ci_ split PR build and release build to manually trigger builds of artifacts · r

The problem could lay somewhere else, the reason for it not working could be that from a PR we don't have access to the owned repository. It might just magically work, when it is merged.

If that's still not the case, the last approach would be, to use something like C-bug, C-enhancement and other labels as triggers, where we would like to have artifacts. In the majority of cases, though, I would say, that we don't need artifacts of rustic and probably rather build it ourselves locally.

So all the lints, tests, checks, and audits should be giving us a good overview, whether we will run a full cross-platform build or not. With improving the test coverage, running tests should be more meaningful anyway. There are some platforms, where at least one build before merging is beneficial, this use case we already have with setting S-build manually.

Currently, we would need to remove and re-set the S-build flag manually, if we want more cross-platform build. I do believe that this should solve itself after merging, though.

@simonsan simonsan added S-waiting-for-review Status: PRs waiting for review and removed S-build Status: Command to trigger a build of platform artifacts labels Aug 21, 2023
Signed-off-by: simonsan <14062932+simonsan@users.noreply.github.com>
@simonsan simonsan marked this pull request as draft August 22, 2023 05:23
@simonsan simonsan removed the S-waiting-for-review Status: PRs waiting for review label Aug 22, 2023
Signed-off-by: simonsan <14062932+simonsan@users.noreply.github.com>
@simonsan simonsan added S-build Status: Command to trigger a build of platform artifacts and removed S-build Status: Command to trigger a build of platform artifacts labels Sep 11, 2023
@simonsan
Copy link
Contributor Author

gh api --method DELETE -H "Accept: application/vnd.github+json" -H "X-GitHub-Api-Version: 2022-11-28" /repos/rustic-rs/rustic/issues/838/labels/S-build i tested the API call manually, and it works, I assume it will also work from the main branch when it's within the repository.

@simonsan simonsan marked this pull request as ready for review September 11, 2023 02:22
@simonsan simonsan added the S-waiting-for-review Status: PRs waiting for review label Sep 11, 2023
@simonsan
Copy link
Contributor Author

Also, this: Could you give it a rough look, @aawsome because I'm unsure if the direction is fine? Thanks a lot!

Personally, I think it would help a lot to have only the basic checks running, and the platform builds only when we explicitly ask for them.

@simonsan
Copy link
Contributor Author

We have a few possibilities with the nightly builds on https://github.com/rustic-rs/nightly now:

  • we could get rid of the release workflow from main completely for now and create one to have a release PR to make it easier to release more often
  • we could edit this workflow here and make it listen to workflow_dispatch so we can trigger it in the actions tab if we need to
  • or we could merge this here, I'm pretty positive that it would work like that, and if not we can still go down the route with the workflow_dispatch

@simonsan simonsan marked this pull request as draft September 12, 2023 09:30
@simonsan simonsan closed this Sep 12, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-ci Area: Issues/Pull requests related to CI C-enhancement Category: New feature or request S-waiting-for-review Status: PRs waiting for review
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant