Skip to content

Test statically linked clang outside of Alpine Linux#13

Merged
nathanchance merged 10 commits into
ClangBuiltLinux:mainfrom
nathanchance:test-clang-pipeline
May 27, 2022
Merged

Test statically linked clang outside of Alpine Linux#13
nathanchance merged 10 commits into
ClangBuiltLinux:mainfrom
nathanchance:test-clang-pipeline

Conversation

@nathanchance

Copy link
Copy Markdown
Member

We should verify that we can use the toolchain produced in Alpine Linux
on other distributions as part of the continuous integration pipeline.

We download the toolchain out of the container, mount it into the
containers that we want to test it in, and test dynamically and
statically linking hello.{c,cpp} with musl. Eventually, glibc should be
tested but there are issues during the link phase.

Initially, the only container that is being tested is Fedora. However, I
intentionally wrote this in such a way that we should be able to use
this across multiple distributions and test more than just
hello.{c,cpp}.

This was tested on a self-hosted GitHub Actions runner:

https://github.com/nathanchance/containers/actions/runs/2387476728

We should verify that we can use the toolchain produced in Alpine Linux
on other distributions as part of the continuous integration pipeline.

We download the toolchain out of the container, mount it into the
containers that we want to test it in, and test dynamically and
statically linking hello.{c,cpp} with musl. Eventually, glibc should be
tested but there are issues during the link phase.

Initially, the only container that is being tested is Fedora. However, I
intentionally wrote this in such a way that we should be able to use
this across multiple distributions and test more than just
hello.{c,cpp}.

This was tested on a self-hosted GitHub Actions runner:

https://github.com/nathanchance/containers/actions/runs/2387476728

Signed-off-by: Nathan Chancellor <nathan@kernel.org>

@nickdesaulniers nickdesaulniers left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

wow thanks! Looks good. doesn't matter to me if my feedback is handled in this PR or follow ups.

Feel free to re-kick the build once this is merged, too!

Comment thread ci/test-clang.sh Outdated
Comment thread ci/test-clang.sh
Comment thread ci/test-clang.sh
Comment thread ci/test-clang.sh Outdated
Comment thread ci/test-clang-docker.sh
Comment thread ci/test-clang-docker.sh Outdated
Comment thread ci/test-clang-docker.sh
Comment thread ci/test-clang-docker.sh Outdated
Comment thread ci/test-clang-docker.sh Outdated
This makes it clearer what '"$rootdir":/repo:ro' is doing.

Signed-off-by: Nathan Chancellor <nathan@kernel.org>
I find this a little easier to parse when multiple arguments are
involved.

Signed-off-by: Nathan Chancellor <nathan@kernel.org>
Signed-off-by: Nathan Chancellor <nathan@kernel.org>
Per the 'docker cp' documentation, if SRC_PATH specifies a directory
that does not end with '/.' and DEST_PATH is a directory that exists,
SRC_PATH is copied into DEST_PATH.

'docker cp' does NOT create parent directories so we still need the
intial 'mkdir' call but we do not need the '-p' flag as '$toolchain' is
a subfolder within the repo, which has to exist if we are running this
command.

Link: https://docs.docker.com/engine/reference/commandline/cp/
Signed-off-by: Nathan Chancellor <nathan@kernel.org>
Signed-off-by: Nathan Chancellor <nathan@kernel.org>
Signed-off-by: Nathan Chancellor <nathan@kernel.org>
@nathanchance

Copy link
Copy Markdown
Member Author

Alright, I should have addressed all outstanding comments. Please take a look.

Currently, we are not actually testing the image that is being built
during CI because there is no '--output' flag to 'docker buildx':

  WARNING: No output specified for docker-container driver. Build result
  will only remain in the build cache. To push result image into registry
  use --push or to load image into docker use --load

The docker/build-push-action supports "load" and "push" as shorthands
for different '--output' values but they are mutually exclusive, meaning
that if we want to test the image before we push it, we need to use
"load: true" to make the image available locally, test it, then run a
simple 'docker push' once it passes.

Signed-off-by: Nathan Chancellor <nathan@kernel.org>
@nathanchance

Copy link
Copy Markdown
Member Author

a6a0b02 addresses an issue I noticed while bringing up arm64 support on a self-hosted runner. It looks like it is not possible to use the Docker action just to push and they appear to have no intention of supporting that but this is an easy enough workaround.

Comment thread ci/test-clang-docker.sh
Comment thread ci/test-clang.sh Outdated
Comment thread ci/test-clang-docker.sh Outdated
Signed-off-by: Nathan Chancellor <nathan@kernel.org>
Comment thread ci/test-clang-docker.sh Outdated
Comment on lines +24 to +26
-B /usr/lib/musl/lib # Scrt1.o, crti.o, crtn.o
-I /usr/lib/musl/include # stdio.h
-L /usr/lib/musl/lib # -lc

@nickdesaulniers nickdesaulniers May 27, 2022

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

What about --sysroot? Can --sysroot=/usr/lib/musl subsume all of -B, -I, -L?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

I'll test.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I'll file a todo to see what we can do to simplify this. Basically, if we want to support kernel builds for make LLVM=1 and not just make CC=clang with this (i.e. HOSTCC=clang) we might have to add these somehow.

Though perhaps I'm getting ahead of myself, and they can simply just use --target=x86_64-linux-gnu and it will work just fine.

Perhaps we need to find the most portable way of finding the expected target triple for a given system. Is it glibc based or musl based? Is $(CC) -print-target-triple the most portable way to find out? Is there a way to find out without a compiler installed? IDK

example
$ clang --target=$($(CC) -print-target-triple

so that our default implicit target doesn't matter.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Perhaps we need to find the most portable way of finding the expected target triple for a given system.

Right, I think this is the root of the problem: for HOSTCC=clang, we don't set a triple, we rely on the implicit default, which is not going to work if we want to support both glibc and musl with this toolchain.

Is it glibc based or musl based?

I suppose one option would be trying to look at a binary for GLIBC symbols but that feels like a massive hack...

We could potentially make this work with a wrapper like musl ships? On my host:

$ cat $(which musl-clang)
#!/bin/sh
cc="clang"
libc="/usr/lib/musl"
libc_inc="/usr/lib/musl/include"
libc_lib="/usr/lib/musl/lib"
thisdir="`cd "$(dirname "$0")"; pwd`"

# prevent clang from running the linker (and erroring) on no input.
sflags=
eflags=
for x ; do
    case "$x" in
        -l*) input=1 ;;
        *) input= ;;
    esac
    if test "$input" ; then
        sflags="-l-user-start"
        eflags="-l-user-end"
        break
    fi
done

exec $cc \
    -B"$thisdir" \
    -fuse-ld=musl-clang \
    -static-libgcc \
    -nostdinc \
    --sysroot "$libc" \
    -isystem "$libc_inc" \
    -L-user-start \
    $sflags \
    "$@" \
    $eflags \
    -L"$libc_lib" \
    -L-user-end

I suspect that might make the regular kernel build unhappy but maybe not?

Signed-off-by: Nathan Chancellor <nathan@kernel.org>

@nickdesaulniers nickdesaulniers left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

LGTM 🌳 🌲

@nathanchance nathanchance merged commit c37dc71 into ClangBuiltLinux:main May 27, 2022
@nathanchance nathanchance deleted the test-clang-pipeline branch May 27, 2022 19:29
@nickdesaulniers

Copy link
Copy Markdown
Member

@nathanchance do you want to manually kick the workflow to trigger a run of this?

@nickdesaulniers

Copy link
Copy Markdown
Member

@nathanchance

Copy link
Copy Markdown
Member Author

I'll keep an eye on it to make sure everything works!

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