feat: print build logs on verbose mode#499
Conversation
🦋 Changeset detectedLatest commit: 6c4c342 The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
Coverage Report
📁 File Coverage (19 files)
|
brunomenezes
force-pushed
the
feat/print-build-logs-on-verbose-opt
branch
from
b508cef to
2243094
Compare
| fs.readFileSync(tmpFile, "utf-8"), | ||
| ) as BuildxMetadata; | ||
|
|
||
| const imageName = metadata["image.name"]?.split(",")[0]; |
There was a problem hiding this comment.
I'd stick with what is in the documentation, which is "containerimage.digest"
https://docs.docker.com/reference/cli/docker/buildx/build/#metadata-file
There was a problem hiding this comment.
I will remove image.name and leave the existing digest below. But the correct one is containerimage.config.digest here is the first build that was only using the one mentioned https://github.com/cartesi/cli/actions/runs/28516129205/job/84528221902#step:9:697. Looks like in the OCI image spec they're used for different purposes, and the containerimage.config.digest is the one we see when running the command docker images under the ID column.
There was a problem hiding this comment.
Hmm, that's weird. Will double check this later.
There was a problem hiding this comment.
containerimage.digest is the right one
config namespace if something else
There was a problem hiding this comment.
@endersonmaia I linked the build above. Using that containerimage.digest causes problems as the Docker daemon reports that it is not able to find the image requested with the hash passed.
For context, when using locally and running the tests, it runs fine, and upon inspecting the metadata, both containerimage.digest and containerimage.config.digest have the same SHA256, but once pushed and the CI runs with QEMU, the story is different, and both options have different hashes, and the option discussed is the one that cause the error in the build link I shared above.
There was a problem hiding this comment.
What if we stop supporting these info coming from docker (CMD, ENTRYPOINT, ENV, WORKDIR), and use only what is defined in the cartesi.toml. Then maybe we don't even need the image id and the docker output.
There was a problem hiding this comment.
@tuler, I would like to take a closer look at that specific part later. Perhaps in a separate PR/branch.
There was a problem hiding this comment.
But still I did not understand yet in what circumstances using containerimage.digest is correct.
There was a problem hiding this comment.
But still I did not understand yet in what circumstances using containerimage.digest is correct.
When building with the embedded buildkit via the local Docker daemon, you should use containerimage.digest, as the Docker daemon only tracks single-platform images, and buildkit generates a "flat" local image format, so the build and manifest are treated as one and the same.
Just to reiterate on the other side, when building with the docker-container, buildkit operates independently of the Docker daemon's local "constraints and, as I mentioned before, outputs a standard OCI multi-platform images structure. So in that case, we use the containerimage.config.digest as Buildkit maps the concrete image ID of the specific platform to this field.
There was a problem hiding this comment.
So, based on some tests, here is my conclusion:
-
we should use
--iidfileinstead of--metadata-file. It's much simpler, we don't need to commit to any json structure, just read the image id from the produced text file. (this by itself don't solve the inconsistency problem yet, see point 2 below). -
I think we should have more control over the buildx instance to use, to make the build process more consistent across platforms and docker installations. We can create a builder named
cartesi, conditionally if it doesn't exist yet), and use that builder explicitly. This will give more control over the builder platform and the builder driver. It has been tested on macOS with Docker Desktop and on Linux with Docker Engine, with consistent results.
A builder can be created with the command below.
docker buildx create --driver docker-container --name cartesi --platform linux/riscv64
If it already exists and no --append is specified it exits with an error. Or it's also possible to run docker buildx inspect --name cartesi.
When building a builder can be used with docker buildx build --builder cartesi
brunomenezes
force-pushed
the
feat/print-build-logs-on-verbose-opt
branch
5 times, most recently
from
685f739 to
389efa2
Compare
* Improve the --verbose flag to stream logs from the build process. * Mainly affects the docker usage that was silent by default.
brunomenezes
force-pushed
the
feat/print-build-logs-on-verbose-opt
branch
from
389efa2 to
6c4c342
Compare
3 participants
Summary
Currently, the
cartesi buildcommand, when using Docker under the hood, is in silent mode even when the--verboseflag is passed. If the build takes a while to happen, the user has no idea what is going on unless they realize to go intodocker desktop -> builds -> click tab active build -> tab logsand have a look at the current situation. However, in a CI environment, that is not an option.Now, by passing the
--verboseflag, you will be able to see the logs of the Docker build.PS: We're dogfooding by enabling it on our own integration tests that require an application to be built before the cases are run against it.