Skip to content
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

Pass arch value to --platform #153

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

LecrisUT
Copy link

@LecrisUT LecrisUT commented Oct 9, 2024

This comes from a debugging session in #152. It seems to resolve issues like:

  Dockerfile.s390x.ubuntu18.04:1
  --------------------
     1 | >>> FROM s390x/fedora:latest
     2 |     COPY ./run-on-arch-install.sh /root/run-on-arch-install.sh
     3 |     RUN chmod +x /root/run-on-arch-install.sh && /root/run-on-arch-install.sh
  --------------------
  ERROR: failed to solve: s390x/fedora:latest: failed to resolve source metadata for docker.io/s390x/fedora:latest: no match for platform in manifest: not found

LecrisUT referenced this pull request in LecrisUT/run-on-arch-action Oct 9, 2024
src/run-on-arch.js Outdated Show resolved Hide resolved
src/run-on-arch.sh Outdated Show resolved Hide resolved

# Defaults
ACTION_DIR="$(cd "$(dirname "$0")"/.. >/dev/null 2>&1 ; pwd -P)"
LOWERCASE_REPOSITORY=$(printf "%s" "$GITHUB_REPOSITORY" | tr '[:upper:]' '[:lower:]')
PACKAGE_REGISTRY="ghcr.io/${LOWERCASE_REPOSITORY}/${CONTAINER_NAME}"
DEBIAN_FRONTEND=noninteractive

# Sanitize inputs
if [[ "$ARCH" == "none" ]]; then
unset ARCH
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
unset ARCH
export ARCH=amd64

Copy link
Author

@LecrisUT LecrisUT Oct 9, 2024

Choose a reason for hiding this comment

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

I was thinking if we should allow an interface to not pass --platform, that was the reason for unset approach

@@ -49,6 +55,7 @@ build_container () {
then
docker build \
"${ACTION_DIR}/Dockerfiles" \
${ARCH:+--platform=linux/$ARCH} \
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
${ARCH:+--platform=linux/$ARCH} \
--platform=linux/$ARCH \

@@ -70,6 +77,7 @@ build_container () {
docker pull "$PACKAGE_REGISTRY:latest" || true
docker build \
"${ACTION_DIR}/Dockerfiles" \
${ARCH:+--platform=linux/$ARCH} \
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
${ARCH:+--platform=linux/$ARCH} \
--platform=linux/$ARCH \

src/run-on-arch.js Outdated Show resolved Hide resolved
@LecrisUT
Copy link
Author

LecrisUT commented Oct 9, 2024

One other thing I should ask. I was able to debug locally with podman and I got much further than with this action. I suspect it's because this needs to be passed to docker run as well?

Edit: I've tried adding it to docker run as well, but that didn't help, still the same error: maciejhirsz/logos#428. Running locally with podman works just fine 🤷

@@ -49,6 +55,7 @@ build_container () {
then
docker build \
Copy link
Contributor

Choose a reason for hiding this comment

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

Probably it will be a good idea to start using buildx instead

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.

2 participants