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

Revert tinygo reflect.SliceHeader specialization #2210

Merged
merged 3 commits into from
Jun 18, 2024

Conversation

ncruces
Copy link
Collaborator

@ncruces ncruces commented May 20, 2024

In light of tinygo-org/tinygo#4156, we need to revert #2161.

Also use unsafe.Add where feasible, and remove TODOs to use unsafe.Slice where doing so would raise a go vet warning about misuse of unsafe.Pointer.

@ncruces
Copy link
Collaborator Author

ncruces commented May 20, 2024

Drafting while waiting for the TinyGo release, to update failed integration tests.

@deadprogram
Copy link
Contributor

We are in process of releasing v0.32 so this PR should be prepared, please.

@ncruces
Copy link
Collaborator Author

ncruces commented Jun 17, 2024

Thanks for the heads up.

I think this is mostly ready, I just need to update CI to download the v0.32 binaries, and run tests.

Signed-off-by: Nuno Cruces <[email protected]>
@ncruces
Copy link
Collaborator Author

ncruces commented Jun 18, 2024

Linux binaries still missing from here, will try again.

@ncruces
Copy link
Collaborator Author

ncruces commented Jun 18, 2024

@deadprogram it seems TinyGo v0.32 lost the ability to build --target=wasi binaries with Go 1.20?

Signed-off-by: Nuno Cruces <[email protected]>
@@ -84,6 +84,7 @@ jobs:

- name: Build TinyGo examples
run: make build.examples.tinygo
if: matrix.go-version != '1.20' # fails with TinyGo v0.32.0
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Idk how we feel about skipping building tests with Go 1.20 + TinyGo v0.32.0, as that seems to be an unsupported combination.

@ncruces ncruces marked this pull request as ready for review June 18, 2024 11:40
Copy link
Member

@mathetake mathetake left a comment

Choose a reason for hiding this comment

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

yeah looks like a bug in TinyGo that -target=wasi is now trying to use GOOS=wasip1, which is not existent in Go 1.20, for source resolution vs previously it didn't use wasip1 target internally (I remember it used arm target virtually). I think anyway this is harmless so I will go ahead and merge. cc @deadprogram my guess is that the latest TinyGo breaks wasi builds for all Go versions <= 1.20.

@mathetake mathetake merged commit f902fb4 into tetratelabs:main Jun 18, 2024
60 checks passed
@ncruces ncruces deleted the tinygo branch August 13, 2024 21:33
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.

3 participants