-
Notifications
You must be signed in to change notification settings - Fork 114
Conversation
Did you check |
@@ -37,14 +37,14 @@ mkdir -p "${SRCPATH}" | |||
ln -s "${TOPWD}/ecs-init" "${SRCPATH}" | |||
cd "${SRCPATH}/ecs-init" | |||
if [[ "$1" == "dev" ]]; then | |||
go build -tags 'development' -ldflags "${VERSION_FLAG} ${GIT_HASH_FLAG} ${GIT_DIRTY_FLAG}" \ | |||
CGO_ENABLED=1 CGO_LDFLAGS_ALLOW='-Wl,--unresolved-symbols=ignore-in-object-files' go build -tags 'development' -ldflags "${VERSION_FLAG} ${GIT_HASH_FLAG} ${GIT_DIRTY_FLAG}" \ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit: maybe put the flags into another variable since we repeat later
@@ -37,14 +37,14 @@ mkdir -p "${SRCPATH}" | |||
ln -s "${TOPWD}/ecs-init" "${SRCPATH}" | |||
cd "${SRCPATH}/ecs-init" | |||
if [[ "$1" == "dev" ]]; then | |||
go build -tags 'development' -ldflags "${VERSION_FLAG} ${GIT_HASH_FLAG} ${GIT_DIRTY_FLAG}" \ | |||
CGO_ENABLED=1 CGO_LDFLAGS_ALLOW='-Wl,--unresolved-symbols=ignore-in-object-files' go build -tags 'development' -ldflags "${VERSION_FLAG} ${GIT_HASH_FLAG} ${GIT_DIRTY_FLAG}" \ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I couldn't find anything to clarify the --unresolved-symbols=ignore-in-object-files
portion of the flag, can you give some background on why these linker options are required for building with CGO specifically for enabling nvidia code?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
vendor package for nvml APIs uses the cgo ldflags here -
Line 5 in 84a9381
// #cgo LDFLAGS: -ldl -Wl,--unresolved-symbols=ignore-in-object-files |
Looks like only certain ldflags are whitelisted by default (golang/go#23749) and hence the build error as mentioned in the summary of this PR.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks, that explains it 👍
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, let's work on getting a test build and confirm that this doesn't affect anything we're not thinking about right now.
@@ -37,14 +37,14 @@ mkdir -p "${SRCPATH}" | |||
ln -s "${TOPWD}/ecs-init" "${SRCPATH}" | |||
cd "${SRCPATH}/ecs-init" | |||
if [[ "$1" == "dev" ]]; then | |||
go build -tags 'development' -ldflags "${VERSION_FLAG} ${GIT_HASH_FLAG} ${GIT_DIRTY_FLAG}" \ | |||
CGO_ENABLED=1 CGO_LDFLAGS_ALLOW='-Wl,--unresolved-symbols=ignore-in-object-files' go build -tags 'development' -ldflags "${VERSION_FLAG} ${GIT_HASH_FLAG} ${GIT_DIRTY_FLAG}" \ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks, that explains it 👍
Summary
address build error
go build github.com/aws/amazon-ecs-init/ecs-init/vendor/github.com/NVIDIA/gpu-monitoring-tools/bindings/go/nvml: invalid flag in #cgo LDFLAGS: -Wl,--unresolved-symbols=ignore-in-object-files
Implementation details
Testing
New tests cover the changes:
Description for the changelog
Licensing
This contribution is under the terms of the Apache 2.0 License: yes