Skip to content

Upate golang.org/x/sys version to fix bug on darwin with go 1.18 #520

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 1 commit into from
Apr 2, 2022
Merged

Upate golang.org/x/sys version to fix bug on darwin with go 1.18 #520

merged 1 commit into from
Apr 2, 2022

Conversation

@juanmaia
Copy link
Contributor

@juanmaia juanmaia commented Mar 19, 2022

When using gdamore/tcell/v2 in the latest release of Go 1.18 we get the following error:

# golang.org/x/sys/unix
../../../go/pkg/mod/golang.org/x/[email protected]/unix/syscall_darwin.1_13.go:29:3: //go:linkname must refer to declared function or variable
../../../go/pkg/mod/golang.org/x/[email protected]/unix/zsyscall_darwin_amd64.1_13.go:27:3: //go:linkname must refer to declared function or variable
../../../go/pkg/mod/golang.org/x/[email protected]/unix/zsyscall_darwin_amd64.1_13.go:40:3: //go:linkname must refer to declared function or variable
../../../go/pkg/mod/golang.org/x/[email protected]/unix/zsyscall_darwin_amd64.go:28:3: //go:linkname must refer to declared function or variable
../../../go/pkg/mod/golang.org/x/[email protected]/unix/zsyscall_darwin_amd64.go:43:3: //go:linkname must refer to declared function or variable
../../../go/pkg/mod/golang.org/x/[email protected]/unix/zsyscall_darwin_amd64.go:59:3: //go:linkname must refer to declared function or variable
../../../go/pkg/mod/golang.org/x/[email protected]/unix/zsyscall_darwin_amd64.go:75:3: //go:linkname must refer to declared function or variable
../../../go/pkg/mod/golang.org/x/[email protected]/unix/zsyscall_darwin_amd64.go:90:3: //go:linkname must refer to declared function or variable
../../../go/pkg/mod/golang.org/x/[email protected]/unix/zsyscall_darwin_amd64.go:105:3: //go:linkname must refer to declared function or variable
../../../go/pkg/mod/golang.org/x/[email protected]/unix/zsyscall_darwin_amd64.go:121:3: //go:linkname must refer to declared function or variable
../../../go/pkg/mod/golang.org/x/[email protected]/unix/zsyscall_darwin_amd64.go:121:3: too many errors

This is due to tcell using an old version of golang.org/x/sys. The issue has been reported and fixed already: golang/go#49219

This PR updates to the latest version given by go get -u golang.org/x/sys

@gdamore
Copy link
Owner

gdamore commented Mar 21, 2022

I am seriously conflicted about this.

Why? Because the switch basically brings in go1.17 and makes that the new baseline go version required.

Go 1.18 only just came out. I'd honestly prefer not to force folks to upgrade if not necessary.

I really wish the go people had just made the x/sys/unix package part of the standard runtime -- doing it this way (as modules) means that we have to deal with incompatibilities which are breaking for third parties.

I haven't yet decided -- folks who are using this package with go 1.18 may wish to go ahead and locally make this change ... we might need to make a fork of the package for folks using older Go versions.

(The go people have a 2 supported releases philosophy, which unfortunately makes it really easy for third party software to fall behind (given how often they release.) (Then there are others like gccgo that typically trail the cgo toolchain by a little bit as well.)

For example, I'm currently using go 1.15 in one of the commercial projects I work on, and it would be considered unduly risky to upgrade to a newer version (because that project is using a sustaining release.). This change means that they cannot update to a more modern version of tcell until they are ready to do so. :(

@juanmaia
Copy link
Contributor Author

juanmaia commented Mar 21, 2022

Does this change breaks in go1.17? It seems that the build is using go1.13 and it passes: https://ci.appveyor.com/project/gdamore/tcell/builds/42949542

I opened this pull request trusting on the build and that this change would be backwards compatible. I myself haven't tried it with older versions of go.

If this is the case, where this change will require people to upgrade to go1.18 then I would think carefully.

The state of things right now is:

  • the latest version of go is 1.18
  • the latest version of tcell is v2.4.0
  • these 2 versions are in a incompatible state. If someone download both the latest versions of Go and tcell, it's not possible to use tcell. This means that it would require someone to downgrade their go version.

IMO I would think that it's better to release a new version of tcell with this change, and point people using go <1.18 to use the v2.4.0 tag. Isn't that feasible? Then all versions of go has a compatible working version of tcell.

@gdamore
Copy link
Owner

gdamore commented Mar 21, 2022

Hmmm... this is interesting... I'll run some tests myself. I was just looking at the go.mod file from the dependency, which appears to declare go1.17. If it works to still run on older go releases that would be good news.

@gdamore gdamore merged commit 1f27c5e into gdamore:master Apr 2, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@juanmaia @gdamore