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: automatically bump node parts and use remote build #181

Merged
merged 8 commits into from
Nov 13, 2023
Merged

ci: automatically bump node parts and use remote build #181

merged 8 commits into from
Nov 13, 2023

Conversation

jnsgruk
Copy link
Member

@jnsgruk jnsgruk commented Nov 1, 2023

Fixes #180. This PR is dependent on #179 and should be merged after.

Because we now have multiple parts, which are dependent on the version of Signal, we need to augment the CI to ensure that when we bump the Signal version, we also bump the parts versions if required.

This PR also switches the build process to using snapcraft remote-build in a matrix that creates a job per architecture.

This will be hard to review until #179 is merged, but the diff can be seen cleanly here: 7eb0bff

@jnsgruk
Copy link
Member Author

jnsgruk commented Nov 1, 2023

@kenvandine :)

@jnsgruk
Copy link
Member Author

jnsgruk commented Nov 5, 2023

@merlijn-sebrechts I've added the Launchpad build stuff here.

@jnsgruk jnsgruk changed the title ci: automatically bump node parts ci: automatically bump node parts and use remote build Nov 5, 2023
Copy link
Member

@merlijn-sebrechts merlijn-sebrechts left a comment

Choose a reason for hiding this comment

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

Required changes:

  • Remove the use of artifacts if we don't need it. Artifacts are expensive.

Optional: Publishing workflow

While the building aspect seems fixed with this PR, we also need to rework the publishing workflow, so we can publish both architectures to stable too. We can do this here, or in a separate PR.

I think the ideal workflow is as follows:

  1. After a build, an issue gets created that explains for which architectures there is a new revision.
  2. Testers can either promote individual versions (using the existing command), or promote multiple versions at once (using /promote 22 23 stable, or something like this)
  3. The issue only closes once all revisions are pushed to stable.

Changes needed:

  • Change .github/testing-issue-template.md to reflect multiple revisions are pushed. Ideally it says for which architectures there are new versions.
  • Change the promote-to-stable workflow so it can receive multiple revisions to promote to stable at once.
  • Change the promote-to-stable workflow so it only closes the issue once all revisions are pushed to stable.

.github/workflows/snap-store-publish-to-candidate.yml Outdated Show resolved Hide resolved
@merlijn-sebrechts
Copy link
Member

Ah, and one final thing; we might also need to update the test-snap-can-build workflow, so we can find issues with the arm64 build before we push:

- uses: snapcore/action-build@v1

@merlijn-sebrechts
Copy link
Member

FYI: added an issue in action-build to support remote-build. https://github.com/snapcore/action-build/issues/68

@jnsgruk
Copy link
Member Author

jnsgruk commented Nov 8, 2023

@merlijn-sebrechts I think that's all done.

Unfortunately it seems that remote-build --build-for doesn't like being used to just pick a particular arch to be built when they're already specified in the snapcraft.yaml, so I've made some changes there. I'll also have a chat with the starcraft team about making that behave a bit more nicely, and then we should be able to tidy this up.

The /promote command now also supports taking multiple revisions.

The only thing we now need is a new Github Secret setting called LP_BUILD_SECRET, then it should all go green!

Copy link
Member

@merlijn-sebrechts merlijn-sebrechts left a comment

Choose a reason for hiding this comment

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

Tiny note: You'll also need to update the workflow_call in the publish to candidate workflow to add an additional required secret.

.github/workflows/test-snap-can-build.yml Outdated Show resolved Hide resolved
.github/workflows/test-snap-can-build.yml Outdated Show resolved Hide resolved
@merlijn-sebrechts
Copy link
Member

FYI: LP_BUILD_SECRET is now set org-wide, BTW. For a new, clean LP user snapcrafters-bot.

@jnsgruk
Copy link
Member Author

jnsgruk commented Nov 8, 2023

Okay - I've reverted the build action for now to use the Github Action as before. FWIW I don't think using the LP secret in a PR workflow is an issue, provided the repo is configured such that workflows only run after a review from a maintainer (that way the reviewer can check there is no funny business). I did some looking at the conditions for running workflows, and I think checking user membership in a group might be beyond what's possible - but I'll circle back to it later.

The other point here is that in reality the LP credential is somewhat "invaluable" because literally anyone can generate one with the same privileges, but I agree we shouldn't be flippent about that.

Let me know if you'd like any further changes.

@jnsgruk
Copy link
Member Author

jnsgruk commented Nov 13, 2023

@merlijn-sebrechts can we get this merged and check it all works?

If you could add me as a collaborator on the signal-desktop snap I can keep an eye on the builds?

@merlijn-sebrechts
Copy link
Member

@jnsgruk Are you asking to become a maintainer of this repo? All snapcrafters are, so do you want to join snapcrafters? :)

Copy link
Member

@merlijn-sebrechts merlijn-sebrechts left a comment

Choose a reason for hiding this comment

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

LGTM!

@merlijn-sebrechts merlijn-sebrechts merged commit 60d3e79 into snapcrafters:candidate Nov 13, 2023
1 check passed
@jnsgruk jnsgruk deleted the ci-bump-parts branch November 13, 2023 09:34
@jnsgruk
Copy link
Member Author

jnsgruk commented Nov 13, 2023

@merlijn-sebrechts correct - happy to become a maintainer of this :) (and thus a snapcrafter!)

@merlijn-sebrechts
Copy link
Member

Done, welcome to the fold!

We also have a telegram channel. Send me your telegram handle if you want to join that.

@jnsgruk
Copy link
Member Author

jnsgruk commented Nov 13, 2023

Perfect thanks! My telegram handle is the same as here: @jnsgruk

Can you make me a collaborator on signal-desktop on snapcraft.io?

@soumyaDghosh
Copy link
Member

@jnsgruk @merlijn-sebrechts How can we get the launchpad build secret?

@merlijn-sebrechts
Copy link
Member

@soumyaDghosh I add it to your repo once you have a PR with the GitHub actions ready

For more info on how i create these secrets, and what they are, take a look at https://github.com/snapcrafters/.github/wiki/Permissions-and-Secrets

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.

[Enhancement]: Automate update of parts versions
3 participants