Skip to content

We should avoid setting GO env variables #3231

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.

Sign up for GitHub

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

Merged
merged 1 commit into from
Mar 17, 2025
Merged

We should avoid setting GO env variables #3231

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion .containerversion
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1 +1 @@
28
29
18 changes: 7 additions & 11 deletions Dockerfile
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,25 +11,21 @@
# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
# See the License for the specific language governing permissions and
# limitations under the License.
FROM thyrlian/android-sdk:4.0 as android
FROM thyrlian/android-sdk:4.0 AS android

FROM ubuntu:22.04

ENV DEBIAN_FRONTEND noninteractive

# Android
COPY --from=android /opt/android-sdk /opt/android-sdk

ADD scripts/docker_install.sh /tmp/
RUN /tmp/docker_install.sh
RUN --mount=target=/mnt,source=scripts DEBIAN_FRONTEND=noninteractive /mnt/docker_install.sh

ENV GOPATH /opt/go
ENV GOROOT /opt/go_dist/go
ENV PATH $GOROOT/bin:$GOPATH/bin:$PATH
# In this path executables will be installed during docker build
ARG SYS_GOPATH=/opt/go
ENV PATH=${SYS_GOPATH}/bin:/usr/local/go/bin:$PATH

ADD Makefile /tmp/
RUN make -C /tmp/ envinit
RUN --mount=target=/mnt/Makefile,source=Makefile GOPATH=${SYS_GOPATH} make -C /mnt envinit

ENV PATH /opt/qt6/6.8.2/gcc_64/bin:/opt/qt6/6.8.2/gcc_64/libexec:$PATH
ENV PATH=/opt/qt6/6.8.2/gcc_64/bin:/opt/qt6/6.8.2/gcc_64/libexec:$PATH

CMD ["bash"]
9 changes: 5 additions & 4 deletions Makefile
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,14 +14,12 @@

SHELL := /bin/bash
WEBROOT := frontends/web
GOPATH ?= $(HOME)/go
Copy link
Collaborator Author

@NickeZ NickeZ Mar 10, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

GOPATH can be any directory on your system. In Unix examples, we will set it to $HOME/go (the default since Go 1.8).

PATH := $(PATH):$(GOPATH)/bin

catch:
@echo "Choose a make target."
envinit:
# Keep golangci-lint version in sync with what's in .github/workflows/ci.yml.
curl -sfL https://raw.githubusercontent.com/golangci/golangci-lint/master/install.sh | sh -s -- -b $(GOPATH)/bin v1.61.0
curl -sfL https://raw.githubusercontent.com/golangci/golangci-lint/master/install.sh | sh -s -- -b $(shell go env GOPATH)/bin v1.61.0
go install github.com/vektra/mockery/[email protected]
go install github.com/matryer/[email protected]
go install golang.org/x/tools/cmd/goimports@latest
Expand Down Expand Up @@ -84,8 +82,11 @@ clean:
cd frontends/qt && $(MAKE) clean
cd frontends/android && $(MAKE) clean
cd backend/mobileserver && $(MAKE) clean

# The container image only supports amd64 bercause of "thyrlian/android-sdk"
# that downloads amd64 specific binaries
dockerinit:
./scripts/container.sh build --pull --force-rm -t shiftcrypto/bitbox-wallet-app .
./scripts/container.sh build --platform linux/amd64 --pull -t shiftcrypto/bitbox-wallet-app:$(shell cat .containerversion) .
dockerdev:
./scripts/dockerdev.sh
locize-push:
Expand Down
2 changes: 0 additions & 2 deletions android-env.mk.inc
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,2 @@
export ANDROID_SDK_ROOT := /opt/android-sdk
export ANDROID_NDK_HOME := /opt/android-sdk/ndk/21.2.6472646
# gomodcache location for gomobile build pkgs. Set to tmp folder, because writing access is needed.
export GOMODCACHE_ROOT := /tmp/gomodcache/pkg/mod
7 changes: 3 additions & 4 deletions backend/mobileserver/Makefile
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,10 +1,9 @@
include ../../android-env.mk.inc

# GOMODCACHE to /tmp directory, because gomobile needs to write pkgs.
# Also set -glflags to fix the vendor issue with gomobile, see: https://github.com/golang/go/issues/67927#issuecomment-2241523694
# Set -glflags to fix the vendor issue with gomobile, see: https://github.com/golang/go/issues/67927#issuecomment-2241523694
build-android:
GOMODCACHE=${GOMODCACHE_ROOT} ANDROID_HOME=${ANDROID_SDK_ROOT} gomobile bind -x -a -glflags="-mod=readonly" -ldflags="-s -w" -target android .
ANDROID_HOME=${ANDROID_SDK_ROOT} gomobile bind -x -a -glflags="-mod=readonly" -ldflags="-s -w" -target android .
build-ios:
GOMODCACHE=${GOMODCACHE_ROOT} gomobile bind -x -a -glflags="-mod=readonly" -ldflags="-s -w" -target ios,iossimulator .
gomobile bind -x -a -glflags="-mod=readonly" -ldflags="-s -w" -target ios,iossimulator .
clean:
rm -f mobileserver.aar mobileserver-sources.jar
4 changes: 1 addition & 3 deletions scripts/docker_install.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,6 @@
# See the License for the specific language governing permissions and
# limitations under the License.


apt-get update
apt-get install -y --no-install-recommends curl ca-certificates

Expand Down Expand Up @@ -83,8 +82,7 @@ aqt install-qt linux desktop 6.8.2 -m qtpositioning qtserialport qtwebchannel qt
npm install -g npm@10
npm install -g locize-cli

mkdir -p /opt/go_dist
curl https://dl.google.com/go/go1.22.5.linux-amd64.tar.gz | tar -xz -C /opt/go_dist
curl https://dl.google.com/go/go1.22.5.linux-amd64.tar.gz | tar -xz -C /usr/local

# fuse is needed to run the linuxdeployqt appimage.
apt-get install -y --no-install-recommends fuse
Expand Down
20 changes: 7 additions & 13 deletions scripts/github-ci.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ if [ "$OS_NAME" == "linux" ]; then
# Which docker image to use to run the CI. Defaults to Docker Hub.
# Overwrite with CI_IMAGE=docker/image/path environment variable.
# Keep this in sync with .github/workflows/ci.yml.
: "${CI_IMAGE:=shiftcrypto/bitbox-wallet-app:26}"
: "${CI_IMAGE:=shiftcrypto/bitbox-wallet-app:$(cat .containerversion)}"
# Time image pull to compare in the future.
time docker pull "$CI_IMAGE"

Expand All @@ -22,24 +22,18 @@ if [ "$OS_NAME" == "linux" ]; then
# CI (https://github.com/actions/checkout/issues/760)
docker run --privileged \
-v $HOME/.gradle:/root/.gradle \
-v ${GITHUB_BUILD_DIR}:/opt/go/${GO_SRC_DIR}/ \
-v ${GITHUB_BUILD_DIR}:/root/go/${GO_SRC_DIR}/ \
-i "${CI_IMAGE}" \
bash -c "git config --global --add safe.directory \$GOPATH/${GO_SRC_DIR} && make -C \$GOPATH/${GO_SRC_DIR} ${WHAT}"
bash -c "git config --global --add safe.directory \$(go env GOPATH)/${GO_SRC_DIR} && make -C \$(go env GOPATH)/${GO_SRC_DIR} ${WHAT}"
fi

# The following is executed only on macOS machines.
if [ "$OS_NAME" == "osx" ]; then
# GitHub CI installs Go and Qt directly in the macos action, before executing
# this script.
go version
export GOPATH=~/go
export PATH="$PATH:~/go/bin"
mkdir -p $GOPATH/$(dirname $GO_SRC_DIR)
# GitHub checkout action (git clone) seem to require current work dir
# to be the root of the repo during its clean up phase. So, we push it
# here and pop in the end.
pushd ../ && cp -a bitbox-wallet-app $GOPATH/$(dirname $GO_SRC_DIR)
cd $GOPATH/$GO_SRC_DIR
make "$WHAT"
popd
export PATH="~/go/bin:$PATH"
mkdir -p $(go env GOPATH)/$(dirname $GO_SRC_DIR)
cp -a ../bitbox-wallet-app $(go env GOPATH)/$(dirname $GO_SRC_DIR)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this seems wrong, there should be no need to copy the repo somewhere else.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is that something from how go used to work? that the go code had to be copied to a certain directory structure?

Copy link
Collaborator Author

@NickeZ NickeZ Mar 17, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There is this comment earlier:

# Because we need to compile some Go code without modules,
# the source must be placed in a specific directory as expected by Go.
# The path is relative to GOPATH.
GO_SRC_DIR=src/github.com/BitBoxSwiss/bitbox-wallet-app

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since Go modules has been introduced, the repo can live anywhere. There is, or was, a catch with gomobile, which isn't compatible with modules:

This forced us to turn off modules, which required the repo to be again in the GOPATH like it was required in the past.

However, GO111MODULE=off was removed in this commit 684e62e#diff-76ed074a9305c04054cdebb9e9aad2d818052b07091de1f20cad0bbac34ffb52L31, so maybe even android/iOS can now be built regardless of the location, but I am not sure.

I guess for this PR, you could just leave it as is, as changing this is unrelated to this PR.

make -C $(go env GOPATH)/$GO_SRC_DIR "$WHAT"
fi