Skip to content

Create GH release with buildenvs#158

Open
acolombier wants to merge 4 commits intomixxxdj:2.6from
acolombier:feat/create-gh-release-with-buildenvs
Open

Create GH release with buildenvs#158
acolombier wants to merge 4 commits intomixxxdj:2.6from
acolombier:feat/create-gh-release-with-buildenvs

Conversation

@acolombier
Copy link
Member

This is the first part of the proposal to also use GH release to reduce data round-trips on our runner and improve speed. The current test seems to yield about a ~5x improvement on CI and about the same performance locally, tho slightly slower as the speed tends to fluctuate more.

Note: it also restrains the push and pull_requests CI trigger to prevent double builds

@acolombier acolombier requested review from daschuer March 23, 2025 18:56
Comment on lines +201 to +204
gh release create --repo "${{ github.repository }}" "${RELEASE_NAME}" \
--generate-notes \
--latest \
--target "${{ github.push.after }}" ${{ !endsWith( github.event.ref, '-rel' ) && '--prerelease' || '' }}
Copy link
Member

Choose a reason for hiding this comment

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

This will create a tag like "2.6-557befb" for every single commit. An set it to latest. We have always some issues with every new build. So I am not sure if this is a good idea.

How does the '-rel' thing work?

Copy link
Member

Choose a reason for hiding this comment

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

I have here a branch which uses the build artifact directly form the action tab:
https://github.com/daschuer/mixxx/commits/vcpkg_update_direct/
This has the issue that it is removed after a while, but it works nicely during development.
Do we want to inc operate this model and do the GitHub Release after testing?

Copy link
Member Author

Choose a reason for hiding this comment

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

That's a good point. We could make the non -rel branch create a pre-release instead, and reserve stable, latest releases for the rel branches. Am I right in thinking that -rel branch only gets updated upon successful tests?

Copy link
Member

Choose a reason for hiding this comment

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

This happened probably implicit. The -rel branch mainly produces smaller archive because it only contains one set of files without debug builds.

Copy link
Member Author

@acolombier acolombier Mar 24, 2025

Choose a reason for hiding this comment

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

This happened probably implicit

Great then, I'll leverage that. FYI, a release can always be mannually changed, if we realise that a rel release would turns out to be unstable.
image

Copy link
Member Author

Choose a reason for hiding this comment

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

Actually, it turns out I had already implemented that:

  • *-rel will be published a pre-release
  • * will be published as "stable" release (again we can manually untick if we wanted to, although the follow up release/merge would take precedence anyway)

Both branch type will set the latest alias to themselves, so one can easily retried the latest fresh build, similarly to a main branch on the Mixxx repo.

if: ${{ github.event_name == 'push' }}
id: ghrelease
shell: bash
name: "Creates a release in GitHub if it doesn't exist yet"
Copy link
Member

Choose a reason for hiding this comment

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

We normally have the name on top. You seems to have reverted the sort order of elements.
Please keep the normal sort order.

Copy link
Member Author

Choose a reason for hiding this comment

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

normal sort order

Does that mean just "name first" or is there some kind of bespoke ranking I should follow?

Copy link
Member

Choose a reason for hiding this comment

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

I am not sure if there is something like this.
It is just a good practice to follow the examples in the GitHub Docs:
https://docs.github.com/en/actions/writing-workflows/workflow-syntax-for-github-actions

- name:
  if:
  env:
  run: 

Copy link
Member Author

@acolombier acolombier Mar 24, 2025

Choose a reason for hiding this comment

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

Not sure which example you are referring to. Only order I can find is the following:
image

It is just a good practice

Let's just agree to disagree here. However, I'm not willing to block this pressing optimisation over such a minor point. Here are the used keys:

  • run
  • env
  • if
  • id
  • shell
  • name

Please give me what order you want and I will update that.

Copy link
Member

Choose a reason for hiding this comment

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

I have no strong demands in detail, but it shall be consistent with our other workflow for a good reading experience.

In https://github.com/mixxxdj/mixxx/blob/main/.github/workflows/build.yml we have:

  • name
  • id
  • if
  • uses
  • with
  • shell
  • run
  • working-directory
  • env (above if when required for if)

@JoergAtGithub
Copy link
Member

It would be nice if the user has only a single BUILDENV_URL with a single BUILDENV_SHA256 to specify, also in the case of the archive in multiple parts.
What do you think about generating an additional text file with a list of the archives (and it's checksums) that belongs to this buildenv, like this:

mixxx-deps-2.6-arm64-osx-16f5e4b.filelist

mixxx-deps-2.6-arm64-osx-16f5e4b.parta.zip 1a6f2076da506f9fccd7409cb432b96606ac16eba10b2046e1bbfdb1f7dee456
mixxx-deps-2.6-arm64-osx-16f5e4b.partb.zip f33f430087ea56bc1595424df8738328b508cbfc57a16f3947f64238bfa45929
mixxx-deps-2.6-arm64-osx-16f5e4b.partc.zip 2a1bcb2cacef3b00ac31ac74b5752805f42bc8a25bf8b2b57c6bd25ef9184b4b

Than the user could select the the buildenv just by the URL of this filelist and it's SHA256.

@daschuer
Copy link
Member

Let's look at the whole workflow. For example this:

  • Merging a vcpkg PR
  • Testing using in the local repository using the GitHub artefacts
  • Merging another PR
  • Merging 2.6 branch to 2.6-rel
  • Testing using in the local repository using the GitHub artefacts
  • Creating a mixxxdj/mixxx PR updating to a new environment

Only just before the last step we need the GitHub release.

It would be a major time saver if we have that automated:

  • Push the vcpkg archives to GH releases (ideally the tested artefacts)
  • Commit a file with the new name and the checksums to the feature branch
  • Create a PR

For my understanding a manual started workflow will remove a bit of complexity here and will also ease up our work.
What do you think?

Regarding the checksums of the parts. They are IMHO pointless if the combined file has later a different checksum than the tested
packages form GitHub artefacts.

Proposal just use split and cat to split the original zip. Since we have the checksum it is sufficiently secured.
What do you think?

@daschuer
Copy link
Member

In the past we ALWAYS updated the environments for all at once.
So we can go hear the easy route to maintain one script on our favourite runner to do it for all our targets at once.

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants