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

Unable to clean modules installed by xcaddy build #102

Closed
akeskimo opened this issue Jun 7, 2022 · 12 comments · Fixed by #104
Closed

Unable to clean modules installed by xcaddy build #102

akeskimo opened this issue Jun 7, 2022 · 12 comments · Fixed by #104

Comments

@akeskimo
Copy link
Contributor

akeskimo commented Jun 7, 2022

While building with xcaddy with Yocto I am getting the following error

ERROR: Command '['rm', '-rf', '/work/yocto/build/tmp/work/aarch64-poky-linux/caddy/2.4.6-r0/build/pkg']' returned non-zero exit status 1.

In our build machine, we need write access to modules directory in order to re-compile the project but go modules are installed with read access, only. This can be fixed by executing go build with flag '-modcacherw` but xcaddy does not support passing custom flags from environment.

@akeskimo
Copy link
Contributor Author

akeskimo commented Jun 7, 2022

The flag modcacherw has been introduced since go1.4 and sets write bit on pulled go modules. Certain build tools will try to clean up the modules directory and do not have root priviledges so it would be convenient to add this support for xcaddy.

@mholt
Copy link
Member

mholt commented Jun 7, 2022

Why do you need write access to the modules? 🤔 What is the xcaddy command you are running and full output?

@akeskimo
Copy link
Contributor Author

akeskimo commented Jun 9, 2022

The command I am executing is

xcaddy build e7457b43e4703080ae8713ada798ce3e20b83690 --with github.com/ggicci/caddy-jwt@v0.7.1

which works just fine for the first time. It fails in consequent runs because the build system must clean the files before it can be re-compiled but it fails because go installs modules as RO which causes rm to bail out since my user has no root privileges.

@akeskimo
Copy link
Contributor Author

akeskimo commented Jun 9, 2022

There is also another related issue with xcaddy is that Yocto trims the produced binaries automatically but xcaddy enforces this by adding flags

builder.go

cmd.Args = append(cmd.Args,
	"-ldflags", "-w -s", // trim debug symbols
	"-trimpath",
)

so I will need to add the following exception to my recipe

# Binaries procuded by xcaddy are stripped already which would cause QA error during build so we can safely ignore this warning.
INSANE_SKIP_${PN} += "already-stripped"

This could be avoided if there would be an environment variable to pass the build flags to xcaddy which would allow the user to control the behavior of compilation in contrast to hardcoding 'em.

@akeskimo akeskimo changed the title Modules created in read-only mode Unable to clean modules installed by xcaddy build Jun 9, 2022
@mholt
Copy link
Member

mholt commented Jun 9, 2022

It fails in consequent runs because the build system must clean the files before it can be re-compiled

Sorry, I don't understand. Go doesn't delete files to re-compile a second time.

This sounds like more of a problem with whatever Yocto is, than xcaddy or Go. 🤔

@akeskimo
Copy link
Contributor Author

akeskimo commented Jun 10, 2022

Sorry, I don't understand. Go doesn't delete files to re-compile a second time.

Yocto is a distributed build-system to compile Linux kernels which are used by millions of Embedded Linux devices. Yes, you are correct: Go does not delete files during recompiling, but a build system might.

This sounds like more of a problem with whatever Yocto is, than xcaddy or Go. thinking

No, this is not an issue with Go or Yocto. Let me explain.

Go installs modules with read-only access (by default) because Golang was developed by Google who uses monolithic repository structure which can benefit from using global module path. The global module install cannot be used for distributed build-systems because modules need to declare their own dependencies which would blow up in disk space. Fortunately, this has been addressed by Golang maintainers a while ago by introducing flag modcacherw which xcaddy seems to be ignoring, for some reason.

Here is background information and discussion related to why the flag is useful golang/go#27161.

@akeskimo
Copy link
Contributor Author

akeskimo commented Jun 10, 2022

In Caddy-speak, what I need is something like this

env XCADDY_GOBUILDFLAGS="-modcacherw" xcaddy build e7457b43e4703080ae8713ada798ce3e20b83690 --with github.com/ggicci/caddy-jwt@v0.7.1

which allows me to redefine build arguments from our build system.

I have already patched this in our build but I decided to bring this up because the caddy community would benefit from it.

@mholt
Copy link
Member

mholt commented Jun 11, 2022

Oh, I see what you mean now. Thanks for the explanation. Yeah, that env var seems like a good way to work around it, I guess. (I'm way less picky about xcaddy than I am about caddy anyway.) Want to submit a PR?

@akeskimo
Copy link
Contributor Author

Sure, the patch needs a bit of polishing but I should be able to submit it by next week.

akeskimo added a commit to akeskimo/xcaddy that referenced this issue Jun 16, 2022
Add environment variable

 XCADDY_GOBUILDFLAGS

to override default build arguments from environment (caddyserver#102).

To support flags with variable arguments Unix-style quoting is supported.
@akeskimo
Copy link
Contributor Author

I am not sure what is your process for contributing so I created a fork for the pull request. You can comment on the PR if any changes are needed.

@mholt
Copy link
Member

mholt commented Jun 16, 2022

Perfect, thanks @akeskimo !

akeskimo added a commit to akeskimo/xcaddy that referenced this issue Jun 17, 2022
Add environment variable

 XCADDY_GO_BUILD_FLAGS

to override default build arguments from environment (caddyserver#102).

To support flags with variable arguments Unix-style quoting is supported.
mholt pushed a commit that referenced this issue Jun 20, 2022
Add environment variable

 XCADDY_GO_BUILD_FLAGS

to override default build arguments from environment (#102).

To support flags with variable arguments Unix-style quoting is supported.
akeskimo added a commit to akeskimo/xcaddy that referenced this issue Aug 13, 2022
Commit 47f9ded is not sufficient alone to
ensure that all go modules are installed with write bit set because go mod
and go get installs modules of other modules with read-only access.

The environment variable sets -modcacherw to both go get and go mod commands
to ensure that the build/pkg/mod directory does not have sub directories
that are read-only for the current user.
akeskimo added a commit to akeskimo/xcaddy that referenced this issue Aug 13, 2022
Commit 47f9ded is not sufficient alone to
ensure that all go modules are installed with write bit set because go mod
and go get installs modules of other modules with read-only access.

The environment variable sets -modcacherw to both go get and go mod commands
to ensure that the build/pkg/mod directory does not have sub directories
that are read-only for the current user.
akeskimo added a commit to akeskimo/xcaddy that referenced this issue Aug 13, 2022
Commit 47f9ded is not sufficient alone to
ensure that all go modules are installed with write bit set because go mod
and go get installs modules of other modules with read-only access.

If set, the environment variable sets -modcacherw to ensure that also module
sub directories have write-access to the current user.
akeskimo added a commit to akeskimo/xcaddy that referenced this issue Aug 13, 2022
Commit 47f9ded is not sufficient alone to
ensure that all go modules are installed with write bit set because 'go mod'
or 'go get' might install submodules of other modules as read-only.

If set, the environment variable appends '-modcacherw' to go mod and go get
to ensure that submodules of other modules have write access.
@akeskimo
Copy link
Contributor Author

Hi @mholt,

We still have the problem that go modules that are pulled in by go get or go mod might be read-only which we are unable to clean from our build environment. I have added another pull request which should fully resolve this issue. We currently have this patch applied in our build system (Yocto) but here it is if you would like to have this in upstream, as well.

akeskimo added a commit to akeskimo/xcaddy that referenced this issue Aug 15, 2022
Commit 47f9ded is not sufficient alone to
ensure that all go modules are installed with write bit set because 'go mod'
or 'go get' might install modules as read-only, too.

If set, the environment variable is appended for the other go commands that
support build flags.
akeskimo added a commit to akeskimo/xcaddy that referenced this issue Aug 15, 2022
Commit 47f9ded is not sufficient alone to
ensure that all go modules are installed with write bit set because 'go mod'
or 'go get' might install modules as read-only, too.

If set, the environment variable is appended for the other go commands that
support build flags.
mohammed90 pushed a commit that referenced this issue Aug 16, 2022
* Extend XCADDY_GO_BUILD_FLAGS usage (#102)

Commit 47f9ded is not sufficient alone to
ensure that all go modules are installed with write bit set because 'go mod'
or 'go get' might install modules as read-only, too.

If set, the environment variable is appended for the other go commands that
support build flags.

* Make running 'go' implicit for build environment

The method newCommand runs only go commands so let's make command 'go'
implicit and rename it to make it more verbose.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants