diff mbox series

package/pkg-golang: enable pkg specific go env also for download step

Message ID 20221122212002.139494-1-pge@ik.me
State Changes Requested
Headers show
Series package/pkg-golang: enable pkg specific go env also for download step | expand

Commit Message

Patrick Gerber Nov. 22, 2022, 9:20 p.m. UTC
Currently package secific go env is used only during package build step.

Go vendering is done during the download step and it's sometimes required
to specify package secific go env also for this step.

For example, when importing custom go modules who are hosted on a private
host, it’s required to set GOPRIVATE to avoid public sum checking.
---
 package/pkg-golang.mk | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

Comments

Yann E. MORIN Dec. 3, 2022, 1:29 p.m. UTC | #1
PGE, All,

We require that submitters use their real identity; initials are not
enough.

On 2022-11-22 22:20 +0100, PGE via buildroot spake thusly:
> Currently package secific go env is used only during package build step.
> 
> Go vendering is done during the download step and it's sometimes required
> to specify package secific go env also for this step.
> 
> For example, when importing custom go modules who are hosted on a private
> host, it’s required to set GOPRIVATE to avoid public sum checking.

I am not sure I fully understand what "set GOPRIVATE to avoid public sum
checking" is supposed to mean.

Does that mean that setting GOPRIVATE (to what value: boolean? list of
modules? something else?) would prevent go (the program) from performing
checksum verification on the modules it downloads?

As a consequence, what impact does that have on the reproducibility of
the vendored archive? And so, what about the hashes maanged by
Buildroot?


We also require that submissions include a sign-off tag, matching the
real identity.

Please see;
    https://buildroot.org/downloads/manual/manual.html#submitting-patches

> ---
>  package/pkg-golang.mk | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/package/pkg-golang.mk b/package/pkg-golang.mk
> index 0b3dc3d32f..cdc67b32c0 100644
> --- a/package/pkg-golang.mk
> +++ b/package/pkg-golang.mk
> @@ -87,7 +87,8 @@ $(2)_POST_PATCH_HOOKS += $(2)_GEN_GOMOD
>  $(2)_DOWNLOAD_POST_PROCESS = go
>  $(2)_DL_ENV += \
>  	$$(HOST_GO_COMMON_ENV) \
> -	GOPROXY=direct
> +	GOPROXY=direct \
> +	$$($(2)_GO_ENV)

Currently, we have two packages that set _GO_ENV:

  - docker-cli: I don't think this would have an impact omn the download
    step and the vendoring:
        DOCKER_CLI_GO_ENV = CGO_ENBALED=no

  - mender-artifacts: there we do ecplicitly refer to an inconsistency
    about the vendoring:
        HOST_MENDER_ARTIFACT_GO_ENV = GOFLAGS="-mod=vendor"

    so I wonder how that would play when we also pass this during the
    vendoring step. I hope that it would have no effect, because
    'vendor' is actually the path where the vendoring happens.

Did you check if they were impacted by this change?

Barring any issue with the above questions, I think this is a good
change. Please respin with your real identity, a proper sign-off tag.

Regards,
Yann E. MORIN.

>  # Due to vendoring, it is pretty likely that not all licenses are
>  # listed in <pkg>_LICENSE.
> -- 
> 2.25.1
> 
> _______________________________________________
> buildroot mailing list
> buildroot@buildroot.org
> https://lists.buildroot.org/mailman/listinfo/buildroot
Patrick Gerber Dec. 4, 2022, 2:06 p.m. UTC | #2
> PGE, All,
>
> We require that submitters use their real identity; initials are not
> enough.
Sorry for that, it's fixed
> On 2022-11-22 22:20 +0100, PGE via buildroot spake thusly:
>> Currently package secific go env is used only during package build step.
>>
>> Go vendering is done during the download step and it's sometimes required
>> to specify package secific go env also for this step.
>>
>> For example, when importing custom go modules who are hosted on a private
>> host, it’s required to set GOPRIVATE to avoid public sum checking.
> I am not sure I fully understand what "set GOPRIVATE to avoid public sum
> checking" is supposed to mean.
>
> Does that mean that setting GOPRIVATE (to what value: boolean? list of
> modules? something else?) would prevent go (the program) from performing
> checksum verification on the modules it downloads?
GOPRIVATE shall be set to a glob pattern matching your private repo host 
name, so go vendoring do not check the sum for theses packages. See 
https://pkg.go.dev/cmd/go#hdr-Configuration_for_downloading_non_public_code 
for more details.
> As a consequence, what impact does that have on the reproducibility of
> the vendored archive? And so, what about the hashes maanged by
> Buildroot?
In my understanding it has strictly no impact on buildroot managed 
hashes as vendoring is performed, during download step, before 
generating the dl archive which include the vendor directory. So all 
external package downloaded by go are included in the buildroot hash
> We also require that submissions include a sign-off tag, matching the
> real identity.
>
> Please see;
>      https://buildroot.org/downloads/manual/manual.html#submitting-patches
>
>> ---
>>   package/pkg-golang.mk | 3 ++-
>>   1 file changed, 2 insertions(+), 1 deletion(-)
>>
>> diff --git a/package/pkg-golang.mk b/package/pkg-golang.mk
>> index 0b3dc3d32f..cdc67b32c0 100644
>> --- a/package/pkg-golang.mk
>> +++ b/package/pkg-golang.mk
>> @@ -87,7 +87,8 @@ $(2)_POST_PATCH_HOOKS += $(2)_GEN_GOMOD
>>   $(2)_DOWNLOAD_POST_PROCESS = go
>>   $(2)_DL_ENV += \
>>   	$$(HOST_GO_COMMON_ENV) \
>> -	GOPROXY=direct
>> +	GOPROXY=direct \
>> +	$$($(2)_GO_ENV)
> Currently, we have two packages that set _GO_ENV:
>
>    - docker-cli: I don't think this would have an impact omn the download
>      step and the vendoring:
>          DOCKER_CLI_GO_ENV = CGO_ENBALED=no
>
>    - mender-artifacts: there we do ecplicitly refer to an inconsistency
>      about the vendoring:
>          HOST_MENDER_ARTIFACT_GO_ENV = GOFLAGS="-mod=vendor"
>
>      so I wonder how that would play when we also pass this during the
>      vendoring step. I hope that it would have no effect, because
>      'vendor' is actually the path where the vendoring happens.
>
> Did you check if they were impacted by this change?
I checked for mender-artifact and it has no issue. I even do think that 
this extra go env is not required, as all go packages built by buildroot 
are built with GOFLAGS="-mod=vendor" (see HOST_GO_COMMON_ENV in go/go.mk).

download/go-post-process is checking the absence of vendor directory 
before doing the vendoring. If present vendoring is bypassed.

I tested without HOST_MENDER_ARTIFACT_GO_ENV = GOFLAGS="-mod=vendor" and 
mender-artifact build without issue
> Barring any issue with the above questions, I think this is a good
> change. Please respin with your real identity, a proper sign-off tag.
>
> Regards,
> Yann E. MORIN.
>
>>   # Due to vendoring, it is pretty likely that not all licenses are
>>   # listed in <pkg>_LICENSE.
>> -- 
>> 2.25.1
>>
>> _______________________________________________
>> buildroot mailing list
>> buildroot@buildroot.org
>> https://lists.buildroot.org/mailman/listinfo/buildroot
Best regards
Patrick
diff mbox series

Patch

diff --git a/package/pkg-golang.mk b/package/pkg-golang.mk
index 0b3dc3d32f..cdc67b32c0 100644
--- a/package/pkg-golang.mk
+++ b/package/pkg-golang.mk
@@ -87,7 +87,8 @@  $(2)_POST_PATCH_HOOKS += $(2)_GEN_GOMOD
 $(2)_DOWNLOAD_POST_PROCESS = go
 $(2)_DL_ENV += \
 	$$(HOST_GO_COMMON_ENV) \
-	GOPROXY=direct
+	GOPROXY=direct \
+	$$($(2)_GO_ENV)
 
 # Due to vendoring, it is pretty likely that not all licenses are
 # listed in <pkg>_LICENSE.