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

x/crypto/curve25519,crypto/ed25519: avo doesn't generate go:build lines #46155

Closed
FiloSottile opened this issue May 13, 2021 · 4 comments
Closed
Labels
FrozenDueToAge NeedsFix The path to resolution is known, but the work has not been done.
Milestone

Comments

@FiloSottile
Copy link
Contributor

Due to mmcloughlin/avo#183, the autogenerated assembly is lacking //go:build lines.

See #41184.

/cc @tklauser

@gopherbot gopherbot added this to the Unreleased milestone May 13, 2021
@gopherbot
Copy link

Change https://golang.org/cl/319471 mentions this issue: curve25519/internal/field: fix generator module reference to x/crypto

gopherbot pushed a commit to golang/crypto that referenced this issue May 13, 2021
The generator submodule needs a module dependency on golang.org/x/crypto
to find the type information it needs.

This removes the Comment call from CL 319469 because it does not seem to
generate the intended output. See golang/go#46155.

Fixes golang/go#46133

Change-Id: Iec21c6379d81271047ebf370a76329ed3fdac85c
Reviewed-on: https://go-review.googlesource.com/c/crypto/+/319471
Trust: Filippo Valsorda <filippo@golang.org>
Run-TryBot: Filippo Valsorda <filippo@golang.org>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Tobias Klauser <tobias.klauser@gmail.com>
@FiloSottile FiloSottile modified the milestones: Unreleased, Go1.17 May 13, 2021
@FiloSottile FiloSottile added the NeedsFix The path to resolution is known, but the work has not been done. label May 13, 2021
@FiloSottile FiloSottile self-assigned this May 13, 2021
@dmitshur
Copy link
Contributor

dmitshur commented Aug 1, 2021

CL 338629 updates this issue. CC @mmcloughlin, @josharian, @ianlancetaylor.

@mmcloughlin If updating the generator is a change that needs more time, should we fix it inside the Go tree by adding a temporary "//go:generate gofmt" step to run on the avo-generated files? After the generator is updated to emit gofmt'ed Go code, the extra gofmt invocation can go away. (If updating the generator is equally fast and easy, then of course there's no need for this intermediate step.)

@mknyszek mknyszek modified the milestones: Go1.17, Go1.18 Aug 18, 2021
@ianlancetaylor
Copy link
Contributor

According to mmcloughlin/avo#183, this is done for our purposes. Thanks @mmcloughlin .

@gopherbot
Copy link

Change https://go.dev/cl/388874 mentions this issue: curve25519/internal/field: update generator to avo v0.4.0

@dmitshur dmitshur modified the milestones: Go1.18, Go1.19 Mar 4, 2022
gopherbot pushed a commit to golang/crypto that referenced this issue May 17, 2022
This version generates //go:build lines.

For golang/go#46155

Change-Id: I23e4617aa96bc5c15c10f3cd0882028ca08e09e8
Reviewed-on: https://go-review.googlesource.com/c/crypto/+/388874
Reviewed-by: Dmitri Shuralyov <dmitshur@google.com>
Run-TryBot: Tobias Klauser <tobias.klauser@gmail.com>
Auto-Submit: Tobias Klauser <tobias.klauser@gmail.com>
Reviewed-by: Ian Lance Taylor <iant@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Dmitri Shuralyov <dmitshur@golang.org>
maisem pushed a commit to tailscale/golang-x-crypto that referenced this issue Nov 15, 2022
This version generates //go:build lines.

For golang/go#46155

Change-Id: I23e4617aa96bc5c15c10f3cd0882028ca08e09e8
Reviewed-on: https://go-review.googlesource.com/c/crypto/+/388874
Reviewed-by: Dmitri Shuralyov <dmitshur@google.com>
Run-TryBot: Tobias Klauser <tobias.klauser@gmail.com>
Auto-Submit: Tobias Klauser <tobias.klauser@gmail.com>
Reviewed-by: Ian Lance Taylor <iant@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Dmitri Shuralyov <dmitshur@golang.org>
maisem pushed a commit to tailscale/golang-x-crypto that referenced this issue Nov 15, 2022
This version generates //go:build lines.

For golang/go#46155

Change-Id: I23e4617aa96bc5c15c10f3cd0882028ca08e09e8
Reviewed-on: https://go-review.googlesource.com/c/crypto/+/388874
Reviewed-by: Dmitri Shuralyov <dmitshur@google.com>
Run-TryBot: Tobias Klauser <tobias.klauser@gmail.com>
Auto-Submit: Tobias Klauser <tobias.klauser@gmail.com>
Reviewed-by: Ian Lance Taylor <iant@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Dmitri Shuralyov <dmitshur@golang.org>
LewiGoddard pushed a commit to LewiGoddard/crypto that referenced this issue Feb 16, 2023
The generator submodule needs a module dependency on golang.org/x/crypto
to find the type information it needs.

This removes the Comment call from CL 319469 because it does not seem to
generate the intended output. See golang/go#46155.

Fixes golang/go#46133

Change-Id: Iec21c6379d81271047ebf370a76329ed3fdac85c
Reviewed-on: https://go-review.googlesource.com/c/crypto/+/319471
Trust: Filippo Valsorda <filippo@golang.org>
Run-TryBot: Filippo Valsorda <filippo@golang.org>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Tobias Klauser <tobias.klauser@gmail.com>
LewiGoddard pushed a commit to LewiGoddard/crypto that referenced this issue Feb 16, 2023
This version generates //go:build lines.

For golang/go#46155

Change-Id: I23e4617aa96bc5c15c10f3cd0882028ca08e09e8
Reviewed-on: https://go-review.googlesource.com/c/crypto/+/388874
Reviewed-by: Dmitri Shuralyov <dmitshur@google.com>
Run-TryBot: Tobias Klauser <tobias.klauser@gmail.com>
Auto-Submit: Tobias Klauser <tobias.klauser@gmail.com>
Reviewed-by: Ian Lance Taylor <iant@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Dmitri Shuralyov <dmitshur@golang.org>
@golang golang locked and limited conversation to collaborators Jun 23, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
FrozenDueToAge NeedsFix The path to resolution is known, but the work has not been done.
Projects
None yet
Development

No branches or pull requests

5 participants