refactor(bindings/zig): enable tests and more (#2375)

* chore: improve zig bindings setup Signed-off-by: tison <[email protected]> * ci: check zig files license header Signed-off-by: tison <[email protected]> * docs: zig contributing guide Signed-off-by: tison <[email protected]> * build: format build.zig Signed-off-by: tison <[email protected]> * docs: update README for Zig binding Signed-off-by: tison <[email protected]> * refactor: code Signed-off-by: tison <[email protected]> * fix: how to get data len Signed-off-by: tison <[email protected]> * ci: check format Signed-off-by: tison <[email protected]> * ci: update target name Signed-off-by: tison <[email protected]> * Update .github/workflows/bindings_zig.yml * trigger CI Signed-off-by: tison <[email protected]> --------- Signed-off-by: tison <[email protected]>
apache · May 31, 2023 · fd5a287 · fd5a287
1 parent 174533f
commit fd5a287
Show file tree

Hide file tree

Showing 7 changed files with 114 additions and 55 deletions.
diff --git a/.github/workflows/bindings_zig.yml b/.github/workflows/bindings_zig.yml
@@ -27,6 +27,8 @@ on:
     branches:
       - main
     paths:
+      - "core/**"
+      - "bindings/c/**"
       - "bindings/zig/**"
       - ".github/workflows/bindings_zig.yml"
   workflow_dispatch:
@@ -43,21 +45,25 @@ jobs:
     runs-on: ubuntu-latest
     steps:
       - uses: actions/checkout@v3
-      - uses: goto-bus-stop/setup-zig@v2
+      - uses: korandoru/setup-zig@v1
         with:
-          version: master # 0.11.0 (nightly/master)
-          #version: 0.11.0 # TODO: replace to stable version - wait new zig release
+          zig-version: master # 0.11.0 (nightly/master)
+          #zig-version: 0.11.0 # TODO: replace to stable version - wait new zig release
 
       - name: Setup Rust toolchain
         uses: ./.github/actions/setup
 
       - name: Build Zig binding
-        working-directory: "bindings/zig"
-        run: zig build opendal_c
+        working-directory: bindings/zig
+        run: zig build libopendal_c
 
       - name: Check diff
         run: git diff --exit-code
 
-      - name: Build and Run BDD tests
-        working-directory: "bindings/zig"
+      - name: Check
+        working-directory: bindings/zig
+        run: zig fmt --check .
+
+      - name: Run tests
+        working-directory: bindings/zig
         run: zig build test -fsummary
diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml
@@ -45,7 +45,7 @@ jobs:
         uses: ./.github/actions/setup
 
       - name: Check license headers
-        uses: korandoru/hawkeye@v2.0.0
+        uses: korandoru/hawkeye@v2.1.0
 
       - name: Cargo format
         run: cargo fmt --all -- --check

diff --git a/bindings/zig/.gitignore b/bindings/zig/.gitignore
@@ -1,2 +1,2 @@
 zig-cache/
-zig-out/
+zig-out/
diff --git a/bindings/zig/CONTRIBUTING.md b/bindings/zig/CONTRIBUTING.md
@@ -1,4 +1,5 @@
 # Contributing
+
 - [Contributing](#contributing)
    - [Setup](#setup)
       - [Using a dev container environment](#using-a-dev-container-environment)
@@ -11,35 +12,40 @@
 ## Setup
 
 ### Using a dev container environment
+
 OpenDAL provides a pre-configured [dev container](https://containers.dev/) that could be used in [Github Codespaces](https://github.com/features/codespaces), [VSCode](https://code.visualstudio.com/), [JetBrains](https://www.jetbrains.com/remote-development/gateway/), [JuptyerLab](https://jupyterlab.readthedocs.io/en/stable/). Please pick up your favourite runtime environment.
 
 The fastest way is:
 
 [![Open in GitHub Codespaces](https://github.com/codespaces/badge.svg)](https://codespaces.new/apache/incubator-opendal?quickstart=1&machine=standardLinux32gb)
 
 ### Bring your own toolbox
-To build OpenDAL Zig binding, the following is all you need:
-- **Zig toolchain** is based on **LLVM-toolchain**(clang/++, libcxx, lld).
 
-## Need
-- opendal C binding
+To build OpenDAL Zig binding locally, you need:
+
+- [Zig](https://ziglang.org/download) 0.11.0 or higher
+
+> **Note**:
+>
+> 0.11.0 is not released yet. You can use master instead before the official 0.11.0 released.
 
 ## Build
-To build the library and header file.
-```shell
-zig build opendal_c
-```
 
-- zig build need add header file `opendal.h` is under `../c/include` 
-- The library is under `../../target/debug` or `../../target/release` after building.
+First, build the C bindings:
 
-To clean the build results.
 ```shell
-zig build uninstall
+zig build libopendal_c
 ```
 
+> **Note**:
+>
+> - `zig build` adds the header file `opendal.h` under `../c/include`
+> - The library is under `../../target/debug` or `../../target/release` after building.
+
 ## Test
+
 To build and run the tests.
+
 ```shell
 zig build test -fsummary
 ```

diff --git a/bindings/zig/README.md b/bindings/zig/README.md
@@ -1,14 +1,19 @@
 # OpenDAL Zig Binding (WIP)
 
-# Build Zig bindings
+# Build
 
-To compile OpenDAL from source code, you'll need:
-- [Zig 0.11.0 or higher](https://ziglang.org/download), totally portable.
+To compile OpenDAL from source code, you need:
+
+- [Zig](https://ziglang.org/download) 0.11.0 or higher
+
+> **Note**:
+>
+> 0.11.0 is not released yet. You can use master instead before the official 0.11.0 released.
 
 ```bash
-# build opendal_c library (call make -C ../c)
-zig build opendal_c
-# Build and run test
+# build libopendal_c (underneath call make -C ../c)
+zig build libopendal_c
+# build and run unit tests
 zig build test -fsummary
 ```
 

diff --git a/bindings/zig/build.zig b/bindings/zig/build.zig
@@ -1,3 +1,20 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements.  See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership.  The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License.  You may obtain a copy of the License at
+//
+//   http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied.  See the License for the
+// specific language governing permissions and limitations
+// under the License.
+
 const std = @import("std");
 
 pub fn build(b: *std.Build) void {
@@ -12,6 +29,12 @@ pub fn build(b: *std.Build) void {
         },
         .dependencies = &.{},
     });
+
+    // Creates a step for building the dependent C bindings
+    const libopendal_c = buildLibOpenDAL(b);
+    const build_libopendal_c = b.step("libopendal_c", "Build OpenDAL C bindings");
+    build_libopendal_c.dependOn(&libopendal_c.step);
+
     // Creates a step for unit testing. This only builds the test executable
     // but does not run it.
     const unit_tests = b.addTest(.{
@@ -22,27 +45,27 @@ pub fn build(b: *std.Build) void {
         .optimize = optimize,
     });
     unit_tests.addIncludePath("../c/include");
-    if (optimize == .Debug)
-        unit_tests.addLibraryPath("../../target/debug")
-    else
+    if (optimize == .Debug) {
+        unit_tests.addLibraryPath("../../target/debug");
+    } else {
         unit_tests.addLibraryPath("../../target/release");
+    }
     unit_tests.linkSystemLibrary("opendal_c");
     unit_tests.linkLibC();
 
-    const opendal_c = buildOpendalC(b);
-    const make_opendal_c = b.step("opendal_c", "Build opendal_c library");
-    make_opendal_c.dependOn(&opendal_c.step);
+    // Creates a step for running unit tests.
     const run_unit_tests = b.addRunArtifact(unit_tests);
-    const test_step = b.step("test", "Run opendal tests");
+    const test_step = b.step("test", "Run OpenDAL Zig bindings tests");
     test_step.dependOn(&run_unit_tests.step);
 }
-fn buildOpendalC(b: *std.Build) *std.Build.Step.Run {
-    const rootdir = (comptime std.fs.path.dirname(@src().file) orelse null) ++ "/";
-    const opendalCdir = rootdir ++ "../c";
+
+fn buildLibOpenDAL(b: *std.Build) *std.Build.Step.Run {
+    const basedir = comptime std.fs.path.dirname(@src().file) orelse null;
+    const c_bindings_dir = basedir ++ "/../c";
     return b.addSystemCommand(&[_][]const u8{
         "make",
         "-C",
-        opendalCdir,
+        c_bindings_dir,
         "build",
     });
 }
diff --git a/bindings/zig/src/opendal.zig b/bindings/zig/src/opendal.zig
@@ -1,13 +1,29 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements.  See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership.  The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License.  You may obtain a copy of the License at
+//
+//   http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied.  See the License for the
+// specific language governing permissions and limitations
+// under the License.
+
 pub const opendal = @cImport(@cInclude("opendal.h"));
 
 const std = @import("std");
 const testing = std.testing;
 
 test "Opendal BDD test" {
-    const c_str = [*:0]const u8; // const char*
-    // const zig_str = []const u8;
+    const c_str = [*:0]const u8; // define a type for 'const char*' in C
 
-    const OpendalBddTest = struct {
+    const OpendalBDDTest = struct {
         p: opendal.opendal_operator_ptr,
         scheme: c_str,
         path: c_str,
@@ -20,39 +36,42 @@ test "Opendal BDD test" {
             self.content = "Hello, World!";
 
             var options: opendal.opendal_operator_options = opendal.opendal_operator_options_new();
+            defer opendal.opendal_operator_options_free(&options);
             opendal.opendal_operator_options_set(&options, "root", "/myroot");
 
             // Given A new OpenDAL Blocking Operator
             self.p = opendal.opendal_operator_new(self.scheme, options);
-            defer opendal.opendal_operator_options_free(&options);
-            // try testing.expect(self.p != null);
+            std.debug.assert(self.p != null);
+
             return self;
         }
+
         pub fn deinit(self: *Self) void {
             opendal.opendal_operator_free(&self.p);
         }
 
         const Self = @This();
     };
-    var bddTest = OpendalBddTest.init();
-    defer bddTest.deinit();
+
+    var testkit = OpendalBDDTest.init();
+    defer testkit.deinit();
 
     // When Blocking write path "test" with content "Hello, World!"
     const data: opendal.opendal_bytes = .{
-        .data = bddTest.content,
-        // c_str hasn't len field (.* is ptr)
-        .len = std.mem.len(bddTest.content),
+        .data = testkit.content,
+        // c_str does not have len field (.* is ptr)
+        .len = std.mem.len(testkit.content),
     };
-    const code = opendal.opendal_operator_blocking_write(bddTest.p, bddTest.path, data);
+    const code = opendal.opendal_operator_blocking_write(testkit.p, testkit.path, data);
     try testing.expectEqual(code, opendal.OPENDAL_OK);
 
     // The blocking file "test" should exist
-    var e: opendal.opendal_result_is_exist = opendal.opendal_operator_is_exist(bddTest.p, bddTest.path);
+    var e: opendal.opendal_result_is_exist = opendal.opendal_operator_is_exist(testkit.p, testkit.path);
     try testing.expectEqual(e.code, opendal.OPENDAL_OK);
     try testing.expect(e.is_exist);
 
     // The blocking file "test" entry mode must be file
-    var s: opendal.opendal_result_stat = opendal.opendal_operator_stat(bddTest.p, bddTest.path);
+    var s: opendal.opendal_result_stat = opendal.opendal_operator_stat(testkit.p, testkit.path);
     try testing.expectEqual(s.code, opendal.OPENDAL_OK);
     var meta: opendal.opendal_metadata = s.meta;
     try testing.expect(opendal.opendal_metadata_is_file(&meta));
@@ -62,13 +81,13 @@ test "Opendal BDD test" {
     defer opendal.opendal_metadata_free(&meta);
 
     // The blocking file "test" must have content "Hello, World!"
-    var r: opendal.opendal_result_read = opendal.opendal_operator_blocking_read(bddTest.p, bddTest.path);
+    var r: opendal.opendal_result_read = opendal.opendal_operator_blocking_read(testkit.p, testkit.path);
     defer opendal.opendal_bytes_free(r.data);
     try testing.expect(r.code == opendal.OPENDAL_OK);
-    try testing.expectEqual(std.mem.len(r.data.*.data), std.mem.len(bddTest.content));
+    try testing.expectEqual(std.mem.len(testkit.content), r.data.*.len);
 
     var count: usize = 0;
-    while (count < std.mem.len(r.data.*.data)) : (count += 1) {
-        try testing.expectEqual(bddTest.content[count], r.data.*.data[count]);
+    while (count < r.data.*.len) : (count += 1) {
+        try testing.expectEqual(testkit.content[count], r.data.*.data[count]);
     }
 }