google / starlark-go

Starlark in Go: the Starlark configuration language, implemented in Go
BSD 3-Clause "New" or "Revised" License
2.31k stars 210 forks source link

Build failure on Go 1.16/1.17 for lib/json/json.go:238:11: v.UnsafePointer #473

Closed hyorigo closed 1 year ago

hyorigo commented 1 year ago

Here's the error message:

../../../go/pkg/mod/go.starlark.net@v0.0.0-20230525235612-a134d8f9ddca/lib/json/json.go:238:11: v.UnsafePointer undefined (type reflect.Value has no field or method UnsafePointer)

As per the Go.mod file, Go 1.16 is still in support.

This build break was introduced in: https://github.com/google/starlark-go/pull/431

https://pkg.go.dev/reflect#Value.UnsafePointer

So please fix this build break, and maybe add Go 1.16 and 1.17 in GitHub Actions CI?

adonovan commented 1 year ago

Thanks for reporting this. You're right, we should have CI builds for all supported versions. That said, I'm not sure we should continue to support 1.16 and 1.17, since there have been three Go releases since then. I propose that we instead drop 1.16 and 1.17 but document that CI should cover all supported releases.

Any objections?

hyorigo commented 1 year ago

Go 1.16 is four releases behind, but it's released only 2 years ago. It's still used in many prod environments.

Since Starlark in Go is a basic library, dropping the support for Go 1.16 will force the app in 1.16 to upgrade or locked in the older version of Starlark.

yo8 commented 1 year ago

@adonovan Please don't drop 1.16 or 1.17. Since Go 1.16 is latest version that ChatGPT and GPT-4 can understand 🥇 My company still use 1.16 in production and some sub-system use Starlark as config language as well.

adonovan commented 1 year ago

If we keep 1.16 until 1.21 is released, and then declare continued support the four most recent numbered Go releases, will that suffice?

hyorigo commented 1 year ago

@adonovan Sounds good. But will this bug be fixed before Aug?

hyorigo commented 1 year ago

It breaks in Go 1.17 as well 😿