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

Enforce osusergo build tag for releases #6862

Merged
merged 3 commits into from
May 6, 2019

Conversation

sapk
Copy link
Member

@sapk sapk commented May 6, 2019

Similar to #1690, we need a new tag to use the go version and not the linked cgo version that fail on some distrib. In this case, we are using it to finding the runnning user.

Should fix #6841 and also reported bug on discourse

@zeripath
Copy link
Contributor

zeripath commented May 6, 2019

@sapk are you able to reproduce this problem? We need to ensure that the issue is solved by this addition.

@GiteaBot GiteaBot added the lgtm/need 2 This PR needs two approvals by maintainers to be considered for merging. label May 6, 2019
@codecov-io
Copy link

codecov-io commented May 6, 2019

Codecov Report

Merging #6862 into master will decrease coverage by <.01%.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #6862      +/-   ##
==========================================
- Coverage   41.18%   41.17%   -0.01%     
==========================================
  Files         425      425              
  Lines       58490    58490              
==========================================
- Hits        24091    24086       -5     
- Misses      31212    31216       +4     
- Partials     3187     3188       +1
Impacted Files Coverage Δ
models/repo_list.go 66.84% <0%> (-1.06%) ⬇️
models/gpg_key.go 55.83% <0%> (-0.84%) ⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update d9d538c...d31a319. Read the comment docs.

@lafriks lafriks added this to the 1.9.0 milestone May 6, 2019
@sapk
Copy link
Member Author

sapk commented May 6, 2019

@zeripath I can't reproduce the bug exactly since I don't know (yet) which glib is incompatible with which glib version but the output of the linker go in this direction :

#Without osuserog
root@a4d05b9c2e6c:/gitea# CGO_ENABLED=1 go build -tags 'netgo sqlite sqlite_unlock_notify' -ldflags '-linkmode external -extldflags "-static"' .
# code.gitea.io/gitea
/tmp/go-link-710302265/000011.o: In function `unixDlOpen':
/root/go/pkg/mod/github.com/mattn/go-sqlite3@v1.10.0/sqlite3-binding.c:38461: warning: Using 'dlopen' in statically linked applications requires at runtime the shared libraries from the glibc version used for linking
/tmp/go-link-710302265/000015.o: In function `mygetgrouplist':
/workdir/go/src/os/user/getgrouplist_unix.go:16: warning: Using 'getgrouplist' in statically linked applications requires at runtime the shared libraries from theglibc version used for linking
/tmp/go-link-710302265/000014.o: In function `mygetgrgid_r':
/workdir/go/src/os/user/cgo_lookup_unix.go:38: warning: Using 'getgrgid_r' in statically linked applications requires at runtime the shared libraries from the glibc version used for linking
/tmp/go-link-710302265/000014.o: In function `mygetgrnam_r':
/workdir/go/src/os/user/cgo_lookup_unix.go:43: warning: Using 'getgrnam_r' in statically linked applications requires at runtime the shared libraries from the glibc version used for linking
/tmp/go-link-710302265/000014.o: In function `mygetpwnam_r':
/workdir/go/src/os/user/cgo_lookup_unix.go:33: warning: Using 'getpwnam_r' in statically linked applications requires at runtime the shared libraries from the glibc version used for linking
/tmp/go-link-710302265/000014.o: In function `mygetpwuid_r':
/workdir/go/src/os/user/cgo_lookup_unix.go:28: warning: Using 'getpwuid_r' in statically linked applications requires at runtime the shared libraries from the glibc version used for linking
#With osusergo
root@a4d05b9c2e6c:/gitea# CGO_ENABLED=1 go build -tags 'netgo osusergo sqlite sqlite_unlock_notify' -ldflags '-linkmode external -extldflags "-static"' .
# code.gitea.io/gitea
/tmp/go-link-514533019/000011.o: In function `unixDlOpen':
/root/go/pkg/mod/github.com/mattn/go-sqlite3@v1.10.0/sqlite3-binding.c:38461: warning: Using 'dlopen' in statically linked applications requires at runtime the shared libraries from the glibc version used for linking

The released gitea 1.8 binary seems to work on the docker image debian:9.6. I will try to find a tag that doesn't work.

@zeripath
Copy link
Contributor

zeripath commented May 6, 2019

I've been trying to find the broken version too... I think it's libc6 2.26 but I don't know which version of debian has that.

@zeripath
Copy link
Contributor

zeripath commented May 6, 2019

hmm... Is it possible that this might fix the static build problems for Arm7 might be fixed by osusergo too?

@sapk
Copy link
Member Author

sapk commented May 6, 2019

On the docker image 9.6 it is libc6 2.24

@zeripath
Copy link
Contributor

zeripath commented May 6, 2019

ubuntu:bionic has 2.27-3ubuntu1 and works

@sapk
Copy link
Member Author

sapk commented May 6, 2019

For testing, I use :

$ cat test.sh
#!/bin/sh
apt-get update
apt-get upgrade -y
apt-get install -y git wget
dpkg -l libc6
/gitea &
sleep 5s
wget localhost:3000/install
sleep 5s

wget https://github.com/go-gitea/gitea/releases/download/v1.8.0/gitea-1.8.0-linux-amd64 && chmod +x ./gitea-1.8.0-linux-amd64
docker run -ti --rm -v "$(pwd)/gitea-1.8.0-linux-amd64:/gitea" -v "$(pwd)/test.sh:/test.sh" debian:9.6 /test.sh

@zeripath
Copy link
Contributor

zeripath commented May 6, 2019

17.10 works even though its version is 2.26-0ubuntu2.1

Makefile Show resolved Hide resolved
Makefile Show resolved Hide resolved
Makefile Show resolved Hide resolved
@zeripath
Copy link
Contributor

zeripath commented May 6, 2019

so static_build appears to remove a few more warnings too - leaving only dlopen warnings which appears to be because sqlite3 can load shared objects so I think that might be irremovable.

Now the arm7 build from that still doesn't work - it just gives me an illegal instruction.

@sapk
Copy link
Member Author

sapk commented May 6, 2019

@zeripath what warning do you got that where remove with static_build ? unixDlOpen was the only I still got using only netgo and osusergo.

@zeripath
Copy link
Contributor

zeripath commented May 6, 2019

oh sorry I'm talking nonsense. I misread your message earlier.

And I've misunderstood how the suggestion was working - the static_build tag must have been internal to that particular commenter's build.

@zeripath
Copy link
Contributor

zeripath commented May 6, 2019

@sapk why don't you come to discord and we can chat a bit more without spamming this

@GiteaBot GiteaBot added lgtm/need 1 This PR needs approval from one additional maintainer to be merged. and removed lgtm/need 2 This PR needs two approvals by maintainers to be considered for merging. labels May 6, 2019
@zeripath
Copy link
Contributor

zeripath commented May 6, 2019

I've decided to approve even though we can't prove that this will solve the problem.

Unfortunately it looks like this doesn't solve the arm7 problem (at least on pi2)

@GiteaBot GiteaBot added lgtm/done This PR has enough approvals to get merged. There are no important open reservations anymore. and removed lgtm/need 1 This PR needs approval from one additional maintainer to be merged. labels May 6, 2019
@lafriks lafriks merged commit 650df0b into go-gitea:master May 6, 2019
@zeripath
Copy link
Contributor

zeripath commented May 6, 2019

So this will need to be backported to 1.8 in order for us to fix the issue.

@sapk
Copy link
Member Author

sapk commented May 6, 2019

@zeripath done

@lafriks lafriks added the backport/done All backports for this PR have been created label May 6, 2019
@go-gitea go-gitea locked and limited conversation to collaborators Nov 24, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
backport/done All backports for this PR have been created lgtm/done This PR has enough approvals to get merged. There are no important open reservations anymore. type/bug
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Debian 9.6 AMD64: Segfault on start at os/user.Current (1.7.0+)
5 participants