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

refactor (bindings/zig): Improvements #5247

Merged
merged 1 commit into from
Nov 6, 2024
Merged

Conversation

kassane
Copy link
Contributor

@kassane kassane commented Oct 26, 2024

Which issue does this PR close?

N/D

Rationale for this change

It tends to be usable and testable.

What changes are included in this PR?

  • Operator wrapper added
  • more unittests
  • wip: add stackful-async (zigcoro library) support
  • replace @cImport/@cInclude to translate-c only (see: zig#20875)
  • add custom test-runner output (more info)

Note

zig build [run|test] --summary [all|new|failures|none] show build-runner tree-output, not test output.

Are there any user-facing changes?

testable and easy to use.

@kassane
Copy link
Contributor Author

kassane commented Oct 28, 2024

Why??

================================================================================
    "operator advanced operations" - Unsupported
================================================================================
    /home/kassane/opendal/bindings/zig/src/opendal.zig:269:5: 0x103d9a0 in codeToError (test)
        return switch (code) {
        ^
    /home/kassane/opendal/bindings/zig/src/opendal.zig:113:13: 0x103f725 in copy (test)
                try codeToError(err.*.code);
                ^
    /home/kassane/opendal/bindings/zig/src/opendal.zig:385:5: 0x1040031 in test.operator advanced operations (test)
        try op.copy("/testdir/renamed.txt", "/testdir/copied.txt");
        ^

// Test rename operation
// try op.rename("/testdir/file.txt", "/testdir/renamed.txt");
// try testing.expect(!try op.exists("/testdir/file.txt"));
// try testing.expect(try op.exists("/testdir/renamed.txt"));
// Test copy operation
// try op.copy("/testdir/renamed.txt", "/testdir/copied.txt");
// try testing.expect(try op.exists("/testdir/renamed.txt"));
// try testing.expect(try op.exists("/testdir/copied.txt"));

@kassane
Copy link
Contributor Author

kassane commented Oct 28, 2024

@Xuanwo,

async unittest get error:

GDB output
Warning: 'set target-async', an alias for the command 'set mi-async', is deprecated.
Use 'set mi-async'.
No line 468 in file "/home/kassane/opendal/bindings/zig/src/opendal.zig".
Running executable

This GDB supports auto-downloading debuginfo from the following URLs:
  <https://debuginfod.archlinux.org>
Enable debuginfod for this session? (y or [n]) [answered N; input not from terminal]
Debuginfod has been disabled.
To make this setting permanent, add 'set debuginfod enabled off' to .gdbinit.
[Thread debugging using libthread_db enabled]
Using host libthread_db library "/usr/lib/libthread_db.so.1".
undefined
Error Tests (0.07ms)
Semantic Analyzer (0.01ms)
operator basic operations (6.53ms)
operator advanced operations (1.36ms)


Breakpoint 2, opendal.writeData () at opendal.zig:448
448	fn writeData(op: *Operator, path: []const u8, data: []const u8) anyerror!void {

Program
 received signal SIGSEGV, Segmentation fault.
0x00007ffff7500837 in core::str::validations::next_code_point<core::slice::iter::Iter<u8>> (bytes=0xaaaaaaaaaaaaaaaa) at /rustc/f6e511eec7342f59a25f7c0534f1dbea00d01b14/library/core/src/str/validations.rs:35
warning: 35	/rustc/f6e511eec7342f59a25f7c0534f1dbea00d01b14/library/core/src/str/validations.rs: File or directory does not exist

Tested

  • zig-version: 0.13.0
  • rustc-version: 1.82.0 (f6e511eec 2024-10-15)

Edit

May be blocked by:

@kassane kassane force-pushed the zig-async branch 2 times, most recently from 3119fc7 to 2d88433 Compare October 28, 2024 17:40
@kassane kassane marked this pull request as ready for review October 28, 2024 17:41
@Xuanwo
Copy link
Member

Xuanwo commented Oct 28, 2024

Thank you @kassane for you effort, I will review this PR later this week.

* Operator wrapper added
* more unittests
* add async (library) support
* replace `@cImport/@cInclude` to `translate-c` only
* add custom testrunner output
* clean `build.zig`

Signed-off-by: Matheus C. França  <[email protected]>
@kassane
Copy link
Contributor Author

kassane commented Oct 28, 2024

Thank you @kassane for you effort, I will review this PR later this week.

Nice!
Now, latest commit clean build.zig - opendal_module provide all dependencies.

Copy link
Member

@Xuanwo Xuanwo left a comment

Choose a reason for hiding this comment

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

Thank you @kassane for this great work!

@Xuanwo Xuanwo merged commit a594365 into apache:main Nov 6, 2024
31 checks passed
@kassane kassane deleted the zig-async branch November 6, 2024 11:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants