From d0cafb91b0e4ef47d24cfa6d2736097394a1275e Mon Sep 17 00:00:00 2001 From: Andrew Brown Date: Mon, 18 Mar 2024 13:50:58 -0700 Subject: [PATCH 1/4] Replace multiple version scripts with a Python script In the past, I've had trouble keeping the current set of version scripts to correctly output the version, especially in light of how we append Git suffixes for a non-tagged commit and dirty state. This change replaces those scripts with a single Python one which, though much more wordy than the previous one, may be easier for contributors to read and modify. The original scripts relied on Perl; this one relies on Python, which seems like a fair exchange. Having a single script also makes it easier to solve #372. --- Makefile | 5 +- RELEASING.md | 2 +- deb_from_installation.sh | 2 +- llvm_version_major.sh | 4 -- tar_from_installation.sh | 2 +- version.py | 122 +++++++++++++++++++++++++++++++++++++++ version.sh | 6 -- 7 files changed, 128 insertions(+), 15 deletions(-) delete mode 100755 llvm_version_major.sh create mode 100755 version.py delete mode 100755 version.sh diff --git a/Makefile b/Makefile index b30a62a3b..cf3ccb17c 100644 --- a/Makefile +++ b/Makefile @@ -3,6 +3,7 @@ ROOT_DIR=${CURDIR} LLVM_PROJ_DIR?=$(ROOT_DIR)/src/llvm-project +VERSION_SCRIPT=python3 ./version.py # Windows needs munging ifeq ($(OS),Windows_NT) @@ -45,8 +46,8 @@ endif TARGETS = wasm32-wasi wasm32-wasip1 wasm32-wasip2 wasm32-wasip1-threads wasm32-wasi-threads # Only the major version is needed for Clang, see https://reviews.llvm.org/D125860. -CLANG_VERSION=$(shell $(BASH) ./llvm_version_major.sh $(LLVM_PROJ_DIR)) -VERSION:=$(shell $(BASH) ./version.sh) +CLANG_VERSION=$(shell $(VERSION_SCRIPT) llvm-major --llvm-dir=$(LLVM_PROJ_DIR)) +VERSION:=$(shell $(VERSION_SCRIPT)) DEBUG_PREFIX_MAP=-fdebug-prefix-map=$(ROOT_DIR)=wasisdk://v$(VERSION) default: build diff --git a/RELEASING.md b/RELEASING.md index d7eaef2f7..c684149be 100644 --- a/RELEASING.md +++ b/RELEASING.md @@ -3,7 +3,7 @@ To publish a new version of `wasi-sdk` as a GitHub release: 1. Tag a commit with an annotated tag. Note that this must be an annotated tag, - not a lightweight tag, so that `version.sh` can use it for calculating the + not a lightweight tag, so that `version.py` can use it for calculating the package version (use `git show wasi-sdk-...` to show other tag messages). Note that you may need to clear the repository cache to avoid problems with cached artifacts [^cache]. diff --git a/deb_from_installation.sh b/deb_from_installation.sh index 2c1596163..2699c819c 100755 --- a/deb_from_installation.sh +++ b/deb_from_installation.sh @@ -18,7 +18,7 @@ fi if [ -n "$2" ]; then VERSION="$2" else - VERSION=`./version.sh` + VERSION=`./version.py` fi if [ -n "$3" ]; then diff --git a/llvm_version_major.sh b/llvm_version_major.sh deleted file mode 100755 index 93fe9b01b..000000000 --- a/llvm_version_major.sh +++ /dev/null @@ -1,4 +0,0 @@ -#/bin/bash -LLVM_PROJ_DIR=${1:-./src/llvm-project} -MAJOR=`(grep "set(LLVM_VERSION_MAJOR" $LLVM_PROJ_DIR/llvm/CMakeLists.txt || grep "set(LLVM_VERSION_MAJOR" $LLVM_PROJ_DIR/cmake/Modules/LLVMVersion.cmake) | awk '{print substr($2, 1, length($2) - 1)}'` -echo $MAJOR diff --git a/tar_from_installation.sh b/tar_from_installation.sh index f90000bf5..7d0943266 100755 --- a/tar_from_installation.sh +++ b/tar_from_installation.sh @@ -10,7 +10,7 @@ fi if [ -n "$2" ]; then VERSION="$2" else - VERSION=`./version.sh` + VERSION=`./version.py` fi if [ -n "$3" ]; then diff --git a/version.py b/version.py new file mode 100755 index 000000000..52dc07ab1 --- /dev/null +++ b/version.py @@ -0,0 +1,122 @@ +#!/usr/bin/env python3 + +# This script finds and prints the various versions in this project: wasi-sdk +# itself, LLVM, and the Git revisions of dependencies. +# +# Usage: version [wasi-sdk|llvm|llvm-major|dump] [--llvm-dir=] + +import sys +import argparse +import subprocess + +# The number of characters to use for the abbreviated Git revision. +GIT_REF_LEN = 12 + + +def exec(command, cwd=None): + result = subprocess.run(command, stdout=subprocess.PIPE, + universal_newlines=True, check=True, cwd=cwd) + return result.stdout.strip() + + +def git_commit(dir='.'): + return exec(['git', 'rev-parse', f'--short={GIT_REF_LEN}', 'HEAD'], dir) + + +def parse_git_version(version): + # Parse, e.g.: wasi-sdk-21-0-g317548590b40+m + parts = version.replace('+', '-').split('-') + assert parts.pop(0) == 'wasi' + assert parts.pop(0) == 'sdk' + + major, minor = parts.pop(0), parts.pop(0) + git = None + dirty = False + + if parts: + # Check: git|dirty. + next = parts.pop(0) + if next == 'm': + dirty = True + else: + git = next[1:] + + # Check: dirty. + if parts: + assert parts.pop(0) == 'm', f'expected dirty flag: +m' + dirty = True + + assert not parts, f'unexpected suffixes: {parts}' + return major, minor, git, dirty + + +# Some inline tests to check Git version parsing: +assert parse_git_version( + 'wasi-sdk-21-0-g317548590b40+m') == ('21', '0', '317548590b40', True) +assert parse_git_version('wasi-sdk-21-2+m') == ('21', '2', None, True) +assert parse_git_version( + 'wasi-sdk-23-0-g317548590b40') == ('23', '0', '317548590b40', False) + + +def git_version(): + version = exec(['git', 'describe', '--long', '--candidates=999', + '--match=wasi-sdk-*', '--dirty=+m', f'--abbrev={GIT_REF_LEN}']) + major, minor, git, dirty = parse_git_version(version) + version = f'{major}.{minor}' + if git: + version += f'g{git}' + if dirty: + version += '+m' + return version + + +def parse_cmake_set(line): + return line.split(' ')[1].split(')')[0] + + +def llvm_cmake_version(llvm_dir): + with open(f'{llvm_dir}/llvm/CMakeLists.txt') as file: + for line in file: + line = line.strip() + if line.startswith('set(LLVM_VERSION_MAJOR'): + llvm_version_major = parse_cmake_set(line) + elif line.startswith('set(LLVM_VERSION_MINOR'): + llvm_version_minor = parse_cmake_set(line) + elif line.startswith('set(LLVM_VERSION_PATCH'): + llvm_version_patch = parse_cmake_set(line) + return llvm_version_major, llvm_version_minor, llvm_version_patch + + +def main(action, llvm_dir): + if action == 'wasi-sdk': + print(git_version()) + elif action == 'llvm': + major, minor, path = llvm_cmake_version(llvm_dir) + print(f'{major}.{minor}.{path}') + elif action == 'llvm-major': + major, _, _ = llvm_cmake_version(llvm_dir) + print(major) + elif action == 'dump': + print(git_version()) + print(f'wasi-libc: {git_commit("src/wasi-libc")}') + print(f'llvm: {git_commit(llvm_dir)}') + major, minor, path = llvm_cmake_version(llvm_dir) + print(f'llvm-version: {major}.{minor}.{path}') + print(f'config: {git_commit("src/config")}') + + +if __name__ == '__main__': + parser = argparse.ArgumentParser( + description='Print the various kinds of versions in wasi-sdk') + parser.add_argument('action', + choices=['wasi-sdk', 'llvm', 'llvm-major', 'dump'], + nargs='?', + default='wasi-sdk', + help='Which kind of version to print (default: wasi-sdk).') + parser.add_argument('--llvm-dir', + nargs='?', + default='src/llvm-project', + help='Override the location of the LLVM source directory (default: src/llvm-project).') + args = parser.parse_args() + main(args.action, args.llvm_dir) + sys.exit(0) diff --git a/version.sh b/version.sh deleted file mode 100755 index 35c8a908e..000000000 --- a/version.sh +++ /dev/null @@ -1,6 +0,0 @@ -#!/usr/bin/env bash -set -e -git config --global --add safe.directory "/workspace" -GIT_DESCR=$(git describe --long --candidates=999 --match='wasi-sdk-*' --dirty='+m' --abbrev=12) -GIT_PACKAGE_VERSION=$(echo $GIT_DESCR | perl -ne 'if(/^wasi-sdk-(\d+)-(\d+)-g([0-9a-f]{7,12})([+]m)?$/) { if($2 == 0) { print "$1.$2$4" } else { print "$1.$2g$3$4" } exit } else { print "could not parse git description"; exit 1 }';) -echo $GIT_PACKAGE_VERSION From 5a17d2aef4dbe5c05388f4fff1fecd5bbd6fd8a7 Mon Sep 17 00:00:00 2001 From: Andrew Brown Date: Mon, 18 Mar 2024 13:55:57 -0700 Subject: [PATCH 2/4] Dump a `VERSION` file in the build directory As noted in #372, some users may need to identify which version of wasi-sdk they are using after it has been downloaded. There are many ways to solve this, but the one I chose here is to dump the wasi-sdk version into a `VERSION` file and follow that up with several dependency versions that may be helpful to note when troubleshooting. Ideally someone could paste the contents of that file when filing a bug. If we adopt this approach, this fixes #372. --- Makefile | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/Makefile b/Makefile index cf3ccb17c..9c7acffbf 100644 --- a/Makefile +++ b/Makefile @@ -256,7 +256,11 @@ build/config.BUILT: cp cmake/Platform/WASI.cmake $(BUILD_PREFIX)/share/cmake/Platform touch build/config.BUILT -build: build/llvm.BUILT build/wasi-libc.BUILT build/compiler-rt.BUILT build/libcxx.BUILT build/config.BUILT +build/version.BUILT: + $(VERSION_SCRIPT) dump > $(BUILD_PREFIX)/VERSION + touch build/version.BUILT + +build: build/llvm.BUILT build/wasi-libc.BUILT build/compiler-rt.BUILT build/libcxx.BUILT build/config.BUILT build/version.BUILT strip: build/llvm.BUILT ./strip_symbols.sh $(BUILD_PREFIX)/bin From 0436568575e1bda40bd295c15e04b5cb03b75b02 Mon Sep 17 00:00:00 2001 From: Andrew Brown Date: Mon, 18 Mar 2024 13:57:42 -0700 Subject: [PATCH 3/4] Thread UID/GID through Docker When running Git commands inside this Docker container (i.e., commands that the `version.py` script needs for determining version information), the Docker build would run into issues like: ``` fatal: detected dubious ownership in repository at '/workspace' To add an exception for this directory, call: git config --global --add safe.directory /workspace ``` This is due to an extra Git check that detects that the Docker user is not the same one who owns the `.git` directory of this project. After looking into this, the best solution the internet has to offer is to thread the current user's UID and GID through the Docker image (i.e., the new `builder` user) and then `docker run --user ...`. This both avoids the Git check but also seems to be considered a best practice in some circles (?). --- Dockerfile | 32 +++++++++++++++++++++----------- docker_build.sh | 15 +++++++++++++-- 2 files changed, 34 insertions(+), 13 deletions(-) diff --git a/Dockerfile b/Dockerfile index 558c1b285..9a864e63e 100644 --- a/Dockerfile +++ b/Dockerfile @@ -3,18 +3,23 @@ # Here we choose Bionic 18.04. FROM ubuntu:bionic +# We want to use the same UID/GID of the external user to avoid permission +# issues. See the user setup at the end of the file. +ARG UID=1000 +ARG GID=1000 + RUN apt-get update \ - && apt-get install -y --no-install-recommends \ - ccache \ - curl \ - ca-certificates \ - build-essential \ - clang \ - python3 \ - git \ - ninja-build \ - && apt-get clean \ - && rm -rf /var/lib/apt/lists/* + && apt-get install -y --no-install-recommends \ + ccache \ + curl \ + ca-certificates \ + build-essential \ + clang \ + python3 \ + git \ + ninja-build \ + && apt-get clean \ + && rm -rf /var/lib/apt/lists/* RUN curl -sSLO https://github.com/Kitware/CMake/releases/download/v3.25.1/cmake-3.25.1-linux-x86_64.tar.gz \ && tar xf cmake-3.25.1-linux-x86_64.tar.gz \ @@ -22,3 +27,8 @@ RUN curl -sSLO https://github.com/Kitware/CMake/releases/download/v3.25.1/cmake- && mkdir -p /opt \ && mv cmake-3.25.1-linux-x86_64 /opt/cmake ENV PATH /opt/cmake/bin:$PATH + +RUN groupadd -g ${GID} builder && \ + useradd --create-home --uid ${UID} --gid ${GID} builder +USER builder +WORKDIR /workspace diff --git a/docker_build.sh b/docker_build.sh index 38845277f..050862fac 100755 --- a/docker_build.sh +++ b/docker_build.sh @@ -1,7 +1,18 @@ #!/bin/sh set -ex + echo "Building the docker image" -docker build -t wasi-sdk-builder:latest . +docker build \ + --build-arg UID=$(id -u) --build-arg GID=$(id -g) \ + -t wasi-sdk-builder:latest . + echo "Building the package in docker image" mkdir -p ~/.ccache -docker run --rm -v "$PWD":/workspace -v ~/.ccache:/root/.ccache -e NINJA_FLAGS=-v --workdir /workspace --tmpfs /tmp:exec wasi-sdk-builder:latest make package LLVM_CMAKE_FLAGS=-DLLVM_CCACHE_BUILD=ON +docker run --rm \ + --user $(id -u):$(id -g) \ + -v "$PWD":/workspace:Z \ + -v ~/.ccache:/home/builder/.ccache:Z \ + -e NINJA_FLAGS=-v \ + --tmpfs /tmp:exec \ + wasi-sdk-builder:latest \ + make package LLVM_CMAKE_FLAGS=-DLLVM_CCACHE_BUILD=ON From 993f6484da15447afea09de099243a0bbde71e0a Mon Sep 17 00:00:00 2001 From: Andrew Brown Date: Mon, 18 Mar 2024 14:08:40 -0700 Subject: [PATCH 4/4] Handle new LLVM version path In #399, @glandium points out that newer versions of LLVM will place their version information at a different path. This change adapts #399 to the new Python version script. --- version.py | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/version.py b/version.py index 52dc07ab1..2d9de6d5b 100755 --- a/version.py +++ b/version.py @@ -5,9 +5,10 @@ # # Usage: version [wasi-sdk|llvm|llvm-major|dump] [--llvm-dir=] -import sys import argparse +import os import subprocess +import sys # The number of characters to use for the abbreviated Git revision. GIT_REF_LEN = 12 @@ -75,7 +76,11 @@ def parse_cmake_set(line): def llvm_cmake_version(llvm_dir): - with open(f'{llvm_dir}/llvm/CMakeLists.txt') as file: + path = f'{llvm_dir}/cmake/Modules/LLVMVersion.cmake' + if not os.path.exists(path): + # Handle older LLVM versions; see #399. + path = f'{llvm_dir}/llvm/CMakeLists.txt' + with open(path) as file: for line in file: line = line.strip() if line.startswith('set(LLVM_VERSION_MAJOR'):