Skip to content

Commit

Permalink
watcher: major refactor to use Maybe types for better error reporti…
Browse files Browse the repository at this point in the history
…ng (#10492)
  • Loading branch information
paperdave authored Apr 26, 2024
1 parent 9ba1181 commit 006575a
Show file tree
Hide file tree
Showing 27 changed files with 746 additions and 484 deletions.
8 changes: 4 additions & 4 deletions .github/workflows/build-darwin.yml
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ on:
type: boolean

env:
LLVM_VERSION: 17
LLVM_VERSION: 16
BUN_VERSION: 1.1.2

jobs:
Expand Down Expand Up @@ -79,7 +79,7 @@ jobs:
[email protected] \
ninja \
golang \
gnu-sed --force
gnu-sed --force --overwrite
echo "$(brew --prefix ccache)/bin" >> $GITHUB_PATH
echo "$(brew --prefix coreutils)/libexec/gnubin" >> $GITHUB_PATH
echo "$(brew --prefix llvm@$LLVM_VERSION)/bin" >> $GITHUB_PATH
Expand Down Expand Up @@ -136,7 +136,7 @@ jobs:
[email protected] \
ninja \
golang \
gnu-sed --force
gnu-sed --force --overwrite
echo "$(brew --prefix ccache)/bin" >> $GITHUB_PATH
echo "$(brew --prefix coreutils)/libexec/gnubin" >> $GITHUB_PATH
echo "$(brew --prefix llvm@$LLVM_VERSION)/bin" >> $GITHUB_PATH
Expand Down Expand Up @@ -218,7 +218,7 @@ jobs:
[email protected] \
ninja \
golang \
gnu-sed --force
gnu-sed --force --overwrite
echo "$(brew --prefix ccache)/bin" >> $GITHUB_PATH
echo "$(brew --prefix coreutils)/libexec/gnubin" >> $GITHUB_PATH
echo "$(brew --prefix llvm@$LLVM_VERSION)/bin" >> $GITHUB_PATH
Expand Down
2 changes: 1 addition & 1 deletion .github/workflows/build-windows.yml
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ on:

env:
# Must specify exact version of LLVM for Windows
LLVM_VERSION: 17.0.6
LLVM_VERSION: 16.0.6
BUN_VERSION: 1.1.2

jobs:
Expand Down
12 changes: 6 additions & 6 deletions .github/workflows/ci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -81,7 +81,7 @@ jobs:
uses: ./.github/workflows/build-darwin.yml
secrets: inherit
with:
runs-on: ${{ github.repository_owner == 'oven-sh' && 'macos-13-large' || 'macos-13' }}
runs-on: ${{ github.repository_owner == 'oven-sh' && 'macos-12-large' || 'macos-12' }}
tag: darwin-x64
arch: x64
cpu: haswell
Expand All @@ -92,7 +92,7 @@ jobs:
uses: ./.github/workflows/build-darwin.yml
secrets: inherit
with:
runs-on: ${{ github.repository_owner == 'oven-sh' && 'macos-13-large' || 'macos-13' }}
runs-on: ${{ github.repository_owner == 'oven-sh' && 'macos-12-large' || 'macos-12' }}
tag: darwin-x64-baseline
arch: x64
cpu: nehalem
Expand All @@ -103,7 +103,7 @@ jobs:
uses: ./.github/workflows/build-darwin.yml
secrets: inherit
with:
runs-on: ${{ github.repository_owner == 'oven-sh' && 'namespace-profile-bun-ci-darwin-aarch64' || 'macos-13' }}
runs-on: ${{ github.repository_owner == 'oven-sh' && 'namespace-profile-bun-ci-darwin-aarch64' || 'macos-12' }}
tag: darwin-aarch64
arch: aarch64
cpu: native
Expand Down Expand Up @@ -172,7 +172,7 @@ jobs:
with:
run-id: ${{ inputs.run-id }}
pr-number: ${{ github.event.number }}
runs-on: ${{ github.repository_owner == 'oven-sh' && 'macos-13-large' || 'macos-13' }}
runs-on: ${{ github.repository_owner == 'oven-sh' && 'macos-12-large' || 'macos-12' }}
tag: darwin-x64
darwin-x64-baseline-test:
if: ${{ inputs.run-id && always() || github.event_name == 'pull_request' }}
Expand All @@ -183,7 +183,7 @@ jobs:
with:
run-id: ${{ inputs.run-id }}
pr-number: ${{ github.event.number }}
runs-on: ${{ github.repository_owner == 'oven-sh' && 'macos-13-large' || 'macos-13' }}
runs-on: ${{ github.repository_owner == 'oven-sh' && 'macos-12-large' || 'macos-12' }}
tag: darwin-x64-baseline
darwin-aarch64-test:
if: ${{ inputs.run-id && always() || github.event_name == 'pull_request' }}
Expand All @@ -194,7 +194,7 @@ jobs:
with:
run-id: ${{ inputs.run-id }}
pr-number: ${{ github.event.number }}
runs-on: ${{ github.repository_owner == 'oven-sh' && 'namespace-profile-bun-ci-darwin-aarch64' || 'macos-13' }}
runs-on: ${{ github.repository_owner == 'oven-sh' && 'namespace-profile-bun-ci-darwin-aarch64' || 'macos-12' }}
tag: darwin-aarch64
windows-x64-test:
if: ${{ inputs.run-id && always() || github.event_name == 'pull_request' }}
Expand Down
6 changes: 3 additions & 3 deletions .github/workflows/create-release-build.yml
Original file line number Diff line number Diff line change
Expand Up @@ -85,7 +85,7 @@ jobs:
uses: ./.github/workflows/build-darwin.yml
secrets: inherit
with:
runs-on: ${{ github.repository_owner == 'oven-sh' && 'macos-13-large' || 'macos-13' }}
runs-on: ${{ github.repository_owner == 'oven-sh' && 'macos-12-large' || 'macos-12' }}
tag: darwin-x64
arch: x64
cpu: haswell
Expand All @@ -95,7 +95,7 @@ jobs:
uses: ./.github/workflows/build-darwin.yml
secrets: inherit
with:
runs-on: ${{ github.repository_owner == 'oven-sh' && 'macos-13-large' || 'macos-13' }}
runs-on: ${{ github.repository_owner == 'oven-sh' && 'macos-12-large' || 'macos-12' }}
tag: darwin-x64-baseline
arch: x64
cpu: nehalem
Expand All @@ -105,7 +105,7 @@ jobs:
uses: ./.github/workflows/build-darwin.yml
secrets: inherit
with:
runs-on: ${{ github.repository_owner == 'oven-sh' && 'namespace-profile-bun-ci-darwin-aarch64' || 'macos-13' }}
runs-on: ${{ github.repository_owner == 'oven-sh' && 'namespace-profile-bun-ci-darwin-aarch64' || 'macos-12' }}
tag: darwin-aarch64
arch: aarch64
cpu: native
Expand Down
5 changes: 2 additions & 3 deletions CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ cmake_policy(SET CMP0091 NEW)
cmake_policy(SET CMP0067 NEW)

set(Bun_VERSION "1.1.5")
set(WEBKIT_TAG 2ce7bb4314c516d9c7ba12be3581f6a986013781)
set(WEBKIT_TAG 5fe78270f2efca3c0ee3013259776923053d5011)

set(BUN_WORKDIR "${CMAKE_CURRENT_BINARY_DIR}")
message(STATUS "Configuring Bun ${Bun_VERSION} in ${BUN_WORKDIR}")
Expand Down Expand Up @@ -107,7 +107,7 @@ endif()
# we do some extra work afterwards to double-check, and we will rerun BUN_FIND_LLVM if the compiler did not match.
#
# If the user passes -DLLVM_PREFIX, most of this logic is skipped, but we still warn if invalid.
set(LLVM_VERSION 17)
set(LLVM_VERSION 16)

macro(BUN_FIND_LLVM)
find_program(
Expand Down Expand Up @@ -1119,7 +1119,6 @@ if(APPLE)
target_link_options(${bun} PUBLIC "-dead_strip")
target_link_options(${bun} PUBLIC "-dead_strip_dylibs")
target_link_options(${bun} PUBLIC "-Wl,-stack_size,0x1200000")
target_link_options(${bun} PUBLIC "-ld64") # fixes macOS x64 linking issue
target_link_options(${bun} PUBLIC "-exported_symbols_list" "${BUN_SRC}/symbols.txt")
set_target_properties(${bun} PROPERTIES LINK_DEPENDS "${BUN_SRC}/symbols.txt")

Expand Down
12 changes: 7 additions & 5 deletions Dockerfile
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ ARG ZIG_OPTIMIZE=ReleaseFast
ARG CMAKE_BUILD_TYPE=Release

ARG NODE_VERSION="20"
ARG LLVM_VERSION="17"
ARG LLVM_VERSION="16"
ARG ZIG_VERSION="0.12.0-dev.1828+225fe6ddb"

ARG SCCACHE_BUCKET
Expand All @@ -34,7 +34,7 @@ ARG SCCACHE_ENDPOINT
ARG AWS_ACCESS_KEY_ID
ARG AWS_SECRET_ACCESS_KEY

FROM bitnami/minideb:bookworm as bun-base
FROM bitnami/minideb:bullseye as bun-base

ARG BUN_DOWNLOAD_URL_BASE
ARG DEBIAN_FRONTEND
Expand All @@ -53,7 +53,7 @@ ENV BUN_DEPS_OUT_DIR=${BUN_DEPS_OUT_DIR}

ENV CXX=clang++-${LLVM_VERSION}
ENV CC=clang-${LLVM_VERSION}
ENV AR=/usr/bin/llvm-ar
ENV AR=/usr/bin/llvm-ar-${LLVM_VERSION}
ENV LD=lld-${LLVM_VERSION}

ENV SCCACHE_BUCKET=${SCCACHE_BUCKET}
Expand All @@ -67,11 +67,13 @@ RUN install_packages \
ca-certificates \
curl \
gnupg \
&& echo "deb https://apt.llvm.org/bookworm/ llvm-toolchain-bookworm-${LLVM_VERSION} main" > /etc/apt/sources.list.d/llvm.list \
&& echo "deb-src https://apt.llvm.org/bookworm/ llvm-toolchain-bookworm-${LLVM_VERSION} main" >> /etc/apt/sources.list.d/llvm.list \
&& echo "deb https://apt.llvm.org/bullseye/ llvm-toolchain-bullseye-${LLVM_VERSION} main" > /etc/apt/sources.list.d/llvm.list \
&& echo "deb-src https://apt.llvm.org/bullseye/ llvm-toolchain-bullseye-${LLVM_VERSION} main" >> /etc/apt/sources.list.d/llvm.list \
&& curl -fsSL "https://apt.llvm.org/llvm-snapshot.gpg.key" | apt-key add - \
&& echo "deb https://deb.nodesource.com/node_${NODE_VERSION}.x nodistro main" > /etc/apt/sources.list.d/nodesource.list \
&& curl -fsSL "https://deb.nodesource.com/gpgkey/nodesource-repo.gpg.key" | apt-key add - \
&& echo "deb https://apt.kitware.com/ubuntu/ focal main" > /etc/apt/sources.list.d/kitware.list \
&& curl -fsSL "https://apt.kitware.com/keys/kitware-archive-latest.asc" | apt-key add - \
&& install_packages \
wget \
bash \
Expand Down
12 changes: 6 additions & 6 deletions Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -81,8 +81,8 @@ ZIG ?= $(shell which zig 2>/dev/null || echo -e "error: Missing zig. Please make
# This is easier to happen than you'd expect.
# Using realpath here causes issues because clang uses clang++ as a symlink
# so if that's resolved, it won't build for C++
REAL_CC = $(shell which clang-17 2>/dev/null || which clang 2>/dev/null)
REAL_CXX = $(shell which clang++-17 2>/dev/null || which clang++ 2>/dev/null)
REAL_CC = $(shell which clang-16 2>/dev/null || which clang 2>/dev/null)
REAL_CXX = $(shell which clang++-16 2>/dev/null || which clang++ 2>/dev/null)
CLANG_FORMAT = $(shell which clang-format-16 2>/dev/null || which clang-format 2>/dev/null)

CC = $(REAL_CC)
Expand All @@ -107,14 +107,14 @@ CC_WITH_CCACHE = $(CCACHE_PATH) $(CC)
ifeq ($(OS_NAME),darwin)
# Find LLVM
ifeq ($(wildcard $(LLVM_PREFIX)),)
LLVM_PREFIX = $(shell brew --prefix llvm@17)
LLVM_PREFIX = $(shell brew --prefix llvm@16)
endif
ifeq ($(wildcard $(LLVM_PREFIX)),)
LLVM_PREFIX = $(shell brew --prefix llvm)
endif
ifeq ($(wildcard $(LLVM_PREFIX)),)
# This is kinda ugly, but I can't find a better way to error :(
LLVM_PREFIX = $(shell echo -e "error: Unable to find llvm. Please run 'brew install llvm@17' or set LLVM_PREFIX=/path/to/llvm")
LLVM_PREFIX = $(shell echo -e "error: Unable to find llvm. Please run 'brew install llvm@16' or set LLVM_PREFIX=/path/to/llvm")
endif

LDFLAGS += -L$(LLVM_PREFIX)/lib
Expand Down Expand Up @@ -154,7 +154,7 @@ CMAKE_FLAGS_WITHOUT_RELEASE = -DCMAKE_C_COMPILER=$(CC) \
-DCMAKE_OSX_DEPLOYMENT_TARGET=$(MIN_MACOS_VERSION) \
$(CMAKE_CXX_COMPILER_LAUNCHER_FLAG) \
-DCMAKE_AR=$(AR) \
-DCMAKE_RANLIB=$(which llvm-17-ranlib 2>/dev/null || which llvm-ranlib 2>/dev/null)
-DCMAKE_RANLIB=$(which llvm-16-ranlib 2>/dev/null || which llvm-ranlib 2>/dev/null)



Expand Down Expand Up @@ -657,7 +657,7 @@ endif
.PHONY: assert-deps
assert-deps:
@echo "Checking if the required utilities are available..."
@if [ $(CLANG_VERSION) -lt "15" ]; then echo -e "ERROR: clang version >=15 required, found: $(CLANG_VERSION). Install with:\n\n $(POSIX_PKG_MANAGER) install llvm@17"; exit 1; fi
@if [ $(CLANG_VERSION) -lt "15" ]; then echo -e "ERROR: clang version >=15 required, found: $(CLANG_VERSION). Install with:\n\n $(POSIX_PKG_MANAGER) install llvm@16"; exit 1; fi
@cmake --version >/dev/null 2>&1 || (echo -e "ERROR: cmake is required."; exit 1)
@$(PYTHON) --version >/dev/null 2>&1 || (echo -e "ERROR: python is required."; exit 1)
@$(ESBUILD) --version >/dev/null 2>&1 || (echo -e "ERROR: esbuild is required."; exit 1)
Expand Down
4 changes: 2 additions & 2 deletions docs/project/building-windows.md
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,7 @@ By default, running unverified scripts are blocked.

After Visual Studio, you need the following:

- LLVM 17
- LLVM 16
- Go
- Rust
- NASM
Expand All @@ -73,7 +73,7 @@ The Zig compiler is automatically downloaded, installed, and updated by the buil
> irm https://get.scoop.sh | iex
> scoop install nodejs-lts go rust nasm ruby perl
# scoop seems to be buggy if you install llvm and the rest at the same time
> scoop llvm@17.0.6
> scoop llvm@16.0.6
```

If you intend on building WebKit locally (optional), you should install these packages:
Expand Down
22 changes: 11 additions & 11 deletions docs/project/contributing.md
Original file line number Diff line number Diff line change
Expand Up @@ -53,17 +53,17 @@ $ brew install bun

## Install LLVM

Bun requires LLVM 17/Clang 17 (`clang` is part of LLVM). This version requirement is to match WebKit (precompiled), as mismatching versions will cause memory allocation failures at runtime. In most cases, you can install LLVM through your system package manager:
Bun requires LLVM 16 (`clang` is part of LLVM). This version requirement is to match WebKit (precompiled), as mismatching versions will cause memory allocation failures at runtime. In most cases, you can install LLVM through your system package manager:

{% codetabs %}

```bash#macOS (Homebrew)
$ brew install llvm@17
$ brew install llvm@16
```

```bash#Ubuntu/Debian
$ # LLVM has an automatic installation script that is compatible with all versions of Ubuntu
$ wget https://apt.llvm.org/llvm.sh -O - | sudo bash -s -- 17 all
$ wget https://apt.llvm.org/llvm.sh -O - | sudo bash -s -- 16 all
```

```bash#Arch
Expand All @@ -77,17 +77,17 @@ $ sudo dnf install llvm clang lld
```

```bash#openSUSE Tumbleweed
$ sudo zypper install clang17 lld17 llvm17
$ sudo zypper install clang16 lld16 llvm16
```

{% /codetabs %}

If none of the above solutions apply, you will have to install it [manually](https://github.com/llvm/llvm-project/releases/tag/llvmorg-17.0.6).
If none of the above solutions apply, you will have to install it [manually](https://github.com/llvm/llvm-project/releases/tag/llvmorg-16.0.6).

Make sure Clang/LLVM 17 is in your path:
Make sure Clang/LLVM 16 is in your path:

```bash
$ which clang-17
$ which clang-16
```

If not, run this to manually add it:
Expand All @@ -96,13 +96,13 @@ If not, run this to manually add it:

```bash#macOS (Homebrew)
# use fish_add_path if you're using fish
# use path+="$(brew --prefix llvm@17)/bin" if you are using zsh
$ export PATH="$(brew --prefix llvm@17)/bin:$PATH"
# use path+="$(brew --prefix llvm@16)/bin" if you are using zsh
$ export PATH="$(brew --prefix llvm@16)/bin:$PATH"
```

```bash#Arch
# use fish_add_path if you're using fish
$ export PATH="$PATH:/usr/lib/llvm17/bin"
$ export PATH="$PATH:/usr/lib/llvm16/bin"
```

{% /codetabs %}
Expand Down Expand Up @@ -263,7 +263,7 @@ The issue may manifest when initially running `bun setup` as Clang being unable
```
The C++ compiler
"/usr/bin/clang++-17"
"/usr/bin/clang++-16"
is not able to compile a simple test program.
```
Expand Down
2 changes: 1 addition & 1 deletion scripts/setup.sh
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ fail() {
printf "${C_RED}setup error${C_RESET}: %s\n" "$@"
}

LLVM_VERSION=17
LLVM_VERSION=16

# this compiler detection could be better
# it is copy pasted from ./env.sh
Expand Down
24 changes: 16 additions & 8 deletions src/bun.js/javascript.zig
Original file line number Diff line number Diff line change
Expand Up @@ -476,11 +476,19 @@ pub const ImportWatcher = union(enum) {
dir_fd: StoredFileDescriptorType,
package_json: ?*PackageJSON,
comptime copy_file_path: bool,
) !void {
switch (this) {
inline .hot, .watch => |wacher| try wacher.addFile(fd, file_path, hash, loader, dir_fd, package_json, copy_file_path),
else => {},
}
) bun.JSC.Maybe(void) {
return switch (this) {
inline .hot, .watch => |watcher| watcher.addFile(
fd,
file_path,
hash,
loader,
dir_fd,
package_json,
copy_file_path,
),
.none => .{ .result = {} },
};
}
};

Expand Down Expand Up @@ -3552,7 +3560,7 @@ pub fn NewHotReloader(comptime Ctx: type, comptime EventLoopType: type, comptime
// - Directories outside the root directory
// - Directories inside node_modules
if (std.mem.indexOf(u8, file_path, "node_modules") == null and std.mem.indexOf(u8, file_path, watch.fs.top_level_dir) != null) {
watch.addDirectory(dir_fd, file_path, GenericWatcher.getHash(file_path), false) catch {};
_ = watch.addDirectory(dir_fd, file_path, GenericWatcher.getHash(file_path), false);
}
}

Expand All @@ -3566,9 +3574,9 @@ pub fn NewHotReloader(comptime Ctx: type, comptime EventLoopType: type, comptime

pub fn onError(
_: *@This(),
err: anyerror,
err: bun.sys.Error,
) void {
Output.prettyErrorln("<r>Watcher crashed: <red><b>{s}<r>", .{@errorName(err)});
Output.err(@as(bun.C.E, @enumFromInt(err.errno)), "Watcher crashed", .{});
}

pub fn getContext(this: *@This()) *@This().Watcher {
Expand Down
Loading

0 comments on commit 006575a

Please sign in to comment.