Skip to content

ci: update to Go 1.16.6 on CI #8324

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 20 commits into from
Aug 17, 2021
+14 −9
Merged

ci: update to Go 1.16.6 on CI #8324

Show file tree
Hide file tree
Changes from 10 commits
Commits
Show all changes
20 commits
Select commit Hold shift + click to select a range
339fb46
update Go to 1.16.4 on CircleCI
marten-seemann May 14, 2021
bf4cf14
install specific Go version for sharness tests
marten-seemann May 14, 2021
a9dfe38
stop using deprecated Go flag -i
marten-seemann May 15, 2021
7f2630b
debug
marten-seemann May 15, 2021
9d6cbdf
Use Go 1.16.6
guseggert Jul 30, 2021
b5f5419
Fix tar cmd
guseggert Jul 30, 2021
f0ae6e3
Disable MAKE_SKIP_PATH
guseggert Aug 2, 2021
98d2739
Remove MAKE_SKIP_PATH test
guseggert Aug 2, 2021
db2684f
Merge branch 'master' into update-go-on-ci-guseggert
guseggert Aug 2, 2021
d346840
Remove MAKE_SKIP_PATH, update Dockerfile base image
guseggert Aug 3, 2021
5c0061e
Fix space constant
guseggert Aug 12, 2021
c1339c8
save PATH to .bashrc instead of $BASH_ENV
guseggert Aug 13, 2021
4ebec49
use 2xlarge resource class
guseggert Aug 13, 2021
3a7eaaf
Revert "Remove MAKE_SKIP_PATH, update Dockerfile base image"
guseggert Aug 13, 2021
c40f8d1
Update Docker Go version to 1.16.6
guseggert Aug 13, 2021
e162f1a
Revert "Remove MAKE_SKIP_PATH test"
guseggert Aug 13, 2021
b3f1d65
Revert "Disable MAKE_SKIP_PATH"
guseggert Aug 13, 2021
ef1703c
Increase Make job parallelism to 10
guseggert Aug 14, 2021
679c004
Merge branch 'master' into update-go-on-ci-guseggert
guseggert Aug 16, 2021
70b3882
Use Go 1.16.7
guseggert Aug 17, 2021
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
11 changes: 8 additions & 3 deletions .circleci/config.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ default_environment: &default_environment
executors:
golang:
docker:
- image: circleci/golang:1.15.2
- image: cimg/go:1.16.6
working_directory: ~/ipfs/go-ipfs
environment:
<<: *default_environment
Expand All @@ -58,7 +58,7 @@ executors:
E2E_IPFSD_TYPE: go
dockerizer:
docker:
- image: circleci/golang:1.15.2
- image: cimg/go:1.16.6
environment:
IMAGE_NAME: ipfs/go-ipfs
WIP_IMAGE_TAG: wip
Expand Down Expand Up @@ -126,6 +126,12 @@ jobs:
TEST_VERBOSE: 1
steps:
- run: sudo apt update
- run: |
mkdir ~/localgo && cd ~/localgo
wget https://golang.org/dl/go1.16.6.linux-amd64.tar.gz
Copy link
Member

Choose a reason for hiding this comment

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

Should be 1.16.7

tar xfz go1.16.6.linux-amd64.tar.gz
echo "export PATH=$(pwd)/go/bin:$PATH" >> $BASH_ENV
- run: go version
Copy link
Contributor

Choose a reason for hiding this comment

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

What's the reason why we're doing this/what happens when we don't? Is this because the machine executor doesn't have the same version of Go as the golang ones?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good question, this might be unnecessary code, it was carried over from #8138. Let me dig deeper.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah this is required because the sharness test is using an Ubuntu image which is on Go 1.15.2, not the Go image.

- run: sudo apt install socat net-tools
- checkout

Expand All @@ -151,7 +157,6 @@ jobs:
- run:
echo TEST_DOCKER_HOST=$TEST_DOCKER_HOST &&
make -O -j 3 coverage/sharness_tests.coverprofile test/sharness/test-results/sharness.xml TEST_GENERATE_JUNIT=1 CONTINUE_ON_S_FAILURE=1 TEST_DOCKER_HOST=$TEST_DOCKER_HOST

- run:
when: always
command: bash <(curl -s https://codecov.io/bash) -cF sharness -X search -f coverage/sharness_tests.coverprofile
Expand Down
2 changes: 1 addition & 1 deletion Dockerfile
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
# Note: when updating the go minor version here, also update the go-channel in snap/snapcraft.yml
FROM golang:1.15.2-buster
FROM golang:1.16.6-buster
LABEL maintainer="Steven Allen <[email protected]>"

# Install deps
Expand Down
2 changes: 1 addition & 1 deletion docs/plugins.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -140,7 +140,7 @@ To build out-of-tree plugins, use the plugin's Makefile if provided. Otherwise,
you can manually build the plugin by running:

```bash
myplugin$ go build -buildmode=plugin -i -o myplugin.so myplugin.go
myplugin$ go build -buildmode=plugin -o myplugin.so myplugin.go
```

Finally, as with in-tree plugins:
Expand Down
2 changes: 1 addition & 1 deletion plugin/plugins/Rules.mk
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ $($(d)_plugins_main):

$($(d)_plugins_so): %.so : %/main/main.go
$($(d)_plugins_so): $$(DEPS_GO) ALWAYS
$(GOCC) build -buildmode=plugin -i -pkgdir "$(GOPATH)/pkg/linux_amd64_dynlink" $(go-flags-with-tags) -o "$@" "$(call go-pkg-name,$(basename $@))/main"
$(GOCC) build -buildmode=plugin -pkgdir "$(GOPATH)/pkg/linux_amd64_dynlink" $(go-flags-with-tags) -o "$@" "$(call go-pkg-name,$(basename $@))/main"
chmod +x "$@"

CLEAN += $($(d)_plugins_so)
Expand Down
2 changes: 0 additions & 2 deletions test/sharness/Rules.mk
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,8 +26,6 @@ DEPS_$(d) += $(PLUGINS_$(d))
endif
endif

export MAKE_SKIP_PATH=1

$(T_$(d)): $$(DEPS_$(d)) # use second expansion so coverage can inject dependency
@echo "*** $@ ***"
ifeq ($(CONTINUE_ON_S_FAILURE),1)
Expand Down
20 changes: 9 additions & 11 deletions test/sharness/lib/test-lib.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,17 +9,15 @@
# use the ipfs tool to test against

# add current directory to path, for ipfs tool.
if test "$MAKE_SKIP_PATH" != "1"; then
Copy link
Contributor

Choose a reason for hiding this comment

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

What changed with MAKE_SKIP_PATH?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'm not sure exactly why this fixes it. Presumably something somewhere else has a side efffect of setting the PATH so that it contains the correct ipfs binary for the test, which is what the sharness tests were implicitly relying on. Something changed with 1.16 such that the ipfs binary is no longer found in the PATH. I spent a few hours trying to find where, unsuccessfully. I couldn't find any rationale for MAKE_SKIP_PATH and the tests pass when I disable it, b/c PATH is setup correctly before each test run (pointing to the coverage binary).

Copy link
Contributor

Choose a reason for hiding this comment

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

I'm not totally sure, but I think the idea here is that you can locally run sharness tests on your machine from the sharness test folder without causing weird issues. Could you double check those work.

It seems like removing that flag shouldn't change anything (we set it to 1 in the makefile and then check if it equals 1).

I'm not sure exactly why this fixes it.

What was failing before and with which code changes? You've also changed a couple commands here (e.g. from BIN=$(cd .. && echo `pwd`/bin) to BIN=$(cd .. && pwd)/bin ). Any chance those were the changes that fixed things and not MAKE_SKIP_PATH?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I did figure this out finally. It was due to this:

echo "export PATH=$(pwd)/go/bin:$PATH" >> $BASH_ENV

(link)

That writes a PATH env var to BASH_ENV, and BASH_ENV is inherited by every subshell, so every time a subshell is run, Bash applies env vars in BASH_ENV which overwrite whatever PATH was inherited from the parent process, which means setting PATH in any Makefile (or any other env in BASH_ENV) is silently ignored.

(Setting PATH with BASH_ENV is what CircleCI docs recommend....)

BIN=$(cd .. && echo `pwd`/bin)
BIN2=$(cd ../.. && echo `pwd`/cmd/ipfs)
PATH=${BIN2}:${BIN}:${PATH}

# assert the `ipfs` we're using is the right one.
if test `which ipfs` != ${BIN2}/ipfs; then
echo >&2 "Cannot find the tests' local ipfs tool."
echo >&2 "Please check test and ipfs tool installation."
exit 1
fi
BIN=$(cd .. && pwd)/bin
BIN2=$(cd ../.. && pwd)/cmd/ipfs
PATH=${BIN2}:${BIN}:${PATH}

# assert the `ipfs` we're using is the right one.
if test "$(which ipfs)" != "${BIN2}/ipfs"; then
echo >&2 "Cannot find the tests' local ipfs tool."
echo >&2 "Please check test and ipfs tool installation."
exit 1
fi

# set sharness verbosity. we set the env var directly as
Expand Down