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 shim for js #60

Merged
merged 1 commit into from
May 26, 2023
Merged

fix: add shim for js #60

merged 1 commit into from
May 26, 2023

Conversation

paralin
Copy link
Contributor

@paralin paralin commented Jan 4, 2022

Fixes build with:

env GOOS=js GOARCH=wasm go build -trimpath .

@paralin paralin force-pushed the shim-js branch 2 times, most recently from 49fa8c6 to d6ab566 Compare January 4, 2022 23:57
@AkihiroSuda
Copy link
Member

How is this expected to be used?

@paralin
Copy link
Contributor Author

paralin commented Jan 5, 2022

It's a shim so that programs built using the console package can compile against GOOS=js as well.

@dmcgowan
Copy link
Member

dmcgowan commented Jan 7, 2022

@paralin can you add a little bit more context to this issue. We don't currently have any ongoing efforts to support GOOS=js or issues tracking it. Knowing more about the use case would be helpful.

@paralin
Copy link
Contributor Author

paralin commented Jan 7, 2022

@dmcgowan The bubbletea TUI packages import containerd/console. I recently submitted several PRs to projects adding shims so that the build will complete if GOOS=js and GOARCH=wasm with reasonable defaults.

I think it makes sense in general to make the package at least build cleanly with that GOOS. Even if the functions are all shims.

@dmcgowan
Copy link
Member

@paralin if this library is being used for a platform other than what is explicitly supported, our normal pattern is to include a console_other.go file which builds with ! statements of the supported platforms. The current change in console_js.go aren't specific to any platform.

@paralin
Copy link
Contributor Author

paralin commented Jan 11, 2022

@dmcgowan okay, applied the change.

@paralin paralin force-pushed the shim-js branch 2 times, most recently from 0f5f87b to 554c552 Compare January 11, 2022 01:10
console_other.go Outdated
@@ -0,0 +1,35 @@
// +build !darwin,!freebsd,!linux,!netbsd,!openbsd,!solaris
Copy link
Member

Choose a reason for hiding this comment

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

Please gofmt with Go 1.17

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@AkihiroSuda If I do that, the license checker will fail. I previously sent the PR with it formatted that way, and the test failed. The containerd license checker expects this format and doesn't allow the other one.

Copy link
Contributor Author

@paralin paralin Jan 16, 2022

Choose a reason for hiding this comment

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

    - name: Check file headers
      run: ../project/script/validate/fileheader ../project/
      working-directory: src/github.com/containerd/console

@thaJeztah
Copy link
Member

I updated the linters and licence-checker in #61, and formatted with go1.17

@paralin
Copy link
Contributor Author

paralin commented May 26, 2023

@thaJeztah @AkihiroSuda Fixed & rebased, apologies for the delay!

Adds console_other to support not-supported platforms.

Fixes build with:

env GOOS=js GOARCH=wasm go build -trimpath .

Signed-off-by: Christian Stewart <christian@paral.in>
@AkihiroSuda AkihiroSuda merged commit 66136ad into containerd:main May 26, 2023
@paralin paralin deleted the shim-js branch May 26, 2023 09:53
paralin added a commit to paralin/bubbletea that referenced this pull request May 26, 2023
Supports GOOS=js GOARCH=wasm go build

containerd-console now has a shim for js as well:

containerd/console#60

Update that dependency.

Signed-off-by: Christian Stewart <christian@paral.in>
paralin added a commit to paralin/bubbletea that referenced this pull request May 26, 2023
Supports GOOS=js GOARCH=wasm go build

containerd-console now has a shim for js as well:

containerd/console#60

Update that dependency.

Signed-off-by: Christian Stewart <christian@paral.in>
paralin added a commit to paralin/bubbletea that referenced this pull request May 26, 2023
Supports GOOS=js GOARCH=wasm go build

containerd-console now has a shim for js as well:

containerd/console#60

Update that dependency.

Signed-off-by: Christian Stewart <christian@paral.in>
paralin added a commit to paralin/bubbletea that referenced this pull request May 26, 2023
Supports GOOS=js GOARCH=wasm go build

containerd-console now has a shim for js as well:

containerd/console#60

Update that dependency.

Signed-off-by: Christian Stewart <christian@paral.in>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Development

Successfully merging this pull request may close these issues.

4 participants