Skip to content

Using goada-wasm if CGO_ENABLE=0#511

Merged
NGTmeaty merged 7 commits into
mainfrom
goadawasm
Dec 9, 2025
Merged

Using goada-wasm if CGO_ENABLE=0#511
NGTmeaty merged 7 commits into
mainfrom
goadawasm

Conversation

@yzqzss

@yzqzss yzqzss commented Oct 28, 2025

Copy link
Copy Markdown
Collaborator

No description provided.

@codecov-commenter

codecov-commenter commented Oct 28, 2025

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 56.55%. Comparing base (66d8660) to head (357cdde).
⚠️ Report is 37 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main     #511      +/-   ##
==========================================
+ Coverage   56.42%   56.55%   +0.13%     
==========================================
  Files         131      131              
  Lines        6570     6572       +2     
==========================================
+ Hits         3707     3717      +10     
+ Misses       2488     2480       -8     
  Partials      375      375              
Flag Coverage Δ
e2etests 42.11% <100.00%> (+0.13%) ⬆️
unittests 28.83% <100.00%> (+0.02%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@yzqzss

yzqzss commented Oct 28, 2025

Copy link
Copy Markdown
Collaborator Author

golang/go#44695 (comment)

CI fails because CGO_ENABLE="1" but we no longer use cgo.

Update: Hmm, completely removing the cgo goada wasn't a good idea, since wasm has a much greater performance penalty than cgo.
So I changed this PR so that when CGO_ENABLE=1 the build uses goada, and when CGO_ENABLE=0 the build uses my goada-wasm :) .

@yzqzss yzqzss closed this Oct 28, 2025
@yzqzss yzqzss reopened this Oct 28, 2025
@yzqzss yzqzss changed the title Using wasm to avoid CGO for ada Using goada-wasm if CGO_ENABLE=0 Oct 28, 2025
@yzqzss yzqzss requested a review from Copilot October 28, 2025 15:54

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull Request Overview

This PR enables Zeno to build and run without CGO by introducing a WebAssembly-based URL parser (goada-wasm) as an alternative to the CGO-dependent goada library. The change makes CGO optional rather than required, allowing the project to compile on systems without C++ compiler toolchains at the cost of reduced performance on non-amd64/arm64 architectures.

Key Changes:

  • Added CGO-free URL parsing implementation using goada-wasm package
  • Updated build process to test both CGO-enabled and CGO-disabled configurations
  • Revised documentation to reflect that CGO is now optional

Reviewed Changes

Copilot reviewed 7 out of 8 changed files in this pull request and generated 2 comments.

Show a summary per file
File Description
internal/pkg/preprocessor/url_cgofree.go New CGO-free implementation of URL normalization using goada-wasm
internal/pkg/preprocessor/url_cgo.go Added build tag and Backend() function to existing CGO implementation
go.mod Added goada-wasm v1.0.1 dependency
cmd/cmd.go Added URL parser backend information to CLI help output
README.md Updated requirements section to document CGO as optional with performance tradeoffs
.github/workflows/go.yml Enhanced CI to test both CGO-enabled and CGO-disabled builds
.github/copilot-instructions.md Updated internal documentation to reflect optional CGO dependency

Comment thread README.md Outdated
Comment thread .github/copilot-instructions.md Outdated

@NGTmeaty NGTmeaty left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

So sorry for the delay here, been super busy! This is great! Thank you!

@NGTmeaty NGTmeaty merged commit acd2911 into main Dec 9, 2025
6 of 7 checks passed
@NGTmeaty NGTmeaty deleted the goadawasm branch December 9, 2025 01:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants