docker / build-push-action

GitHub Action to build and push Docker images with Buildx
https://github.com/marketplace/actions/build-and-push-docker-images
Apache License 2.0
4.11k stars 527 forks source link

Allow grouping output for the `docker buildx build` step after `Buildx version` step #1036

Closed nitrocode closed 5 months ago

nitrocode commented 6 months ago

Description

It would be nice to group the main build output like the action does in the other groups that precede the docker buildx build step

https://github.com/docker/build-push-action/blob/9f6f8c940b91232557f8699b21341a08624a8dce/src/main.ts#L71-L83

Maybe

# wrap this await
await core.group(`docker build`, async () => {
  # around existing lines 77 to 83
  await Exec.getExecOutput(buildCmd.command, buildCmd.args, { 
    ignoreReturnCode: true 
  }).then(res => { 
    if (res.stderr.length > 0 && res.exitCode != 0) { 
      throw new Error(`buildx failed with: ${res.stderr.match(/(.*)\s*$/)?.[0]?.trim() ?? 'unknown error'}`); 
    } 
  }); 
});

To keep existing functionality, it could be behind a flag such as group_build_output: true which can default to false.

crazy-max commented 5 months ago

Is there a specific use case for this? I recall we tried that before and it cluttered the build progress output and also was meaningless as the intent is to always show the build output.

I'm closing this one but feel free to continue the discussion.

nitrocode commented 5 months ago

I was trying to find other log output and it was an obstacle to scroll above the docker buildx build.

I figured grouping it would be easy as it's already done in other places. I wonder how the previous implementation cluttered the build progress.

If it was behind a non-default flag, would it be more palatable to add?