Skip to content
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

[nfc] use _bazel.yml from the release.yml #3079

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
40 changes: 31 additions & 9 deletions .github/workflows/_bazel.yml
Original file line number Diff line number Diff line change
Expand Up @@ -6,10 +6,6 @@ on:
type: string
required: false
default: "ubuntu-20.04"
os_name:
type: string
required: false
default: "linux"
suffix:
type: string
required: false
Expand All @@ -22,6 +18,18 @@ on:
type: string
required: false
default: ""
cache_prefix:
type: string
required: false
default: "bazel-disk-cache-"
target:
type: string
required: false
default: "//..."
upload_binary:
type: boolean
required: false
default: false
secrets:
BAZEL_CACHE_KEY:
required: true
Expand All @@ -40,12 +48,12 @@ jobs:
uses: actions/cache@v4
with:
path: ~/bazel-disk-cache
key: bazel-disk-cache-${{ inputs.os_name }}-${{ runner.arch }}${{ inputs.suffix }}-${{ hashFiles('.bazelversion', '.bazelrc', 'WORKSPACE') }}
key: ${{ inputs.cache_prefix }}${{ runner.os }}-${{ runner.arch }}${{ inputs.suffix }}-${{ hashFiles('.bazelversion', '.bazelrc', 'WORKSPACE') }}
# Intentionally not reusing an older cache entry using a key prefix, bazel frequently
# ends up with a larger cache at the end when starting with an available cache entry,
# resulting in a snowballing cache size and cache download/upload times.
- name: Setup Linux
if: inputs.os_name == 'linux'
if: runner.os == 'Linux'
# Install dependencies, including clang through the LLVM APT repository. We drop the
# install step so we can install just the packages we need.
# libunwind, libc++abi1 and libc++1 should be automatically installed as dependencies of
Expand All @@ -62,12 +70,20 @@ jobs:
sudo apt-get install -y --no-install-recommends clang-16 lld-16 libunwind-16 libc++abi1-16 libc++1-16 libc++-16-dev libclang-rt-16-dev llvm-16
sed -i -e "s%llvm-symbolizer%/usr/lib/llvm-16/bin/llvm-symbolizer%" .bazelrc
- name: Setup Windows
if: inputs.os_name == 'windows'
if: runner.os == 'Windows'
# Set a custom output root directory to avoid long file name issues.
run: |
git config --global core.symlinks true
git config --show-scope --show-origin core.symlinks
[System.IO.File]::WriteAllLines((Join-Path -Path $env:USERPROFILE -ChildPath '.bazelrc'), 'startup --output_user_root=C:/tmp')
- name: Setup macOS
if: runner.os == 'macOS'
run: |
sudo xcode-select -s "/Applications/Xcode_15.1.app"
Copy link
Collaborator

Choose a reason for hiding this comment

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

Test uses macos-15, where only Xcode 16 is available, so this won't work as-is – let's just land #3078 first so that they have the same OS version and the xcode-select command won't be needed

Copy link
Collaborator

Choose a reason for hiding this comment

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

Thank you for approving! I think ci-macOS-release will also need --config=macos-cross-x86_64 now.

# Install lld and link it to /usr/local/bin. We overwrite any existing link, which may
# exist from an older pre-installed LLVM version on the runner image.
brew install lld
Copy link
Collaborator

Choose a reason for hiding this comment

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

Can we keep this step release-only? Installing llvm + lld takes some time, which will make builds take longer, especially for builds when the bazel cache is warm. lld provides few advantages to the test build except for slightly faster link speeds which won't make up for this (the macOS system linker has become faster in recent releases).

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Can we keep this step release-only?

hard to do without introducing extra switches. Also arguably we should use the same version of xcode and linker both in release and in test configuration?

Copy link
Collaborator

Choose a reason for hiding this comment

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

We are using the same Xcode version now that #3078 has been merged. Apple ld and lld on macOS are high-quality linkers that have very good compatibility/interop, so I think we should only spend the 80s - 90s it takes to fetch lld/LLVM for release builds, where we care more about the binary size.
You can just do if: ${{ (if: runner.os == 'macOS') && inputs.upload_binary }}' right? Since the Xcode flag is no longer needed the setup macOS step only contains the lld step. Then we could rename upload_binary to is_release, making it more descriptive.

sudo ln -s -f $(brew --prefix lld)/bin/ld64.lld /usr/local/bin/ld64.lld
- name: Configure download mirrors
shell: bash
run: |
Expand All @@ -82,11 +98,17 @@ jobs:
# timestamps are no longer being added here, the GitHub logs include timestamps (Use
# 'Show timestamps' on the web interface)
run: |
bazel build --config=ci --config=ci-${{ inputs.os_name }}${{ inputs.suffix }} --remote_cache=https://bazel:${{ secrets.BAZEL_CACHE_KEY }}@bazel-remote-cache.devprod.cloudflare.dev ${{ inputs.extra_bazel_args }} //...
bazel build --config=ci --config=ci-${{ runner.os }}${{ inputs.suffix }} ${{ inputs.extra_bazel_args }} --remote_cache=https://bazel:${{ secrets.BAZEL_CACHE_KEY }}@bazel-remote-cache.devprod.cloudflare.dev ${{ inputs.target }}
- name: Bazel test
if: inputs.run_tests
run: |
bazel test --config=ci --config=ci-${{ inputs.os_name }}${{ inputs.suffix }} --remote_cache=https://bazel:${{ secrets.BAZEL_CACHE_KEY }}@bazel-remote-cache.devprod.cloudflare.dev ${{ inputs.extra_bazel_args }} //...
bazel test --config=ci --config=ci-${{ runner.os }}${{ inputs.suffix }} ${{ inputs.extra_bazel_args }} --remote_cache=https://bazel:${{ secrets.BAZEL_CACHE_KEY }}@bazel-remote-cache.devprod.cloudflare.dev ${{ inputs.target }}
- name: Upload binary
if: inputs.upload_binary
uses: actions/upload-artifact@v4
with:
name: ${{ runner.os }}-${{ runner.arch }}-binary
path: bazel-bin/src/workerd/server/workerd${{ runner.os == 'Windows' && '.exe' || '' }}
- name: Report disk usage (in MB)
if: always()
shell: bash
Expand Down
90 changes: 20 additions & 70 deletions .github/workflows/release.yml
Original file line number Diff line number Diff line change
Expand Up @@ -56,77 +56,27 @@ jobs:
build:
strategy:
matrix:
os: [ubuntu-20.04, macos-13, windows-2022]
image: [ubuntu-20.04, macos-13, windows-2022]
include:
- os-name: linux
os: ubuntu-20.04
bazel-config: release_linux
- os-name: macOS
os: macos-13
bazel-config: release_macos
- os-name: windows
os: windows-2022
bazel-config: release_windows
runs-on: ${{ matrix.os }}
name: build (${{ matrix.os-name }})
steps:
- uses: actions/checkout@v4
with:
show-progress: false
- name: Cache
id: cache
uses: actions/cache@v4
with:
path: ~/bazel-disk-cache
# Use a different cache key than for tests here, otherwise the release cache could end up
# being used for test builds, where it provides little benefit.
key: bazel-disk-cache-release-${{ runner.os }}-${{ runner.arch }}-${{ hashFiles('.bazelversion', '.bazelrc', 'WORKSPACE') }}
- name: Setup Linux
if: runner.os == 'Linux'
run: |
export DEBIAN_FRONTEND=noninteractive
wget https://apt.llvm.org/llvm.sh
sed -i '/apt-get install/d' llvm.sh
chmod +x llvm.sh
sudo ./llvm.sh 16
sudo apt-get install -y --no-install-recommends clang-16 lld-16 libunwind-16 libc++abi1-16 libc++1-16 libc++-16-dev
echo "build:linux --action_env=CC=/usr/lib/llvm-16/bin/clang --action_env=CXX=/usr/lib/llvm-16/bin/clang++" >> .bazelrc
echo "build:linux --host_action_env=CC=/usr/lib/llvm-16/bin/clang --host_action_env=CXX=/usr/lib/llvm-16/bin/clang++" >> .bazelrc
- name: Setup macOS
if: runner.os == 'macOS'
run: |
sudo xcode-select -s "/Applications/Xcode_15.1.app"
# Install lld and link it to /usr/local/bin. We overwrite any existing link, which may
# exist from an older pre-installed LLVM version on the runner image.
brew install lld
sudo ln -s -f $(brew --prefix lld)/bin/ld64.lld /usr/local/bin/ld64.lld
# Enable lld identical code folding to significantly reduce binary size.
echo "build:macos --config=macos_lld_icf" >> .bazelrc
- name: Setup Windows
if: runner.os == 'Windows'
run: |
git config --global core.symlinks true
git config --show-scope --show-origin core.symlinks
# Set a custom output root directory to avoid long file name issues.
[System.IO.File]::WriteAllLines((Join-Path -Path $env:USERPROFILE -ChildPath '.bazelrc'), 'startup --output_user_root=C:/tmp')
- name: Configure download mirrors
shell: bash
run: |
if [ ! -z "${{ secrets.WORKERS_MIRROR_URL }}" ] ; then
# Strip comment in front of WORKERS_MIRROR_URL, then substitute secret to use it.
sed -e '/WORKERS_MIRROR_URL/ { s@# *@@; s@WORKERS_MIRROR_URL@${{ secrets.WORKERS_MIRROR_URL }}@; }' -i.bak WORKSPACE
fi
- name: Bazel build
# Strip debug info here – we don't generate debug info but some is pulled in from external
# static libraries, for example the Rust STL. This is equivalent to the -Wl,-S linker
# option, symbols will not be removed.
run: |
bazel build --disk_cache=~/bazel-disk-cache --strip=always --remote_cache=https://bazel:${{ secrets.BAZEL_CACHE_KEY }}@bazel-remote-cache.devprod.cloudflare.dev --config=ci --config=${{ matrix.bazel-config }} //src/workerd/server:workerd
- name: Upload binary
uses: actions/upload-artifact@v4
with:
name: ${{ runner.os }}-${{ runner.arch }}-binary
path: bazel-bin/src/workerd/server/workerd${{ runner.os == 'Windows' && '.exe' || '' }}
- os: Linux
image: ubuntu-20.04
- os: macOS
image: macos-13
- os: Windows
image: windows-2022
uses: ./.github/workflows/_bazel.yml
with:
image: ${{ matrix.image }}
target: "//src/workerd/server:workerd"
extra_bazel_args: "--config=ci-release --config=ci-${{ matrix.os }}-release"
# Use a different cache key than for tests here, otherwise the release cache could end up
# being used for test builds, where it provides little benefit.
cache_prefix: "bazel-disk-cache-release-"
run_tests: false
upload_binary: true
secrets:
BAZEL_CACHE_KEY: ${{ secrets.BAZEL_CACHE_KEY }}
WORKERS_MIRROR_URL: ${{ secrets.WORKERS_MIRROR_URL }}

upload-artifacts:
name: Upload Artifacts
Expand Down
19 changes: 9 additions & 10 deletions .github/workflows/test.yml
Original file line number Diff line number Diff line change
Expand Up @@ -27,9 +27,9 @@ jobs:
matrix:
os:
[
{ name : linux, image : ubuntu-20.04 },
{ name : macOS, image : macos-15 },
{ name : windows, image : windows-2022 }
{ image : ubuntu-20.04 },
{ image : macos-15 },
{ image : windows-2022 }
]
config:
[
Expand All @@ -40,32 +40,31 @@ jobs:
]
include:
# Add an Address Sanitizer (ASAN) build on Linux for additional checking.
- os: { name: linux, image: ubuntu-20.04 }
- os: { image: ubuntu-20.04 }
config: { suffix: -asan }
# Windows has a custom non-debug bazel config.
- os: { name : windows, image : windows-2022 }
- os: { image : windows-2022 }
config: { suffix: '' }
# TODO (later): The custom Windows-debug configuration consistently runs out of disk
# space on CI, disable it for now. Once https://github.com/bazelbuild/bazel/issues/21615
# has been resolved we can likely re-enable it and possibly fold up the custom
# configurations, as we can more easily disable PDB file generation.
# - os: { name : windows, image : windows-2022 }
# - os: { image : windows-2022 }
# config: { suffix: -debug, bazel-args: --config=windows_dbg }
exclude:
# Skip the matrix generated Windows non-debug config to use the one added above.
- os: { name : windows, image : windows-2022 }
- os: { image : windows-2022 }
config: { suffix: '' }
- os: { name : windows, image : windows-2022 }
- os: { image : windows-2022 }
config: { suffix: -debug }
# due to resource constraints, exclude the macOS-debug runner for now. linux-debug and
# linux-asan should provide sufficient coverage for building in the debug configuration.
- os: { name : macOS, image : macos-15 }
- os: { image : macos-15 }
config: { suffix: -debug }
fail-fast: false
uses: ./.github/workflows/_bazel.yml
with:
image: ${{ matrix.os.image }}
os_name: ${{ matrix.os.name }}
suffix: ${{ matrix.config.suffix }}
extra_bazel_args: "--config=ci-test"
secrets:
Expand Down
46 changes: 30 additions & 16 deletions build/ci.bazelrc
Original file line number Diff line number Diff line change
Expand Up @@ -44,35 +44,49 @@ build:ci-limit-storage --copt="-fno-standalone-debug"

# ci-platform dependent configuration

build:ci-linux-common --copt='-Werror'
build:ci-linux-common --copt='-Wno-error=#warnings'
build:ci-linux-common --copt='-Wno-error=deprecated-declarations'
build:ci-Linux-common --copt='-Werror'
build:ci-Linux-common --copt='-Wno-error=#warnings'
build:ci-Linux-common --copt='-Wno-error=deprecated-declarations'
# keep in sync with .github/workflows/test.yml
build:ci-linux-common --action_env=CC=/usr/lib/llvm-16/bin/clang --action_env=CXX=/usr/lib/llvm-16/bin/clang++
build:ci-linux-common --host_action_env=CC=/usr/lib/llvm-16/bin/clang --host_action_env=CXX=/usr/lib/llvm-16/bin/clang++
build:ci-Linux-common --action_env=CC=/usr/lib/llvm-16/bin/clang --action_env=CXX=/usr/lib/llvm-16/bin/clang++
build:ci-Linux-common --host_action_env=CC=/usr/lib/llvm-16/bin/clang --host_action_env=CXX=/usr/lib/llvm-16/bin/clang++

build:ci-linux --config=ci-linux-common
build:ci-Linux --config=ci-Linux-common
# Some tests (like Python import tests) take a long time to run and don't benefit much
# from multi-platform testing. For that reason, we only run them in a single configuration
# to minimize effect on CI pipeline runtime.
build:ci-linux --build_tag_filters=-off-by-default
test:ci-linux --test_tag_filters=-off-by-default --test_size_filters=small,medium,large,enormous
build:ci-Linux --build_tag_filters=-off-by-default
test:ci-Linux --test_tag_filters=-off-by-default --test_size_filters=small,medium,large,enormous

build:ci-linux-debug --config=ci-linux-common --config=ci-limit-storage
build:ci-linux-debug --config=debug --config=rust-debug
build:ci-Linux-debug --config=ci-Linux-common --config=ci-limit-storage
build:ci-Linux-debug --config=debug --config=rust-debug

build:ci-linux-asan --config=ci-linux-common --config=ci-limit-storage
build:ci-Linux-asan --config=ci-Linux-common --config=ci-limit-storage
# we're really struggling to fit asan build into worker disk size
# having asan without symbols is better than none
build:ci-linux-asan --config=asan --copt="-g0" --strip=always
build:ci-Linux-asan --config=asan --copt="-g0" --strip=always

# Enable lld identical code folding to significantly reduce binary size.
Copy link
Collaborator

@fhanau fhanau Nov 7, 2024

Choose a reason for hiding this comment

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

ICF is primarily useful for release builds – link time slightly more than doubles with this. You could make a case for it based on disk space usage and the time it takes to fetch binaries from cache, but based on the download speed that CI has this would be a net loss for test build time.
Edit: Also, ci-macOS-common step is not being added to ci-macOS-release. Let's just add --config=macos_lld_icf to that directly.

build:ci-macOS-common --config=macos_lld_icf

# Unlike the bazel Unix toolchain the macOS toolchain sets "-O0 -DDEBUG" for fastbuild by
# default. This is unhelpful for compile speeds and test performance, remove the DEBUG
# define.
build:ci-macOS --copt=-UDEBUG
build:ci-macOS --config=ci-macOS-common --copt=-UDEBUG

build:ci-macOS-debug --config=debug
build:ci-macOS-debug --config=ci-macOS-common --config=debug

build:ci-windows --config=windows_no_dbg
build:ci-windows-debug --config=debug
build:ci-Windows --config=windows_no_dbg
build:ci-Windows-debug --config=debug


## Release configurations

# Strip debug info here – we don't generate debug info but some is pulled in from external
# static libraries, for example the Rust STL. This is equivalent to the -Wl,-S linker
# option, symbols will not be removed.
build:ci-release --strip=always

build:ci-Windows-release --config=release_windows
build:ci-Linux-release --config=release_linux
build:ci-macOS-release --config=release_macos
Loading