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

Update submodule to latest master in microsoft/main #1227

Merged

Conversation

microsoft-golang-bot
Copy link
Collaborator

Hi! I'm a bot, and this is an automatically generated upstream sync PR. 🔃

After submitting the PR, I will attempt to enable auto-merge in the "merge commit" configuration.

For more information, visit sync documentation in microsoft/go-infra.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Thanks! Auto-approving.

Copy link
Member

@karianna karianna left a comment

Choose a reason for hiding this comment

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

Failing CI

@dagood
Copy link
Member

dagood commented May 21, 2024

These errors are caused by upstream locking down linkname to reduce the fragility of libraries when standard library internals change: golang/go#67401, specifically golang/go@003683f:

---- Running command: [cmd.exe /c make.bat]
Building Go toolchain2 using go_bootstrap and Go toolchain1.
# cmd/preprofile
link: crypto/internal/backend: invalid reference to crypto/ecdsa.encodeSignature
# cmd/asm
link: crypto/internal/backend: invalid reference to crypto/ecdsa.encodeSignature
# cmd/cgo
link: crypto/internal/backend: invalid reference to crypto/ecdsa.encodeSignature
# cmd/link
link: crypto/internal/backend: invalid reference to crypto/ecdsa.encodeSignature
# cmd/compile
link: crypto/internal/backend: invalid reference to crypto/ecdsa.encodeSignature

There's a compatibility set of linknames that are still allowed for compat (defined by golang/go@41aab30) but they didn't include encodeSignature so we have to add it ourselves. (IMO this is perfectly reasonable: we're a fork, not a library, so we're able to make this change easily, and the upstream restriction lets the wider Go ecosystem be more resilient going forward.)

These errors show up one by one, so I also had to add parseSignature after fixing the first one and seeing that show up.

/cc @qmuntal

@dagood dagood dismissed karianna’s stale review May 21, 2024 21:50

Tests no longer failing

@microsoft-golang-review-bot microsoft-golang-review-bot merged commit 240a8b8 into microsoft/main May 21, 2024
23 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.

None yet

4 participants