-
Notifications
You must be signed in to change notification settings - Fork 32
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
xx-info: handle rhel arch #52
Conversation
args = { | ||
TEST_BASE_TYPE = "rhel" | ||
TEST_BASE_IMAGE = "fedora:35" | ||
TEST_CMDS = "info" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Remove this line in a follow-up when windres is hanlded for rhel:
#24 [test 6/11] RUN --mount=type=cache,target=/pkg-cache [ "${TEST_CMDS:-windres}" = "${TEST_CMDS#*windres}" ] && exit 0; ./test-windres.bats
#24 0.173 1..3
#24 0.254 not ok 1 invalid
#24 0.254 # (from function `xxrun' in file test_helper.bash, line 54,
#24 0.254 # from function `add' in file test_helper.bash, line 7,
#24 0.254 # in test file test-windres.bats, line 7)
#24 0.255 # `add llvm' failed with status 127
#24 0.255 # /work/test_helper.bash: line 54: apt: command not found
#24 0.319 not ok 2 basic
#24 0.319 # (from function `assert_success' in file bats-assert/src/assert.bash, line 114,
#24 0.319 # in test file test-windres.bats, line 20)
#24 0.319 # `assert_success' failed
#24 0.319 #
#24 0.319 # -- command failed --
#24 0.319 # status : 1
#24 0.319 # output : llvm-rc not installed
#24 0.319 # --
#24 0.320 #
#24 0.359 ok 3 clean
#24 ERROR: process "/bin/sh -c [ \"${TEST_CMDS:-windres}\" = \"${TEST_CMDS#*windres}\" ] && exit 0; ./test-windres.bats" did not complete successfully: exit code: 1
------
> [test 6/11] RUN --mount=type=cache,target=/pkg-cache [ "${TEST_CMDS:-windres}" = "${TEST_CMDS#*windres}" ] && exit 0; ./test-windres.bats:
#24 0.319 # (from function `assert_success' in file bats-assert/src/assert.bash, line 114,
#24 0.319 # in test file test-windres.bats, line 20)
#24 0.319 # `assert_success' failed
#24 0.319 #
#24 0.319 # -- command failed --
#24 0.319 # status : 1
#24 0.319 # output : llvm-rc not installed
#24 0.319 # --
#24 0.320 #
#24 0.359 ok 3 clean
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I added a check for this in https://github.com/tonistiigi/xx/pull/49/files#diff-97b6828de99e6a75768dba426569ce1a89f0e32542b0ea9c7a3349f47a99263bR16 . Does centos really ship with very old llvm or why isn't this installed?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Does centos really ship with very old llvm or why isn't this installed?
I have not yet updated the test_helper.bash to include yum/dnf. I can add the add/del implementation so the test would pass but we might also need to add xx-yum
or xx-dnf
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Also centos and rhel don't have the bats
pkg available in their main repo so I might need to fix sources for the test-base-rhel
target in the Dockerfile.
e83c048
to
77d35e2
Compare
Signed-off-by: CrazyMax <crazy-max@users.noreply.github.com>
Handle RHEL based distrib for
xx-info
. This is partially needed for docker/buildx#840 to handle RPM arch package properly. Following distribs are currently handled:I wonder if we should not take into account
ID_LIKE
to properly handle RHEL based distrib. Same could be done for Debian based in #49.In a follow-up we could start thinking about
xx-yum
.Signed-off-by: CrazyMax crazy-max@users.noreply.github.com