Skip to content

Commit

Permalink
Don't set cross-compilation variables in Makefile
Browse files Browse the repository at this point in the history
This feature (cc_platform.mk and its sourcing from Makefiles) was added
by commit dac4171 ("runc-dmz: reduce memfd binary cloning cost
with small C binary") as part of dmz work.

From what I see, this is not needed for `make releaseall` as we already
set these variables using `set_cross_vars` shell function. My best guess
is this was mostly needed for testing (and for cross-i386 CI job).

The big issue though is these variables are guessed based on our
cross-compilation environment, meaning they won't work in other cases
(such as native compilation on non-x86_64 platforms, or using other
cross-compilation tools). One such example is Moby build which uses
https://github.com/tonistiigi/xx.

Besides, it's a duplication of effort in scripts/lib.sh, which is prone
to error.

Let's remove this from the Makefiles, and rework the script which sets
those variables so it can be used for other scenarios as well. For
example, to test cross-compilation locally (assuming we're in the same
environment as created by `make releaseall`), use

	$(./script/cross.sh $ARCH); make $TARGET

Signed-off-by: Kir Kolyshkin <[email protected]>
  • Loading branch information
kolyshkin committed Jul 13, 2024
1 parent 3778ae6 commit 12a0905
Show file tree
Hide file tree
Showing 8 changed files with 15 additions and 81 deletions.
2 changes: 1 addition & 1 deletion .github/workflows/test.yml
Original file line number Diff line number Diff line change
Expand Up @@ -232,4 +232,4 @@ jobs:
- name: unit test
env:
EXTRA_BUILDTAGS: ${{ matrix.dmz }}
run: sudo -E PATH="$PATH" -- make GOARCH=386 localunittest
run: sudo -E PATH="$PATH" sh -c "$(./script/cross.sh 386); make localunittest"
2 changes: 1 addition & 1 deletion Dockerfile
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,7 @@ RUN cd /tmp \

# install libseccomp
ARG LIBSECCOMP_VERSION
COPY script/seccomp.sh script/lib.sh /tmp/script/
COPY script/seccomp.sh script/cross.sh /tmp/script/
RUN mkdir -p /opt/libseccomp \
&& /tmp/script/seccomp.sh "$LIBSECCOMP_VERSION" /opt/libseccomp 386 amd64 arm64 armel armhf ppc64le riscv64 s390x
ENV LIBSECCOMP_VERSION=$LIBSECCOMP_VERSION
Expand Down
3 changes: 0 additions & 3 deletions Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -3,9 +3,6 @@ SHELL = /bin/bash
CONTAINER_ENGINE := docker
GO ?= go

# Get CC values for cross-compilation.
include cc_platform.mk

PREFIX ?= /usr/local
BINDIR := $(PREFIX)/sbin
MANDIR := $(PREFIX)/share/man
Expand Down
61 changes: 0 additions & 61 deletions cc_platform.mk

This file was deleted.

3 changes: 1 addition & 2 deletions libcontainer/dmz/Makefile
Original file line number Diff line number Diff line change
@@ -1,5 +1,4 @@
# Get GO, GOARCH and CC values for cross-compilation.
include ../../cc_platform.mk
STRIP ?= strip

# List of GOARCH that nolibc supports, from:
# https://go.dev/doc/install/source#environment (with GOOS=linux)
Expand Down
11 changes: 6 additions & 5 deletions script/lib.sh → script/cross.sh
100644 → 100755
Original file line number Diff line number Diff line change
@@ -1,10 +1,8 @@
#!/bin/bash

# NOTE: Make sure you keep this file in sync with cc_platform.mk.

# set_cross_vars sets a few environment variables used for cross-compiling,
# Prints a few environment variables used for cross-compiling,
# based on the architecture specified in $1.
function set_cross_vars() {
function print_cross_vars() {
GOARCH="$1" # default, may be overridden below
unset GOARM

Expand Down Expand Up @@ -79,5 +77,8 @@ function set_cross_vars() {
CC="${HOST:+$HOST-}gcc"
STRIP="${HOST:+$HOST-}strip"

export HOST CFLAGS GOARM GOARCH CC STRIP
[ -v GOARM ] && echo "export GOARM=$GOARM"
echo "export HOST=$HOST CFLAGS=\"$CFLAGS\" GOARCH=$GOARCH CC=$CC STRIP=$STRIP"
}

print_cross_vars "$1"
7 changes: 3 additions & 4 deletions script/release_build.sh
Original file line number Diff line number Diff line change
Expand Up @@ -22,9 +22,7 @@ set -e
: "${LIBSECCOMP_VERSION:=2.5.5}"
project="runc"
root="$(readlink -f "$(dirname "${BASH_SOURCE[0]}")/..")"

# shellcheck source=./script/lib.sh
source "$root/script/lib.sh"
cross="$root"/script/cross.sh

# This function takes an output path as an argument, where the built
# (preferably static) binary should be placed.
Expand Down Expand Up @@ -68,7 +66,8 @@ function build_project() {
for arch in "${arches[@]}"; do
# Reset CFLAGS.
CFLAGS="$original_cflags"
set_cross_vars "$arch"
# Obtain cross-compilation environment variables.
eval "$($cross "$arch")"
make -C "$root" \
PKG_CONFIG_PATH="$seccompdir/$arch/lib/pkgconfig" \
"${make_args[@]}"
Expand Down
7 changes: 3 additions & 4 deletions script/seccomp.sh
Original file line number Diff line number Diff line change
@@ -1,9 +1,7 @@
#!/bin/bash

set -e -u -o pipefail

# shellcheck source=./script/lib.sh
source "$(dirname "${BASH_SOURCE[0]}")/lib.sh"
cross="$(dirname "${BASH_SOURCE[0]}")/cross.sh"

# sha256 checksums for seccomp release tarballs.
declare -A SECCOMP_SHA256=(
Expand Down Expand Up @@ -48,7 +46,8 @@ function build_libseccomp() {
for arch in "${arches[@]}"; do
# Reset CFLAGS.
CFLAGS="$original_cflags"
set_cross_vars "$arch"
# Obtain cross-compilation environment variables.
eval "$($cross "$arch")"
./configure --host "$HOST" \
--prefix="$dest/$arch" --libdir="$dest/$arch/lib" \
--enable-static --enable-shared
Expand Down

0 comments on commit 12a0905

Please sign in to comment.