diff mbox series

[1/2] package/pkg-golang.mk: allow packages to override download GOPROXY

Message ID 20240719030537.3156999-1-james.hilliard1@gmail.com
State Superseded
Headers show
Series [1/2] package/pkg-golang.mk: allow packages to override download GOPROXY | expand

Commit Message

James Hilliard July 19, 2024, 3:05 a.m. UTC
Some packages like the upcoming tailscale package need to set GOPROXY
to get the correct dependencies.

Signed-off-by: James Hilliard <james.hilliard1@gmail.com>
---
 package/pkg-golang.mk | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

Comments

Christian Stewart July 19, 2024, 3:26 a.m. UTC | #1
Hi James,

On Thu, Jul 18, 2024 at 8:05 PM James Hilliard
<james.hilliard1@gmail.com> wrote:
>
> Some packages like the upcoming tailscale package need to set GOPROXY
> to get the correct dependencies.
>
> Signed-off-by: James Hilliard <james.hilliard1@gmail.com>


I am confused, why does the tailscale package need this?

If GOPROXY is set to direct it should go to the go module server.

When I build tailscale from source with GOPROXY=direct it works fine:

git clone https://github.com/tailscale/tailscale
cd tailscale
export GOPROXY=direct
go mod vendor
# works

According to the Go spec, it should always be possible to download
with direct always.

I would strongly recommend not overriding this per-package.

Best regards,
Christian Stewart
James Hilliard July 19, 2024, 3:37 a.m. UTC | #2
On Thu, Jul 18, 2024 at 9:26 PM Christian Stewart <christian@aperture.us> wrote:
>
> Hi James,
>
> On Thu, Jul 18, 2024 at 8:05 PM James Hilliard
> <james.hilliard1@gmail.com> wrote:
> >
> > Some packages like the upcoming tailscale package need to set GOPROXY
> > to get the correct dependencies.
> >
> > Signed-off-by: James Hilliard <james.hilliard1@gmail.com>
>
>
> I am confused, why does the tailscale package need this?
>
> If GOPROXY is set to direct it should go to the go module server.
>
> When I build tailscale from source with GOPROXY=direct it works fine:
>
> git clone https://github.com/tailscale/tailscale
> cd tailscale
> export GOPROXY=direct
> go mod vendor
> # works

Weird, doesn't work for me, I get:
verifying fybrik.io/crdoc@v0.6.3: checksum mismatch
    downloaded: h1:Awi+v+URtj5DvPr0LLdVrW2Nn9/G1Y7aGpUPG1IATE4=
    go.sum:     h1:jNNAVINu8up5vrLa0jrV7z7HSlyHF/6lNOrAtrXwYlI=

SECURITY ERROR
This download does NOT match an earlier download recorded in go.sum.
The bits may have been replaced on the origin server, or an attacker may
have intercepted the download attempt.

For more information, see 'go help module-auth'.

>
> According to the Go spec, it should always be possible to download
> with direct always.
>
> I would strongly recommend not overriding this per-package.
>
> Best regards,
> Christian Stewart
Christian Stewart July 19, 2024, 4:23 a.m. UTC | #3
Hi James,


On Thu, Jul 18, 2024 at 8:37 PM James Hilliard
<james.hilliard1@gmail.com> wrote:
> > git clone https://github.com/tailscale/tailscale
> > cd tailscale
> > export GOPROXY=direct
> > go mod vendor
> > # works
>
> Weird, doesn't work for me, I get:
> verifying fybrik.io/crdoc@v0.6.3: checksum mismatch
>     downloaded: h1:Awi+v+URtj5DvPr0LLdVrW2Nn9/G1Y7aGpUPG1IATE4=
>     go.sum:     h1:jNNAVINu8up5vrLa0jrV7z7HSlyHF/6lNOrAtrXwYlI=
>
> SECURITY ERROR
> This download does NOT match an earlier download recorded in go.sum.
> The bits may have been replaced on the origin server, or an attacker may
> have intercepted the download attempt.
>
> For more information, see 'go help module-auth'.

This is a tailscale bug. Rather than work around this by changing
GOPROXY, we should patch go.sum, OR use a different version for that
dependency.

That dependency can be found here:
https://github.com/fybrik/crdoc/commit/edcf4b04e3c32859db138552b39e93bb56b96046

We can fix this then with a patch to add this line to go.mod:

replace fybrik.io/crdoc => github.com/fybrik/crdoc
edcf4b04e3c32859db138552b39e93bb56b96046 // v0.6.3

I have filed a tailscale bug as well:
https://github.com/tailscale/tailscale/issues/12859

Thanks!
Christian Stewart
Thomas Petazzoni Aug. 2, 2024, 9:39 p.m. UTC | #4
On Thu, 18 Jul 2024 21:05:36 -0600
James Hilliard <james.hilliard1@gmail.com> wrote:

> Some packages like the upcoming tailscale package need to set GOPROXY
> to get the correct dependencies.
> 
> Signed-off-by: James Hilliard <james.hilliard1@gmail.com>
> ---
>  package/pkg-golang.mk | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)

Considering the feedback from Christian on this PATCH 1/2, I have
marked the series as Changes Requested. Could you resend once the
vendoring issue is resolved?

Thanks!

Thomas
Thomas Petazzoni Aug. 2, 2024, 9:40 p.m. UTC | #5
On Fri, 2 Aug 2024 23:39:08 +0200
Thomas Petazzoni <thomas.petazzoni@bootlin.com> wrote:

> Considering the feedback from Christian on this PATCH 1/2, I have
> marked the series as Changes Requested. Could you resend once the
> vendoring issue is resolved?

Crap, I realized this was v1 (which was still in patchwork), and you
already sent a v2 and v3. So I'll mark the v1 as Superseded instead.

Thomas
diff mbox series

Patch

diff --git a/package/pkg-golang.mk b/package/pkg-golang.mk
index 8e27602d41..118908a159 100644
--- a/package/pkg-golang.mk
+++ b/package/pkg-golang.mk
@@ -85,10 +85,12 @@  define $(2)_GEN_GOMOD
 endef
 $(2)_POST_PATCH_HOOKS += $(2)_GEN_GOMOD
 
+$(2)_DL_GOPROXY ?= direct
+
 $(2)_DOWNLOAD_POST_PROCESS = go
 $(2)_DL_ENV += \
 	$$(HOST_GO_COMMON_ENV) \
-	GOPROXY=direct \
+	GOPROXY=$$($(2)_DL_GOPROXY) \
 	$$($(2)_GO_ENV)
 
 # Because we append vendored info, we can't rely on the values being empty