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(templates/http-go): Remove scheduler flag from http-go build command #2981

Merged
merged 1 commit into from
Jan 21, 2025

Conversation

adamreese
Copy link
Contributor

This removes the -scheduler=none flag from the build command in the http-go template. Including this flag prevents Spin applications using http.Transport from compiling.

❯ spin build
Building component http-roundtrip-test with `tinygo build -target=wasip1 -gc=leaking -scheduler=none -buildmode=c-shared -no-debug -o main.wasm .`
/opt/homebrew/Cellar/tinygo/0.35.0/src/net/http/transfer.go:213:2: attempted to start a goroutine without a scheduler
Error: Build command for component http-roundtrip-test failed with status Exited(1)

@itowlson
Copy link
Collaborator

@deadprogram I blindly copied -scheduler=none from your sample build command in #2184 (comment) - would you have any concerns with us removing it?

@deadprogram
Copy link
Contributor

would you have any concerns with us removing it?

Not at all! Please remove for compatibility.

I am not using any Go routines in my specific use cases, so I was able to remove even more size from the module that way.

This removes the `-scheduler=none` flag from the build command in the
http-go template. Including this flag prevents Spin applications using
`http.Transport` from compiling.

```
❯ spin build
Building component http-roundtrip-test with `tinygo build -target=wasip1 -gc=leaking -scheduler=none -buildmode=c-shared -no-debug -o main.wasm .`
/opt/homebrew/Cellar/tinygo/0.35.0/src/net/http/transfer.go:213:2: attempted to start a goroutine without a scheduler
Error: Build command for component http-roundtrip-test failed with status Exited(1)
```

Signed-off-by: Adam Reese <adam@reese.io>
Copy link
Collaborator

@itowlson itowlson left a comment

Choose a reason for hiding this comment

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

Man, thanks for finding all this stuff. Sorry for leaving it in this state for you.

@adamreese
Copy link
Contributor Author

It took some artisanal greping to find them all

@itowlson
Copy link
Collaborator

@fermyon/administrators Please force merge. I believe the JS CI failures are unrelated, the double-echo is a known flake, and the Go one is because it's using the templates from main, which this PR fixes. Thanks!

@tschneidereit tschneidereit merged commit c4aa295 into spinframework:main Jan 21, 2025
16 of 17 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.

4 participants