-
Notifications
You must be signed in to change notification settings - Fork 176
[cross-compile] Make pkg/apparmor cross compilable #5084
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
base: master
Are you sure you want to change the base?
Conversation
fix ENV/VAR syntax Signed-off-by: Mikhail Malyshev <[email protected]>
d68971e to
b3559b7
Compare
Make pkg/apparmor cross compilable
- the main trick is how sysroot is created by importing --platform=${TARGETPALTFORM} alpine
- our cross compiler expects sysroot to be at fixed locations and doesn't accept --sysroot
Signed-off-by: Mikhail Malyshev <[email protected]>
b3559b7 to
5d06ac9
Compare
OhmSpectator
left a comment
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.
Running the tests. Though, I don't understand the change yet...
| WORKDIR /apparmor/parser | ||
| RUN ../common/list_af_names.sh > base_af_names.h && \ | ||
| make | ||
| make -j"$(nproc)" |
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.
a wise man once said: #4775 (comment) ;-)
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.
@christoph-zededa yeah.. I was lazy to look for the proper variable name
|
Isn't riscv64 missing?
I don't think it is a good idea to copy all this lines to other packages. Isn't it possible to make it easier for other packages to adopt it? |
It is, but apparmor is not a part of riscv build and we do not have a crosscompiler. Would be nice to move to For the second question. It is possible to some extent like I did in |
| FROM --platform=${BUILDPLATFORM} lfedge/eve-cross-compilers:fb809cfb1909752acb563e0b77cd3799534bce64 AS cross-compilers | ||
|
|
||
| FROM builder-native-base as builder-cross-base | ||
| COPY --from=cross-compilers /packages /packages |
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.
Can you not just move FROM .... AS cross-compilers to before line 6 (previous FROM), and then COPY these into builder-native-base? Or is this a different platform?
This needs comments to make it clear what each one is doing, as well as how the whole flow works. I mostly get what you are doing, but miss some of the steps.
| FROM lfedge/eve-alpine:0f2e0da38e30753c68410727a6cc269e57ff74f2 as build | ||
| ENV BUILD_PKGS linux-headers musl-dev musl-utils musl-libintl git gcc g++ \ | ||
| autoconf automake libtool make flex bison bash sed gettext | ||
| FROM --platform=${BUILDPLATFORM} lfedge/eve-alpine:0f2e0da38e30753c68410727a6cc269e57ff74f2 as builder-native-base |
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.
FROM ... AS ... (as uppercase as well)...
| COPY --from=cross-compilers /packages /packages | ||
|
|
||
| FROM builder-cross-base as builder-target-arm64 | ||
| ARG COMPILER_TARGET_ARCH=aarch64 |
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.
In the documentation (https://github.com/lf-edge/eve/blob/master/docs/BUILD.md#cross-compilation-support), we recommend to call this variable EVE_TARGET_ARCH, as used in other packages... not critical, but would be good to keep the pattern...
We do have cross-compiler for riscv64 from hosts x86_64 and arm64. What we don't have is cross-compiler for x86_64 and arm64 from riscv64 hosts. If this image can be built for riscv64 without issues, let's add the cross-compilation for it as well. If it has issues (like pillar), then is not worth of change.... |
| FROM builder-target as builder-amd64-arm64 | ||
| ENV CONFIGURE_TARGETS="--build=aarch64-alpine-linux-musl --host=x86_64-alpine-linux-musl" | ||
| # copy libraries from target-sysroot | ||
| COPY --from=target-sysroot /usr/lib/ /usr/x86_64-alpine-linux-musl/lib/ |
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.
instead of copying the whole /usr/lib and /usr/include you can use an approach like this to create the target libraries:
https://github.com/lf-edge/eve/blob/master/pkg/pillar/Dockerfile#L40
so you install only the libraries you need and they will be available at /out, you can even use a single code block to automate the process for the target architecture: https://github.com/lf-edge/eve/blob/master/pkg/pillar/Dockerfile#L53
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.
@rene sure, I actually did not mean to merge it to be honest, but I can fix it 👍
Description
Just for fun converted one of the packages into cross compilable. You can follow this pattern and do it for other packages as well. it may be tricky to run ./configure or figure out proper CFLAGS or install proper libraries into sysroot but it is possible
FROM --platform=${TARGETPLATFORM} lfedge/eve-alpine:0f2e0da38e30753c68410727a6cc269e57ff74f2 as target-sysrootand then we can install all required target libraries like
--sysroot=/usr/x86_64-alpine-linux-muslfor aarch64 so we just copy libraries from our sysrot to this locationHow to test and validate this PR
docker buildx build --platform=linux/arm64 -t cross-test-1 --load . docker run -it cross-test-1 /bin/bashrun
file /usr/bin/apparmor_parserto make sure it is of aarch64 architecture.repeat these steps for all other architectures
PR Backports
Checklist
And the last but not least:
check them.
Please, check the boxes above after submitting the PR in interactive mode.