Skip to content
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
61 changes: 61 additions & 0 deletions .github/workflows/ci.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,61 @@
name: CI
on:
push:
branches: [main]
pull_request:
branches: [main]

env:
ZIG_VERSION: 0.14.1

jobs:
build:
strategy:
matrix:
include:
# macOS builds
- os: macos-latest
target: aarch64-macos-none
- os: macos-latest
target: x86_64-macos-none
# Linux builds
- os: ubuntu-latest
target: aarch64-linux-gnu
- os: ubuntu-latest
target: x86_64-linux-gnu

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.

Could we try to add a windows target to test

fail-fast: false
runs-on: ${{matrix.os}}
steps:
- uses: actions/checkout@v4

- name: Setup Zig
id: setup-zig
uses: mlugg/setup-zig@v2
with:
version: ${{ env.ZIG_VERSION }}

- name: Cache Zig installation
uses: actions/cache@v3
with:
path: ${{ steps.setup-zig.outputs.install-path }}
key: ${{ runner.os }}-zig-${{ env.ZIG_VERSION }}

- name: Cache Zig build artifacts
uses: actions/cache@v3
with:
path: |
zig-cache
~/.cache/zig
key: ${{ runner.os }}-${{ matrix.target }}-zig-build-${{ hashFiles('build.zig', 'build.zig.zon') }}
restore-keys: |
${{ runner.os }}-${{ matrix.target }}-zig-build-
Comment on lines +37 to +51

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.

Suggested change
- name: Cache Zig installation
uses: actions/cache@v3
with:
path: ${{ steps.setup-zig.outputs.install-path }}
key: ${{ runner.os }}-zig-${{ env.ZIG_VERSION }}
- name: Cache Zig build artifacts
uses: actions/cache@v3
with:
path: |
zig-cache
~/.cache/zig
key: ${{ runner.os }}-${{ matrix.target }}-zig-build-${{ hashFiles('build.zig', 'build.zig.zon') }}
restore-keys: |
${{ runner.os }}-${{ matrix.target }}-zig-build-

I think for such a small build, we don't really need to cache these.


- name: Formatting
run: zig fmt --check .

# TODO: uncomment when tests are implemented
# - name: Unit testing
# run: zig build test -Dtarget=${{matrix.target}} --summary all
Comment on lines +56 to +58

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.

Suggested change
# TODO: uncomment when tests are implemented
# - name: Unit testing
# run: zig build test -Dtarget=${{matrix.target}} --summary all

I think we can do without these if it's going to be commented out anyway.


- name: Building
run: zig build -Dtarget=${{matrix.target}} -Doptimize=ReleaseFast --summary all

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.

Suggested change
run: zig build -Dtarget=${{matrix.target}} -Doptimize=ReleaseFast --summary all
run: zig build -Dtarget=${{matrix.target}} -Doptimize=ReleaseSafe --summary all

Let's try to default to ReleaseSafe rather than ReleaseFast

7 changes: 6 additions & 1 deletion build.zig
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,10 @@ pub fn build(b: *std.Build) void {
.target = target,
.optimize = optimize,
});
const zlib = b.dependency("zlib", .{
.target = target,
.optimize = optimize,
});

const ssl = boringssl.artifact("ssl");
const crypto = boringssl.artifact("crypto");
Expand All @@ -41,6 +45,8 @@ pub fn build(b: *std.Build) void {

lib.linkLibrary(ssl);
lib.linkLibrary(crypto);
lib.linkLibrary(zlib.artifact("z"));
lib.addIncludePath(zlib.path("src"));

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

critical

The include path for zlib appears to be incorrect. The specified zlib dependency (allyourcodebase/zlib) has its header files (zlib.h, zconf.h) at the root of the project, not within a src/ directory. This will likely cause the build to fail when it cannot locate the headers.

To fix this, you should point to the root of the dependency.

lib.addIncludePath(zlib.path(""));

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

I am not sure about this, since if there's no these lines in build.zig, the local container building would fail. @GrapeBaBa @spiral-ladder

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.

you mean lib.addIncludePath(zlib.path("src")); this line?

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.

am not sure about this, since if there's no these lines in build.zig, the local container building would fail

What is the error message?

lib.addIncludePath(lshpack_dep.path("deps/xxhash"));
lib.addIncludePath(lsqpack_dep.path("deps/xxhash"));
lib.addIncludePath(lshpack_dep.path(""));
Expand Down Expand Up @@ -152,5 +158,4 @@ const lsquic_files: []const []const u8 = &.{
"lsquic_varint.c",
"lsquic_version.c",
"lsquic_xxhash.c",
"lsquic_global.c",
};
Loading