diff mbox series

[next,05/12] support/download/go-post-process: implement Go vendoring support

Message ID 20201119213658.1232531-6-thomas.petazzoni@bootlin.com
State Superseded
Headers show
Series Support for Cargo and Go vendoring | expand

Commit Message

Thomas Petazzoni Nov. 19, 2020, 9:36 p.m. UTC 
This commit introduces the download post-process script
support/download/go-post-process, and hooks it into the Go package
infrastructure.

Signed-off-by: Thomas Petazzoni <thomas.petazzoni@bootlin.com>
---
 package/pkg-golang.mk            |  7 ++++++-
 support/download/go-post-process | 29 +++++++++++++++++++++++++++++
 2 files changed, 35 insertions(+), 1 deletion(-)
 create mode 100755 support/download/go-post-process

Comments

Christian Stewart Nov. 25, 2020, 8:28 p.m. UTC | #1 
Hi Thomas,

On Thu, Nov 19, 2020 at 1:37 PM Thomas Petazzoni
<thomas.petazzoni@bootlin.com> wrote:
>
> This commit introduces the download post-process script
> support/download/go-post-process, and hooks it into the Go package
> infrastructure.

> +++ b/support/download/go-post-process
> @@ -0,0 +1,29 @@
> +#!/usr/bin/env bash
> +
> +set -e
> +
> +. $(dirname $0)/post-process-helpers
> +
> +# Parse our options
> +while getopts "n:o:" OPT; do
> +        case "${OPT}" in
> +        o)  output="${OPTARG}";;
> +        n)  base_name="${OPTARG}";;
> +        :)  error "option '%s' expects a mandatory argument\n" "${OPTARG}";;
> +        \?) error "unknown option '%s'\n" "${OPTARG}";;
> +        esac
> +done
> +
> +# Already vendored tarball, nothing to do
> +if tar tf ${output} | grep -q "^[^/]*/vendor" ; then
> +       exit 0
> +fi
> +
> +unpack ${base_name} ${output}
> +
> +# Do the Go vendoring
> +pushd ${base_name} > /dev/null
> +go mod vendor -v
> +popd > /dev/null
> +
> +repack ${base_name} ${output}

This looks good. There's only one problem - if the package does not
come with a go.mod file (which is the case with
nvidia-container-toolkit).

In this case we need to run the $(2)_GEN_GOMOD script before running
the download post-process.

Today, this is written as so and creates a go.mod if it doesn't already exist:

# Generate a go.mod file if it doesn't exist. Note: Go is configured
# to use the "vendor" dir and not make network calls.
define $(2)_GEN_GOMOD
if [ ! -f $$(@D)/go.mod ]; then \
  printf "module $$($(2)_GOMOD)\n" > $$(@D)/go.mod; \
fi
endef
$(2)_POST_PATCH_HOOKS += $(2)_GEN_GOMOD

Otherwise it looks like it works well, with this one tweak.

Best regards,
Christian Stewart
Thomas Petazzoni Nov. 25, 2020, 8:37 p.m. UTC | #2 
Hello Christian,

Thanks for the feedback!

On Wed, 25 Nov 2020 12:28:31 -0800
Christian Stewart <christian@paral.in> wrote:

> This looks good. There's only one problem - if the package does not
> come with a go.mod file (which is the case with
> nvidia-container-toolkit).
> 
> In this case we need to run the $(2)_GEN_GOMOD script before running
> the download post-process.
> 
> Today, this is written as so and creates a go.mod if it doesn't already exist:
> 
> # Generate a go.mod file if it doesn't exist. Note: Go is configured
> # to use the "vendor" dir and not make network calls.
> define $(2)_GEN_GOMOD
> if [ ! -f $$(@D)/go.mod ]; then \
>   printf "module $$($(2)_GOMOD)\n" > $$(@D)/go.mod; \
> fi
> endef
> $(2)_POST_PATCH_HOOKS += $(2)_GEN_GOMOD

Ah, good point. Do you have an example of such a package (publicly
available)? That would help testing a solution.

I guess the best solution is to modify the POST_PATCH_HOOKS to be done
by the download post-process script ?

Thomas
Christian Stewart Nov. 25, 2020, 8:45 p.m. UTC | #3 
Hi Thomas,


On Wed, Nov 25, 2020 at 12:28 PM Christian Stewart <christian@paral.in> wrote:
> On Thu, Nov 19, 2020 at 1:37 PM Thomas Petazzoni
> <thomas.petazzoni@bootlin.com> wrote:
> >
> > This commit introduces the download post-process script
> > support/download/go-post-process, and hooks it into the Go package
> > infrastructure.
>
> This looks good. There's only one problem - if the package does not
> come with a go.mod file (which is the case with
> nvidia-container-toolkit).
>
> In this case we need to run the $(2)_GEN_GOMOD script before running
> the download post-process.

I was able to solve this with the following patch:

diff --git a/package/pkg-download.mk b/package/pkg-download.mk
index c914d016e2..29462ebaa4 100644
--- a/package/pkg-download.mk
+++ b/package/pkg-download.mk
@@ -112,6 +112,7 @@ define DOWNLOAD
  -o '$($(2)_DL_DIR)/$(notdir $(1))' \
  $(if $($(2)_DOWNLOAD_POST_PROCESS),-p '$($(2)_DOWNLOAD_POST_PROCESS)') \
  $(if $($(2)_GIT_SUBMODULES),-r) \
+ $(if $($(2)_GOMOD),-g '$($(2)_GOMOD)') \
  $(foreach uri,$(call DOWNLOAD_URIS,$(1),$(2)),-u $(uri)) \
  $(QUIET) \
  -- \
diff --git a/support/download/dl-wrapper b/support/download/dl-wrapper
index 2d74554213..2fc530f24f 100755
--- a/support/download/dl-wrapper
+++ b/support/download/dl-wrapper
@@ -17,7 +17,7 @@
 # We want to catch any unexpected failure, and exit immediately.
 set -e

-export BR_BACKEND_DL_GETOPTS=":hc:d:o:n:N:H:ru:qf:e"
+export BR_BACKEND_DL_GETOPTS=":hc:d:g:o:n:N:H:ru:qf:e"

 main() {
     local OPT OPTARG
@@ -25,11 +25,12 @@ main() {
     local -a uris

     # Parse our options; anything after '--' is for the backend
-    while getopts ":c:d:D:o:n:N:H:rf:u:qp:" OPT; do
+    while getopts ":c:d:D:g:o:n:N:H:rf:u:qp:" OPT; do
         case "${OPT}" in
         c)  cset="${OPTARG}";;
         d)  dl_dir="${OPTARG}";;
         D)  old_dl_dir="${OPTARG}";;
+        g)  gomod_init="${OPTARG}";;
         o)  output="${OPTARG}";;
         n)  raw_base_name="${OPTARG}";;
         N)  base_name="${OPTARG}";;
@@ -138,6 +139,7 @@ main() {

         if [ -n "${post_process}" ] ; then
                 ${OLDPWD}/support/download/${post_process}-post-process \
+                         -g "${gomod_init}" \
                          -o "${tmpf}" \
                          -n "${raw_base_name}"
         fi
diff --git a/support/download/go-post-process b/support/download/go-post-process
index 01073aee8b..796fa08e38 100755
--- a/support/download/go-post-process
+++ b/support/download/go-post-process
@@ -5,8 +5,9 @@ set -e
 . $(dirname $0)/post-process-helpers

 # Parse our options
-while getopts "n:o:" OPT; do
+while getopts "n:g:o:" OPT; do
         case "${OPT}" in
+        g)  gomod_init="${OPTARG}";;
         o)  output="${OPTARG}";;
         n)  base_name="${OPTARG}";;
         :)  error "option '%s' expects a mandatory argument\n" "${OPTARG}";;
@@ -23,6 +24,10 @@ unpack ${base_name} ${output}

 # Do the Go vendoring
 pushd ${base_name} > /dev/null
+if [ -n "${gomod_init}" ]; then
+    # Note: does nothing if go.mod already exists.
+    go mod init ${gomod_init}
+fi
 go mod vendor -v
 popd > /dev/null

Not sure if this is the best way to pass the go module identifier into
the script, but running "go mod init" will conditionally create the
go.mod file if it doesn't already exist, just before running "go mod
vendor."

Best,
Christian
Christian Stewart Nov. 25, 2020, 8:51 p.m. UTC | #4 
Hi Thomas,

There are a number of packages that don't have go.mod files yet, but
might have files from other package managers in the Go ecosystem.

On Wed, Nov 25, 2020 at 12:37 PM Thomas Petazzoni
<thomas.petazzoni@bootlin.com> wrote:
>
> Hello Christian,
>
> Thanks for the feedback!
>
> On Wed, 25 Nov 2020 12:28:31 -0800
> Christian Stewart <christian@paral.in> wrote:
>
> > This looks good. There's only one problem - if the package does not
> > come with a go.mod file (which is the case with
> > nvidia-container-toolkit).
> >
> > In this case we need to run the $(2)_GEN_GOMOD script before running
> > the download post-process.
> >
> > Today, this is written as so and creates a go.mod if it doesn't already exist:
> >
> > # Generate a go.mod file if it doesn't exist. Note: Go is configured
> > # to use the "vendor" dir and not make network calls.
> > define $(2)_GEN_GOMOD
> > if [ ! -f $$(@D)/go.mod ]; then \
> >   printf "module $$($(2)_GOMOD)\n" > $$(@D)/go.mod; \
> > fi
> > endef
> > $(2)_POST_PATCH_HOOKS += $(2)_GEN_GOMOD
>
> Ah, good point. Do you have an example of such a package (publicly
> available)? That would help testing a solution.

Yes: https://github.com/NVIDIA/nvidia-container-runtime

Buildroot package:
https://github.com/skiffos/buildroot/commit/983f30d31c7fc8011fbfb57dd3abef1751c3e4d7

And some more like it in this tree:

https://github.com/skiffos/buildroot/tree/pkg-mgr

>
> I guess the best solution is to modify the POST_PATCH_HOOKS to be done
> by the download post-process script ?

The go module system will actually convert those other formats
faithfully into the go.mod and go.sum format.

If we add the patch that I sent in the previous email, which runs "go
mod init" just before "go mod vendor," it will read the legacy formats
in this step.

Alternatively we could do something which was rejected in one of my
original go.mod support revisions - adding the go.mod and go.sum files
into the Buildroot tree.

To generate these:

 1. Clone the repo
 2. "go mod init" and "go mod vendor"
 3. Copy go.mod and go.sum to buildroot tree
 4. These are copied (if exist) into the package just before running
"go mod vendor" in the post-process.

The reasoning behind this is to provide an exact version for all of
the dependencies locked in the Buildroot tree so that the output is
deterministic. We might get hash mismatches otherwise.

Best,
Christian


>
> Thomas
> --
> Thomas Petazzoni, CTO, Bootlin
> Embedded Linux and Kernel engineering
> https://bootlin.com
Thomas Petazzoni Nov. 25, 2020, 8:52 p.m. UTC | #5 
Hello,

On Wed, 25 Nov 2020 12:45:59 -0800
Christian Stewart <christian@paral.in> wrote:


> diff --git a/package/pkg-download.mk b/package/pkg-download.mk
> index c914d016e2..29462ebaa4 100644
> --- a/package/pkg-download.mk
> +++ b/package/pkg-download.mk
> @@ -112,6 +112,7 @@ define DOWNLOAD
>   -o '$($(2)_DL_DIR)/$(notdir $(1))' \
>   $(if $($(2)_DOWNLOAD_POST_PROCESS),-p '$($(2)_DOWNLOAD_POST_PROCESS)') \
>   $(if $($(2)_GIT_SUBMODULES),-r) \
> + $(if $($(2)_GOMOD),-g '$($(2)_GOMOD)') \
>   $(foreach uri,$(call DOWNLOAD_URIS,$(1),$(2)),-u $(uri)) \
>   $(QUIET) \
>   -- \
> diff --git a/support/download/dl-wrapper b/support/download/dl-wrapper
> index 2d74554213..2fc530f24f 100755
> --- a/support/download/dl-wrapper
> +++ b/support/download/dl-wrapper
> @@ -17,7 +17,7 @@
>  # We want to catch any unexpected failure, and exit immediately.
>  set -e
> 
> -export BR_BACKEND_DL_GETOPTS=":hc:d:o:n:N:H:ru:qf:e"
> +export BR_BACKEND_DL_GETOPTS=":hc:d:g:o:n:N:H:ru:qf:e"
> 
>  main() {
>      local OPT OPTARG
> @@ -25,11 +25,12 @@ main() {
>      local -a uris
> 
>      # Parse our options; anything after '--' is for the backend
> -    while getopts ":c:d:D:o:n:N:H:rf:u:qp:" OPT; do
> +    while getopts ":c:d:D:g:o:n:N:H:rf:u:qp:" OPT; do
>          case "${OPT}" in
>          c)  cset="${OPTARG}";;
>          d)  dl_dir="${OPTARG}";;
>          D)  old_dl_dir="${OPTARG}";;
> +        g)  gomod_init="${OPTARG}";;

Unfortunately, this is not really a very good solution, because the
dl-wrapper is generic, and if we start adding language-specific and
backend-specific options, it's really going to become a nightmare :-/

>  # Do the Go vendoring
>  pushd ${base_name} > /dev/null
> +if [ -n "${gomod_init}" ]; then
> +    # Note: does nothing if go.mod already exists.
> +    go mod init ${gomod_init}

So this cannot simply be conditional on go.mod existing or not, because
we need the name of the Go module ?

Best regards,

Thomas
Christian Stewart Nov. 25, 2020, 8:54 p.m. UTC | #6 
Hi Thomas,

On Wed, Nov 25, 2020 at 12:52 PM Thomas Petazzoni
<thomas.petazzoni@bootlin.com> wrote:
> >
> >      # Parse our options; anything after '--' is for the backend
> > -    while getopts ":c:d:D:o:n:N:H:rf:u:qp:" OPT; do
> > +    while getopts ":c:d:D:g:o:n:N:H:rf:u:qp:" OPT; do
> >          case "${OPT}" in
> >          c)  cset="${OPTARG}";;
> >          d)  dl_dir="${OPTARG}";;
> >          D)  old_dl_dir="${OPTARG}";;
> > +        g)  gomod_init="${OPTARG}";;
>
> Unfortunately, this is not really a very good solution, because the
> dl-wrapper is generic, and if we start adding language-specific and
> backend-specific options, it's really going to become a nightmare :-/

Pass it as an environment variable? I don't see what the problem is
aside from the optargs.

> >  # Do the Go vendoring
> >  pushd ${base_name} > /dev/null
> > +if [ -n "${gomod_init}" ]; then
> > +    # Note: does nothing if go.mod already exists.
> > +    go mod init ${gomod_init}
>
> So this cannot simply be conditional on go.mod existing or not, because
> we need the name of the Go module ?

The "go mod init" command will actually do nothing if go.mod already exists.

We do need the name of the go module though in the case that go.mod
does not exist.

Best regards,
Christian
Christian Stewart Nov. 25, 2020, 9:02 p.m. UTC | #7 
Thomas,

On Wed, Nov 25, 2020 at 12:54 PM Christian Stewart <christian@paral.in> wrote:
> > So this cannot simply be conditional on go.mod existing or not, because
> > we need the name of the Go module ?
>
> The "go mod init" command will actually do nothing if go.mod already exists.

Scratch that, "go mod init" will actually fail if go.mod already
exists. So we should instead:

if [ ! -f go.mod ] && [ -n "${gomod_init}" ]; then
    go mod init ${gomod_init}
fi

Apologies for confusion on that.

Best,
Christian
Ryan Barnett Nov. 25, 2020, 9:07 p.m. UTC | #8 
Thomas, All,

I've tried out the new go vendoring support and I am running into an
issue with cleaning the build directory when using this series. I've
added a new package on your pkg-mgr branch called maddy with the
following at package/maddy/maddy.mk:

MADDY_VERSION = 0.4.2
MADDY_SITE = $(call github,foxcpp,maddy,v$(MADDY_VERSION))

$(eval $(golang-package))

Then I run the following commands:

make maddy
make clean

The error I get is as follows:

rm -rf /home/ryan/projects/br/br-pkg-mgr/output/target
/home/ryan/projects/br/br-pkg-mgr/output/images
/home/ryan/projects/br/br-pkg-mgr/output/host  \
        /home/ryan/projects/br/br-pkg-mgr/output/build
/home/ryan/projects/br/br-pkg-mgr/output/staging \
        /home/ryan/projects/br/br-pkg-mgr/output/legal-info
/home/ryan/projects/br/br-pkg-mgr/output/graphs
/home/ryan/projects/br/br-pkg-mgr/output/per-package
rm: cannot remove
'/home/ryan/projects/br/br-pkg-mgr/output/host/share/go-path/pkg/mod/github.com/cespare/xxhash/v2@v2.1.1/README.md':
Permission denied
rm: cannot remove
'/home/ryan/projects/br/br-pkg-mgr/output/host/share/go-path/pkg/mod/github.com/cespare/xxhash/v2@v2.1.1/xxhash_amd64.go':
Permission denied
rm: cannot remove
'/home/ryan/projects/br/br-pkg-mgr/output/host/share/go-path/pkg/mod/github.com/cespare/xxhash/v2@v2.1.1/xxhash_amd64.s':
Permission denied
rm: cannot remove
'/home/ryan/projects/br/br-pkg-mgr/output/host/share/go-path/pkg/mod/github.com/cespare/xxhash/v2@v2.1.1/xxhash_test.go':
Permission denied
rm: cannot remove
'/home/ryan/projects/br/br-pkg-mgr/output/host/share/go-path/pkg/mod/github.com/cespare/xxhash/v2@v2.1.1/xxhash.go':
Permission denied
rm: cannot remove
'/home/ryan/projects/br/br-pkg-mgr/output/host/share/go-path/pkg/mod/github.com/cespare/xxhash/v2@v2.1.1/go.mod':
Permission denied
rm: cannot remove
'/home/ryan/projects/br/br-pkg-mgr/output/host/share/go-path/pkg/mod/github.com/cespare/xxhash/v2@v2.1.1/xxhsum/xxhsum.go':
Permission denied
rm: cannot remove
'/home/ryan/projects/br/br-pkg-mgr/output/host/share/go-path/pkg/mod/github.com/cespare/xxhash/v2@v2.1.1/xxhsum/.gitignore':
Permission denied
rm: cannot remove
'/home/ryan/projects/br/br-pkg-mgr/output/host/share/go-path/pkg/mod/github.com/cespare/xxhash/v2@v2.1.1/xxhash_unsafe.go':
Permission denied
rm: cannot remove
'/home/ryan/projects/br/br-pkg-mgr/output/host/share/go-path/pkg/mod/github.com/cespare/xxhash/v2@v2.1.1/xxhash_other.go':
Permission denied
rm: cannot remove
'/home/ryan/projects/br/br-pkg-mgr/output/host/share/go-path/pkg/mod/github.com/cespare/xxhash/v2@v2.1.1/LICENSE.txt':
Permission denied
rm: cannot remove
'/home/ryan/projects/br/br-pkg-mgr/output/host/share/go-path/pkg/mod/github.com/cespare/xxhash/v2@v2.1.1/go.sum':
Permission denied
rm: cannot remove
'/home/ryan/projects/br/br-pkg-mgr/output/host/share/go-path/pkg/mod/github.com/cespare/xxhash/v2@v2.1.1/.travis.yml':
Permission denied
rm: cannot remove
'/home/ryan/projects/br/br-pkg-mgr/output/host/share/go-path/pkg/mod/github.com/cespare/xxhash/v2@v2.1.1/xxhash_safe.go':
Permission denied
rm: cannot remove
'/home/ryan/projects/br/br-pkg-mgr/output/host/share/go-path/pkg/mod/github.com/cespare/xxhash/v2@v2.1.1/xxhash_unsafe_test.go':
Permission denied

There are about 1000 more lines with this error. The only way I can do
a clean is by running:

sudo rm -rf output/*

Here are the permissions on one of the files:

$ ls -al /home/ryan/projects/br/br-pkg-mgr/output/host/share/go-path/pkg/mod/github.com/cespare/xxhash/v2@v2.1.1/xxhash_unsafe_test.go
-r--r--r-- 1 ryan ryan 402 Nov 25 15:01
/home/ryan/projects/br/br-pkg-mgr/output/host/share/go-path/pkg/mod/github.com/cespare/xxhash/v2@v2.1.1/xxhash_unsafe_test.go

I've tried that same package on the latest version of master and I do
not see this error. With that same maddy.mk file on master I am able
to successfully build the package and run `make clean`. I believe the
error I see it is due to the vendoring of go modules. Before today,
I've never build a go package so I'm not familiar with how those files
could be have incorrect permissions.

Thanks,
-Ryan

On Thu, Nov 19, 2020 at 3:37 PM Thomas Petazzoni
<thomas.petazzoni@bootlin.com> wrote:
>
> This commit introduces the download post-process script
> support/download/go-post-process, and hooks it into the Go package
> infrastructure.
>
> Signed-off-by: Thomas Petazzoni <thomas.petazzoni@bootlin.com>
> ---
>  package/pkg-golang.mk            |  7 ++++++-
>  support/download/go-post-process | 29 +++++++++++++++++++++++++++++
>  2 files changed, 35 insertions(+), 1 deletion(-)
>  create mode 100755 support/download/go-post-process

[...]
Christian Stewart Nov. 25, 2020, 9:12 p.m. UTC | #9 
Hi Ryan, all,

On Wed, Nov 25, 2020 at 1:07 PM Ryan Barnett <ryanbarnett3@gmail.com> wrote:
> I've tried out the new go vendoring support and I am running into an
> issue with cleaning the build directory when using this series. I've
> added a new package on your pkg-mgr branch called maddy with the
> following at package/maddy/maddy.mk:

> make maddy
> make clean

> The error I get is as follows:
>
> rm -rf /home/ryan/projects/br/br-pkg-mgr/output/target
> /home/ryan/projects/br/br-pkg-mgr/output/images
> /home/ryan/projects/br/br-pkg-mgr/output/host  \
>         /home/ryan/projects/br/br-pkg-mgr/output/build
> /home/ryan/projects/br/br-pkg-mgr/output/staging \
>         /home/ryan/projects/br/br-pkg-mgr/output/legal-info
> /home/ryan/projects/br/br-pkg-mgr/output/graphs
> /home/ryan/projects/br/br-pkg-mgr/output/per-package
> rm: cannot remove
> '/home/ryan/projects/br/br-pkg-mgr/output/host/share/go-path/pkg/mod/github.com/cespare/xxhash/v2@v2.1.1/README.md':
> Permission denied

> There are about 1000 more lines with this error. The only way I can do
> a clean is by running:
>
> sudo rm -rf output/*
>
> Here are the permissions on one of the files:
>
> $ ls -al /home/ryan/projects/br/br-pkg-mgr/output/host/share/go-path/pkg/mod/github.com/cespare/xxhash/v2@v2.1.1/xxhash_unsafe_test.go
> -r--r--r-- 1 ryan ryan 402 Nov 25 15:01
> /home/ryan/projects/br/br-pkg-mgr/output/host/share/go-path/pkg/mod/github.com/cespare/xxhash/v2@v2.1.1/xxhash_unsafe_test.go

This is because the Go tool sets restrictive permissions on the
gopath/pkg/mod tree to prevent source code tools from editing the
dependencies code on the filesystem.

Relevant Go issues:

 - https://github.com/golang/go/issues/27455

myitcv says: "This is, as @mark-rushakoff pointed out, working as
intended. The integrity of pkg/mod/** is important. Hence it is not
writable."

 - https://github.com/golang/go/issues/31481

We can solve this by adding "-modcacherw" flag to the "go mod"
commands. I will test this and send some more info.

Best regards,
Christian Stewart
Christian Stewart Nov. 25, 2020, 9:21 p.m. UTC | #10 
Hi Ryan, Thomas, all,

I have a modified version of the go-post-process patch with fixes here:

https://github.com/skiffos/buildroot/commit/5f31774f54402f07c1d83aa407430db5152e6de3

On Wed, Nov 25, 2020 at 1:12 PM Christian Stewart <christian@paral.in> wrote:
> On Wed, Nov 25, 2020 at 1:07 PM Ryan Barnett <ryanbarnett3@gmail.com> wrote:
> > I've tried out the new go vendoring support and I am running into an
> > issue with cleaning the build directory when using this series. I've
> > added a new package on your pkg-mgr branch called maddy with the
> > following at package/maddy/maddy.mk:
>
> > make maddy
> > make clean
>
> > The error I get is as follows:
> >
> > rm -rf /home/ryan/projects/br/br-pkg-mgr/output/target
> > /home/ryan/projects/br/br-pkg-mgr/output/images
> > /home/ryan/projects/br/br-pkg-mgr/output/host  \
> >         /home/ryan/projects/br/br-pkg-mgr/output/build
> > /home/ryan/projects/br/br-pkg-mgr/output/staging \
> >         /home/ryan/projects/br/br-pkg-mgr/output/legal-info
> > /home/ryan/projects/br/br-pkg-mgr/output/graphs
> > /home/ryan/projects/br/br-pkg-mgr/output/per-package
> > rm: cannot remove
> > '/home/ryan/projects/br/br-pkg-mgr/output/host/share/go-path/pkg/mod/github.com/cespare/xxhash/v2@v2.1.1/README.md':
> > Permission denied

Here is the fix for this issue specifically (using the modcacherw flag):

diff --git a/package/pkg-golang.mk b/package/pkg-golang.mk
index 150445bf17..b22720add6 100644
--- a/package/pkg-golang.mk
+++ b/package/pkg-golang.mk
@@ -42,6 +42,7 @@ define inner-golang-package

 $(2)_BUILD_OPTS += \
  -ldflags "$$($(2)_LDFLAGS)" \
+ -modcacherw \
  -tags "$$($(2)_TAGS)" \
  -trimpath \
  -p $(PARALLEL_JOBS)
diff --git a/support/download/go-post-process b/support/download/go-post-process
index 8830ee56c1..1e5441f6e6 100755
--- a/support/download/go-post-process
+++ b/support/download/go-post-process
@@ -24,10 +24,12 @@ unpack ${base_name} ${output}

 # Do the Go vendoring
 pushd ${base_name} > /dev/null
+# modcacherw option leaves directories in the module cache at their default
+# permissions rather than making them read-only.
 if [ ! -f go.mod ] && [ -n "${gomod_init}" ]; then
-    go mod init ${gomod_init}
+    go mod init -modcacherw ${gomod_init}
 fi
-go mod vendor -v
+go mod vendor -modcacherw -v
 popd > /dev/null

 repack ${base_name} ${output}

Best regards,
Christian Stewart
Ryan Barnett Nov. 25, 2020, 9:49 p.m. UTC | #11 
Christian, Thomas,

On Wed, Nov 25, 2020 at 3:21 PM Christian Stewart <christian@paral.in> wrote:
>
> Hi Ryan, Thomas, all,
>
> I have a modified version of the go-post-process patch with fixes here:
>
> https://github.com/skiffos/buildroot/commit/5f31774f54402f07c1d83aa407430db5152e6de3
>
> On Wed, Nov 25, 2020 at 1:12 PM Christian Stewart <christian@paral.in> wrote:
> > On Wed, Nov 25, 2020 at 1:07 PM Ryan Barnett <ryanbarnett3@gmail.com> wrote:
> > > I've tried out the new go vendoring support and I am running into an
> > > issue with cleaning the build directory when using this series. I've
> > > added a new package on your pkg-mgr branch called maddy with the
> > > following at package/maddy/maddy.mk:
> >
> > > make maddy
> > > make clean
> >
> > > The error I get is as follows:
> > >
> > > rm -rf /home/ryan/projects/br/br-pkg-mgr/output/target
> > > /home/ryan/projects/br/br-pkg-mgr/output/images
> > > /home/ryan/projects/br/br-pkg-mgr/output/host  \
> > >         /home/ryan/projects/br/br-pkg-mgr/output/build
> > > /home/ryan/projects/br/br-pkg-mgr/output/staging \
> > >         /home/ryan/projects/br/br-pkg-mgr/output/legal-info
> > > /home/ryan/projects/br/br-pkg-mgr/output/graphs
> > > /home/ryan/projects/br/br-pkg-mgr/output/per-package
> > > rm: cannot remove
> > > '/home/ryan/projects/br/br-pkg-mgr/output/host/share/go-path/pkg/mod/github.com/cespare/xxhash/v2@v2.1.1/README.md':
> > > Permission denied
>
> Here is the fix for this issue specifically (using the modcacherw flag):

[...]

I manually added the '-modcacherw' flag to the pkg-golang.mk and
go-post-process files and I was able to now successfully run 'make
clean'

Thanks for the quick response and the fix.

-Ryan
diff mbox series

Patch

diff --git a/package/pkg-golang.mk b/package/pkg-golang.mk
index 3813e1c406..150445bf17 100644
--- a/package/pkg-golang.mk
+++ b/package/pkg-golang.mk
@@ -47,7 +47,7 @@  $(2)_BUILD_OPTS += \
 	-p $(PARALLEL_JOBS)
 
 # Target packages need the Go compiler on the host.
-$(2)_DEPENDENCIES += host-go
+$(2)_DOWNLOAD_DEPENDENCIES += host-go
 
 $(2)_BUILD_TARGETS ?= .
 
@@ -72,6 +72,11 @@  $(2)_SRC_SOFTWARE = $$(word 2,$$(subst /, ,$$(call notdomain,$$($(2)_SITE))))
 # If the go.mod file does not exist, one is written with this root path.
 $(2)_GOMOD ?= $$($(2)_SRC_DOMAIN)/$$($(2)_SRC_VENDOR)/$$($(2)_SRC_SOFTWARE)
 
+$(2)_DOWNLOAD_POST_PROCESS = go
+$(2)_DL_ENV = \
+	$(HOST_GO_COMMON_ENV) \
+	GOPROXY=direct
+
 # Generate a go.mod file if it doesn't exist. Note: Go is configured
 # to use the "vendor" dir and not make network calls.
 define $(2)_GEN_GOMOD
diff --git a/support/download/go-post-process b/support/download/go-post-process
new file mode 100755
index 0000000000..01073aee8b
--- /dev/null
+++ b/support/download/go-post-process
@@ -0,0 +1,29 @@ 
+#!/usr/bin/env bash
+
+set -e
+
+. $(dirname $0)/post-process-helpers
+
+# Parse our options
+while getopts "n:o:" OPT; do
+        case "${OPT}" in
+        o)  output="${OPTARG}";;
+        n)  base_name="${OPTARG}";;
+        :)  error "option '%s' expects a mandatory argument\n" "${OPTARG}";;
+        \?) error "unknown option '%s'\n" "${OPTARG}";;
+        esac
+done
+
+# Already vendored tarball, nothing to do
+if tar tf ${output} | grep -q "^[^/]*/vendor" ; then
+	exit 0
+fi
+
+unpack ${base_name} ${output}
+
+# Do the Go vendoring
+pushd ${base_name} > /dev/null
+go mod vendor -v
+popd > /dev/null
+
+repack ${base_name} ${output}