Skip to content

Replace earthly #828

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

Open
wants to merge 16 commits into
base: master
Choose a base branch
from
Open

Replace earthly #828

wants to merge 16 commits into from

Conversation

gilligan
Copy link
Contributor

@gilligan gilligan commented May 28, 2025

Description

This PR replaces earthly by instead relying on a more simplistic setup where the build time dependencies are provided via Nix from our flake.nix

Some additional notes:

  • The PR uses the GitHub cache action and the overall runtime is roughly the same
  • The runtime could be improved through parallelization but any optimizations should come as a follow-up
  • The flake.nix has been reduced a lot in size recently and is now just 114 lines
  • With this PR we get a consistent setup where the CI builds are easy to reproduce locally

Checklist

  • Commit sequence broadly makes sense and commits have useful messages.
  • The size limit of 400 LOC isn't needlessly exceeded
  • The PR refers to a JIRA ticket (if one exists)
  • New tests are added if needed and existing tests are updated.
  • New code is documented and existing documentation is updated.
  • Relevant logging and metrics added
  • Any changes are noted in the changelog.md for affected crate
  • Self-reviewed the diff

@gilligan
Copy link
Contributor Author

@skylar-simoncelli All pre-merge steps seem to work. I haven't tested post-merge so I'm gonna assume that's where all my typos are and everything breaks :)

@skylar-simoncelli
Copy link
Contributor

Some issues spotted on a quick review:

  • You have removed some elements of the ./to-build container but not all. I would recommend keeping this mechanism, the idea of it is:
    • It provides the post-merge run the ability to build with the Earthfile / build logic from the branch being merged (in case it is being updated) while still tagging the pushed image with the master branch SHA and not the merge branch SHA
    • Gives the ability for the workflow_dispatch to build a certain commit while retaining the ability to use the build script (Earthfile or devshell + workflow) from master or any given secondary SHA. In this current form it will not be possible to build old commits if the devshell build is failing without forking that SHA to a new SHA with a patch. Instead of just making the patch in a seperate branch so you can still build the SHA you want to build
  • workflow_dispatch uses github.event.pull_request.head.sha, which won't exist. It must use inputs.sha (target branch) and ${{ github.sha }} (source branch the workflow_dispatch is ran from)
  • chain-spec generation is using --chain local for all, when it should only use --chain local for devnet and --chain staging for ci-preview, staging-preview and staging-preprod
  • workflow_dispatch has changed artifact name from partner-chains-node-linux-artifact to partner-chains-node-artifact which will cause the upload-to-s3-workflow-dispatch job to fail as it has this artifact name hardcoded
    removed the python licenses check but haven't implemented the community composite yet

In current state it looks like pre-merge is correct, and post-merge and workflow_dispatch are 90% there with just these minor issues

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