feat(Earthfile): Add checksums and tarballs for crank #5779
Conversation
There was a problem hiding this comment.
This looks like a reasonable approach with reasonable commands @tampakrap!
I tried taking this for a local test spin to verify it works and what the resulting output artifacts look like. The earthly +build works OK for me, I get no errors.
However, I don't see the output files in the _output directory. All I see written there is the _output/charts/ dir. To be clear, this looks to be an issue from before your PR as I don't see any of the crank/crossplane binaries there either:
❯ tree _output
_output
└── charts
└── crossplane-0.0.0-1717543403-806ce6f8.tgz
1 directory, 1 file
Do you have any insight into where these _output files are getting written to? I tried poking around the earthly build container and I couldn't find an easily understandable path for them there either, just various paths under ./tmp/earthly/buildkit/runc-overlayfs/snapshots/snapshots, which look transient in nature. Is there a consistent/final location where all these _output files are expected to be written to? I'd love to find those so I can test these changes further.
Related, but out of scope of this PR: I'd also like to understand how best to support the common scenario from the past of building the crossplane CLI crank binary and running it locally - hard to do that now without the files being written to the _output dir of the host file system 😇
| SAVE ARTIFACT crank AS LOCAL _output/bin/${GOOS}_${GOARCH}/crank | ||
| SAVE ARTIFACT crank.sha256 AS LOCAL _output/bin/${GOOS}_${GOARCH}/crank.sha256 | ||
| SAVE ARTIFACT crank.tar.gz AS LOCAL _output/bundle/${GOOS}_${GOARCH}/crank.tar.gz |
There was a problem hiding this comment.
is there particular "standard" guidance that helped you land on bundle as the name of this dir? curious to see if there's an already defined practice we're following :)
There was a problem hiding this comment.
I don't think there is any standard naming applied here, and I don't really have a preference on how to name the dir. I chose bundle because this is what up uses 😉 https://cli.upbound.io/stable/v0.30.0/bundle
|
ah, I notice that the binary files are saved to |
correct, thanks for the reminder, I also noticed it and forgot to send the PR, I'm submitting it now |
|
|
With #5782 now merged, @tampakrap do you want to fetch/rebase on later in master, then we can review testing and reviewing this PR? 🙏 |
let's finish with #5781 first because it collides with this one |
|
sweet, #5781 is merged now too - want to rebase this one and we'll start testing/reviewing? 💪 |
- The checksums will help people to verify that their downloaded binary is not corrupted - The tarballs provide faster downloads. This is beneficial when downloading crank often, for example in CI jobs. - The tarballs and checksums can be useful for creating distro and brew packages, see crossplane#4899 Signed-off-by: Theo Chatzimichos <[email protected]>
tampakrap
force-pushed
the
earthfile_checksums_tarballs
branch
from
806ce6f
to
9fbdb7e
Compare
|
rebased |
There was a problem hiding this comment.
After reviewing and testing this functionality, these changes look good to me @tampakrap! See some testing notes below.
The only question I still have, which isn't strictly related to this PR, is that I'm not seeing the binaries, checksums, and tarballs published to https://releases.crossplane.io/build/merge/. I wanted to validate that these new checksum/tarball artifacts were being published alongside the existing artifacts, and I expected them to be from the conversation with @negz in https://github.com/crossplane/crossplane/pull/5765/files#r1621518143 that declared:
PRs should be uploaded to https://releases.crossplane.io/build/merge...
So not blocking, because I think publishing to releases.crossplane.io isn't happening for any PRs right now, but something to think about.
Testing information for this PR:
After building for local Mac M1 with earthly +build and for Windows with earthly --platform=windows/amd64 +go-build, I see all the expected binaries, checksums, and tarballs:
❯ tree _output
_output
├── bin
│ ├── darwin_arm64
│ │ ├── crank
│ │ ├── crank.sha256
│ │ ├── crossplane
│ │ └── crossplane.sha256
│ └── windows_amd64
│ ├── crank.exe
│ ├── crank.exe.sha256
│ ├── crossplane.exe
│ └── crossplane.exe.sha256
├── bundle
│ ├── darwin_arm64
│ │ ├── crank.tar.gz
│ │ └── crank.tar.gz.sha256
│ └── windows_amd64
│ ├── crank.tar.gz
│ └── crank.tar.gz.sha256
└── charts
└── crossplane-0.0.0-1718181266-9fbdb7e5.tgz
The sha256 files look to be correct and verifiable with something like:
❯ echo "$(cat crank.sha256) crank" | sha256sum --check
crank: OK
The tarballs appear to have the correct content, e.g. crank and its sha256 file:
_output/bundle/darwin_arm64
❯ ll
total 68664
drwxr-xr-x@ 4 jared staff 128B Jun 12 17:50 .
drwxr-xr-x@ 5 jared staff 160B Jun 13 17:37 ..
-rw-r--r--@ 1 jared staff 34M Apr 16 2020 crank.tar.gz
-rw-r--r--@ 1 jared staff 64B Apr 16 2020 crank.tar.gz.sha256
_output/bundle/darwin_arm64
❯ tar -xzf crank.tar.gz
_output/bundle/darwin_arm64
❯ ll
total 229432
drwxr-xr-x@ 6 jared staff 192B Jun 13 17:37 .
drwxr-xr-x@ 5 jared staff 160B Jun 13 17:37 ..
-rwxr-xr-x@ 1 jared staff 78M Jun 12 17:50 crank
-rw-r--r--@ 1 jared staff 64B Jun 12 17:50 crank.sha256
-rw-r--r--@ 1 jared staff 34M Apr 16 2020 crank.tar.gz
-rw-r--r--@ 1 jared staff 64B Apr 16 2020 crank.tar.gz.sha256
aha, @negz explained this to me offline. The publishing step won't run unless the So it is expected on this PR that publishing binaries won't run. Given that the publishing step just uploads all of |
|
Apart from the functionality though, I would like an opinion on the naming and the structure. Are we fine keeping the dir name as |
|
The current layout and naming is fine for me @tampakrap! ✅ |
Description of your changes
I have:
earthly +reviewableto ensure this PR is ready for review.Added or updated unit tests.Added or updated e2e tests.Linked a PR or a docs tracking issue to document this change.Addedbackport release-x.ylabels to auto-backport this PR.Need help with this checklist? See the cheat sheet.