Change make build to run go build and add install task #663
Conversation
go install builds the executable and move it to GOBIN. This is useful for installing the tool but can become troublesome during development. Having go install performed in the build task is confusing, as running make build would create a "system wide" binary (as most devs have GOBIN in their PATH). To overcome this go install is moved to a dedicated make task: install.
💚 Build Succeeded
Expand to view the summary
Build stats
Test stats 🧪
🤖 GitHub commentsTo re-run your PR in the CI, just comment with:
|
|
Thanks for opening this PR! Could you please also check what documentation needs to be updated? |
Makefile
Outdated
| build: | ||
| go install -ldflags \ | ||
| go build -ldflags \ | ||
| "-X $(VERSION_IMPORT_PATH).CommitHash=`git describe --always --long --dirty` -X $(VERSION_IMPORT_PATH).BuildTime=`date +%s` -X $(VERSION_IMPORT_PATH).Tag=`(git describe --exact-match --tags 2>/dev/null || echo '') | tr -d '\n'`" \ |
There was a problem hiding this comment.
Can this string be factored out since it appears twice?
There was a problem hiding this comment.
In general, there is a rule of three, but if it isn't a big deal, we can extract this line to the top.
There was a problem hiding this comment.
I extracted and split the line to give it more clarity in 8182d2f
There was a problem hiding this comment.
What you have now is very nice. Thanks.
There was a problem hiding this comment.
You have to adjust also all scripts (.sh) as they refer to the globally installed elastic-package:
[2022-01-24T17:32:17.361Z] + make PACKAGE_UNDER_TEST=aws test-check-packages-parallel
[2022-01-24T17:32:17.361Z] PACKAGE_TEST_TYPE=parallel ./scripts/test-check-packages.sh
[2022-01-24T17:32:17.361Z] + trap cleanup EXIT
[2022-01-24T17:32:17.361Z] + OLDPWD=/var/lib/jenkins/workspace/PR-663-2-a78640ec-85fd-454a-a89c-932d6fe3894a/src/github.com/elastic/elastic-package
[2022-01-24T17:32:17.361Z] + for d in test/packages/${PACKAGE_TEST_TYPE:-other}/${PACKAGE_UNDER_TEST:-*}/
[2022-01-24T17:32:17.361Z] + cd test/packages/parallel/aws/
[2022-01-24T17:32:17.361Z] + elastic-package check -v
[2022-01-24T17:32:17.361Z] ./scripts/test-check-packages.sh: line 41: elastic-package: command not found
[2022-01-24T17:32:17.361Z] + cleanup
[2022-01-24T17:32:17.361Z] + r=127
[2022-01-24T17:32:17.361Z] + elastic-package stack dump -v --output build/elastic-stack-dump/check-aws
[2022-01-24T17:32:17.361Z] ./scripts/test-check-packages.sh: line 9: elastic-package: command not found
[2022-01-24T17:32:17.361Z] make: *** [Makefile:79: test-check-packages-parallel] Error 127
As a simple solution you can prepend an extra PATH=$PATH:\pwd`/build(considering that the binary is stored inbuild`).
5 participants
Follow-up for a conversation @jsoriano, @mtojek and me had about how
make buildworks at the moment.make buildperformsgo installunder the hood. This behaviour is unexpected asgo installmoves the built binary toGOBINfolder, which in most system is part of systemPATH. While this is the purpose ofgo installin the context ofelastic-packagemakes a bit troublesome to build binary for development and local testing.The current workaround is to manually run
go build, but we loose version information and is undocumented.To improve the situation this PR moves the current
buildtask logic to a separateinstallone and changes thebuildtask to performgo build, creating the executable in the project folder.