dockerfiles: adding support for linux-ppc64le #6792
Conversation
adilhusain-s
requested review from
niedbalski,
patrick-stephens,
fujimotos,
edsiper,
leonardo-albertovich and
koleini
as code owners
adilhusain-s
force-pushed
the
ppc64le-ci
branch
from
5039abe to
b065f61
Compare
src/flb_utils.c
Outdated
| @@ -706,7 +706,10 @@ int flb_utils_write_str(char *buf, int *off, size_t size, | |||
| encoded_to_buf(p, tmp, len); | |||
| p += len; | |||
| } | |||
| else if (c >= 0x80 && c <= 0xFFFF) { | |||
| #if defined(__powerpc64__) | |||
There was a problem hiding this comment.
This is the only part of this PR I want to understand better before approving it, I think I asked about it in the previous version but there was no response, could you explain why is this change necessary?
There was a problem hiding this comment.
Hi @leonardo-albertovich
I remember you asked for this before. When I tried last time I could not get to the root of it in the limited time that was available. I made this change after the test failed, based on the actual value of c being received for Power. I will look at it again.
There was a problem hiding this comment.
Thank you, getting to the root of this is the only think missing from my point of view, if you need any help with it let me know, I'd be glad to assist you as much as possible.
There was a problem hiding this comment.
Hi @leonardo-albertovich
The issue is plain characters are signed on x86_64, and unsigned on ppc64le. Forcing chars on ppc64le to be signed using the gcc option -fsigned-char. So no change required to the original flb_utils.c. Will update the PR shortly.
There was a problem hiding this comment.
Awesome, with that out of the way I think it's good to go as long as it runs on CI and passes the tests of course.
Edit: Thank you very much for looking into it.
adilhusain-s
force-pushed
the
ppc64le-ci
branch
from
b065f61 to
3ca942e
Compare
|
Just need to make sure the commits are signed and use the right prefix: https://github.com/fluent/fluent-bit/blob/master/CONTRIBUTING.md#commit-changes We should also ensure it builds across all targets with the |
Signed-off-by: Sumit Dubey <[email protected]>
adilhusain-s
force-pushed
the
ppc64le-ci
branch
from
3ca942e to
9ef9cc2
Compare
patrick-stephens
temporarily deployed
to
unstable
GitHub Actions
Inactive
patrick-stephens
temporarily deployed
to
pr-package-test
GitHub Actions
Inactive
patrick-stephens
temporarily deployed
to
pr-package-test
GitHub Actions
Inactive
|
This PR is stale because it has been open 45 days with no activity. Remove stale label or comment or this will be closed in 10 days. |
|
This PR is stale because it has been open 45 days with no activity. Remove stale label or comment or this will be closed in 10 days. |
|
I think there are still some outstanding review comments to address from @leonardo-albertovich |
Signed-off-by: Pat <[email protected]>
|
let's review this @leonardo-albertovich |
Successfully tested the change on my fork.
workflow run link