Skip to content

fix(bazel): don't give absolute path for images#347

Merged
loosebazooka merged 2 commits into
GoogleContainerTools:mainfrom
aspect-forks:main
May 4, 2023
Merged

fix(bazel): don't give absolute path for images#347
loosebazooka merged 2 commits into
GoogleContainerTools:mainfrom
aspect-forks:main

Conversation

@alexeagle
Copy link
Copy Markdown
Collaborator

This causes us to hit #183 in most cases. See https://buildkite.com/bazel/bcr-presubmit/builds/1247#0187e4cc-44e1-4e21-89a7-08e71f612bfc as an example.

We'll need to fix this in order to properly support Windows, but that is already broken for other reasons.

This causes us to hit GoogleContainerTools#183 in most cases.
See https://buildkite.com/bazel/bcr-presubmit/builds/1247#0187e4cc-44e1-4e21-89a7-08e71f612bfc as an example.

We'll need to fix this in order to properly support Windows, but that is already broken for other reasons.
@alexeagle alexeagle marked this pull request as ready for review May 4, 2023 15:43
if [[ -d "$image" ]]; then
readonly DIGEST=$("$yq" eval '.manifests[0].digest | sub(":"; "-")' "${{image}}/index.json")
if [[ -d "{image_path}" ]]; then
readonly DIGEST=$("$yq" eval '.manifests[0].digest | sub(":"; "-")' "{image_path}/index.json")
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Yeah oops! Thanks

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

unrelated question; the newest version of yq requires -oj to suppress the default output warning and -r to restore the output raw which was the default in the previous version. can we also add those to yq call here to allow future versions of yq to be used?

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

let's add in a separate PR. We need some test coverage for this branch as well.

@loosebazooka loosebazooka merged commit 104a53e into GoogleContainerTools:main May 4, 2023
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