| Message ID | 20241028161507.186161-1-james.hilliard1@gmail.com |
|---|---|
| State | New |
| Headers | show |
| Series | [v2,1/2] package/pkg-golang.mk: make GOPROXY configurable | expand |
Hi James, On Mon, Oct 28, 2024, 9:15 AM James Hilliard <james.hilliard1@gmail.com> wrote: > Some packages like the upcoming netbird package don't work > correctly with GOPROXY=direct which is not the default setting > for the go toolchain. > https://github.com/netbirdio/netbird/pull/2789 While it is a good idea to add this option, I don't think mentioning netbird is necessary since the intent of this option is not to make packages that won't work with GOPROXY direct work. It's just for the performance improvement. When merging packages I'd still expect us to test it with GOPROXY direct as well. Lets make the GOPROXY setting configurable and use the default > toolchain setting which is most likely to be what packages end > up typically being tested against. > > Signed-off-by: James Hilliard <james.hilliard1@gmail.com> > --- > Config.in | 11 +++++++++++ > package/pkg-golang.mk | 2 +- > 2 files changed, 12 insertions(+), 1 deletion(-) > > diff --git a/Config.in b/Config.in > index df43db7eff..ad6ac38656 100644 > --- a/Config.in > +++ b/Config.in > @@ -258,6 +258,17 @@ config BR2_PRIMARY_SITE_ONLY > the project can be built even if the upstream tarball > locations disappear. > > +config BR2_GOPROXY_LIST > + string "GOPROXY list" > + default "https://proxy.golang.org,direct" > + depends on BR2_PACKAGE_HOST_GO_TARGET_ARCH_SUPPORTS > + help > + List of module proxy URLs. The go command will attempt to > + download modules from each server in sequence. The keyword > + direct instructs the go command to download modules from > + version control repositories where they’re developed instead > + of using a proxy. > Why do we put this here as opposed to BR2_PACKAGE_GO_GOPROXY I'm not against it, but it's a question that comes to mind. Also, why GOPROXY_LIST instead of just GOPROXY? Otherwise looks good. Best regards, Christian Stewart
On Mon, Oct 28, 2024 at 12:54 PM Christian Stewart <christian@aperture.us> wrote: > > Hi James, > > On Mon, Oct 28, 2024, 9:15 AM James Hilliard <james.hilliard1@gmail.com> wrote: >> >> Some packages like the upcoming netbird package don't work >> correctly with GOPROXY=direct which is not the default setting >> for the go toolchain. > > > https://github.com/netbirdio/netbird/pull/2789 > > While it is a good idea to add this option, I don't think mentioning netbird is necessary since the intent of this option is not to make packages that won't work with GOPROXY direct work. It's just for the performance improvement. IMO making the build work when direct doesn't is one of the use cases, mostly due to the potential for direct downloads breaking randomly. As GOPROXY="https://proxy.golang.org,direct" is the golang default it's what one should expect upstream projects to be testing with and the most important configuration to have functional. Building with only direct is IMO a non-standard configuration that while nice to have working shouldn't block updating packages by itself. > > When merging packages I'd still expect us to test it with GOPROXY direct as well. Possibly, however as GOPROXY=direct is a non-default configuration I think it's less important for buildroot itself. > >> Lets make the GOPROXY setting configurable and use the default >> toolchain setting which is most likely to be what packages end >> up typically being tested against. >> >> Signed-off-by: James Hilliard <james.hilliard1@gmail.com> >> --- >> Config.in | 11 +++++++++++ >> package/pkg-golang.mk | 2 +- >> 2 files changed, 12 insertions(+), 1 deletion(-) >> >> diff --git a/Config.in b/Config.in >> index df43db7eff..ad6ac38656 100644 >> --- a/Config.in >> +++ b/Config.in >> @@ -258,6 +258,17 @@ config BR2_PRIMARY_SITE_ONLY >> the project can be built even if the upstream tarball >> locations disappear. >> >> +config BR2_GOPROXY_LIST >> + string "GOPROXY list" >> + default "https://proxy.golang.org,direct" >> + depends on BR2_PACKAGE_HOST_GO_TARGET_ARCH_SUPPORTS >> + help >> + List of module proxy URLs. The go command will attempt to >> + download modules from each server in sequence. The keyword >> + direct instructs the go command to download modules from >> + version control repositories where they’re developed instead >> + of using a proxy. > > > Why do we put this here as opposed to > > BR2_PACKAGE_GO_GOPROXY > > I'm not against it, but it's a question that comes to mind. > > Also, why GOPROXY_LIST instead of just GOPROXY? I figured it was a bit more descriptive since the variable is a list of goproxies essentially, doesn't matter much to me. > > Otherwise looks good. > > Best regards, > Christian Stewart
diff --git a/Config.in b/Config.in index df43db7eff..ad6ac38656 100644 --- a/Config.in +++ b/Config.in @@ -258,6 +258,17 @@ config BR2_PRIMARY_SITE_ONLY the project can be built even if the upstream tarball locations disappear. +config BR2_GOPROXY_LIST + string "GOPROXY list" + default "https://proxy.golang.org,direct" + depends on BR2_PACKAGE_HOST_GO_TARGET_ARCH_SUPPORTS + help + List of module proxy URLs. The go command will attempt to + download modules from each server in sequence. The keyword + direct instructs the go command to download modules from + version control repositories where they’re developed instead + of using a proxy. + if !BR2_PRIMARY_SITE_ONLY config BR2_BACKUP_SITE diff --git a/package/pkg-golang.mk b/package/pkg-golang.mk index 8e27602d41..f586f75dfa 100644 --- a/package/pkg-golang.mk +++ b/package/pkg-golang.mk @@ -88,7 +88,7 @@ $(2)_POST_PATCH_HOOKS += $(2)_GEN_GOMOD $(2)_DOWNLOAD_POST_PROCESS = go $(2)_DL_ENV += \ $$(HOST_GO_COMMON_ENV) \ - GOPROXY=direct \ + GOPROXY=$$(call qstrip,$$(BR2_GOPROXY_LIST)) \ $$($(2)_GO_ENV) # Because we append vendored info, we can't rely on the values being empty
Some packages like the upcoming netbird package don't work correctly with GOPROXY=direct which is not the default setting for the go toolchain. Lets make the GOPROXY setting configurable and use the default toolchain setting which is most likely to be what packages end up typically being tested against. Signed-off-by: James Hilliard <james.hilliard1@gmail.com> --- Config.in | 11 +++++++++++ package/pkg-golang.mk | 2 +- 2 files changed, 12 insertions(+), 1 deletion(-)