Fix OCI image build #1666
Fix OCI image build #1666
Conversation
knative-prow
bot
added
approved
hack/build-flags.sh
Outdated
| ${EXTERNAL_LD_FLAGS:-}" | ||
| } | ||
|
|
||
| build_flags "$@" |
There was a problem hiding this comment.
@rhuss I went with the following call here, but I don't removing the functions completely.
Codecov Report
@@ Coverage Diff @@
## main #1666 +/- ##
=======================================
Coverage 79.49% 79.49%
=======================================
Files 171 171
Lines 12969 12969
=======================================
Hits 10310 10310
Misses 1941 1941
Partials 718 718 Continue to review full report at Codecov.
|
| # Use vYYYYMMDD-<hash>-local for the version string, if not passed. | ||
| [[ -z "${version}" ]] && version="v${BUILD_TAG}-local" | ||
| # Env var exported by hack/build-flags.sh | ||
| local ld_flags="${KN_BUILD_LD_FLAGS:-}" |
There was a problem hiding this comment.
An empty default doesn't work well because then you would have a -ldflags without value to the option which will very likely lead to an error. IMO you could directly use KN_BUILD_FLAGS and commit ld_flags.
Or use something like
| local ld_flags="${KN_BUILD_LD_FLAGS:-}" | |
| local ld_flags="-ldflags ${KN_BUILD_LD_FLAGS:-}" |
and then just add $ld_flags below, that should also then work with an empty env var.
There was a problem hiding this comment.
The proposal doesn't work smoothly with our date value and quoting. On the other hand I've tested with great success that empty flags from KN_BUILD_LD_FLAGS doesn't break the build or produce any error.
There was a problem hiding this comment.
ok, so let's go with the original approach if this is no problem.
/approve
/lgtm
|
|
||
| export GO111MODULE=on | ||
| export CGO_ENABLED=0 | ||
| echo "🚧 🐧 Building for Linux (amd64)" | ||
| GOOS=linux GOARCH=amd64 go build -mod=vendor -ldflags "${ld_flags}" -o ./kn-linux-amd64 ./cmd/... |
There was a problem hiding this comment.
| GOOS=linux GOARCH=amd64 go build -mod=vendor -ldflags "${ld_flags}" -o ./kn-linux-amd64 ./cmd/... | |
| GOOS=linux GOARCH=amd64 go build -mod=vendor ${ld_flags} -o ./kn-linux-amd64 ./cmd/... |
| basedir=$(basedir) | ||
| ld_flags="$(build_flags $basedir)" | ||
| # Env var exported by hack/build-flags.sh | ||
| local ld_flags="${KN_BUILD_LD_FLAGS:-}" |
There was a problem hiding this comment.
See my comment below on how to deal with an empty default.
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: dsimansk, rhuss The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
|
/retest |
1 similar comment
|
/retest |
Description
Changes
Reference
Related to #257
/cc @rhuss