-
Notifications
You must be signed in to change notification settings - Fork 119
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.
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
enable cgo for nvidia code #214
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 -
amazon-ecs-init/ecs-init/vendor/github.com/NVIDIA/gpu-monitoring-tools/bindings/go/nvml/bindings.go
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