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

fix: add shims for GOARCH=wasm with GOOS=js and GOOS=wasip1 #176

Merged
merged 1 commit into from
Mar 11, 2024

Conversation

paralin
Copy link
Contributor

@paralin paralin commented Feb 20, 2024

Fixes build errors:

GOOS=js GOARCH=wasm go build
GOOS=wasip1 GOARCH=wasm go build

@cmaglie
Copy link
Member

cmaglie commented Feb 26, 2024

@paralin what's the purpose of this patch? since nativeOpen is not implemented the library will not be available on wasm anyway.

@paralin
Copy link
Contributor Author

paralin commented Feb 26, 2024

When the library is used as a dependency it is best to have it compile cleanly when that GOOS is used with stubs that return errors instead of failing to build entirely. This is for a dependency of https://github.com/aperturerobotics/bifrost

@cmaglie
Copy link
Member

cmaglie commented Feb 29, 2024

@paralin I'd like to ask you these changes:

  1. Remove the files with the shims for js and wasip1.
    Since both OS/ARCH pairs wasm/js and wasm/wasip1 belongs to the same ARCH wasm, it should be possible to reduce the number of files by simply leveraging the selection on the OS //go:build wasm.

  2. Remove the go:build flags.
    I think we could avoid the go:build tags altogether by simply naming the source file with the suffix *_wasm.go.

If a file's name, after stripping the extension and a possible _test suffix,
matches any of the following patterns:

*_GOOS
*_GOARCH
*_GOOS_GOARCH
(example: source_windows_amd64.go) where GOOS and GOARCH represent any known
operating system and architecture values respectively, then the file is considered to have
an implicit build constraint requiring those terms (in addition to any explicit constraints
in the file).
  1. Please add the license header to all files.

Fixes build errors:

GOOS=js GOARCH=wasm go build
GOOS=wasip1 GOARCH=wasm go build

Signed-off-by: Christian Stewart <[email protected]>
@paralin
Copy link
Contributor Author

paralin commented Mar 1, 2024

You were right on the build tags, I fixed the PR according to your request.

@cmaglie cmaglie merged commit 0925f99 into bugst:master Mar 11, 2024
8 checks passed
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.

2 participants