-
Notifications
You must be signed in to change notification settings - Fork 26
feat: Add support for conditional static linking of C lib to builds #1292
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
Conversation
serve/package.go
Outdated
| return nil, err | ||
| } | ||
| ldFlags := fmt.Sprintf("-s -w -X %s/plugin.Version=%s", importPath, pluginVersion) | ||
| if getEnvOrDefault("STATIC_LINKING_ENABLED", "0") == "1" { |
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.
Can we make this into a plugin option? See example https://github.com/cloudquery/cloudquery/blob/9e916090240a33ddf5a0163b950bf8fc1a8873ed/plugins/destination/sqlite/main.go#L23
Not sure if we want withStaticLinking bool option or withAdditionalLDFlags []string flag. Maybe withStaticLinking is good for now
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.
Done in 50caf3a
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.
The fix looks good, can we use a plugin option instead of an env variable?
erezrokah
left a comment
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 for the follow up 🚀
🤖 I have created a release *beep* *boop* --- ## [4.13.0](v4.12.5...v4.13.0) (2023-10-16) ### Features * Add support for conditional static linking of C lib to builds ([#1292](#1292)) ([7c27065](7c27065)) * Support Delete Record ([#1282](#1282)) ([1f0a603](1f0a603)) ### Bug Fixes * **deps:** Update github.com/cloudquery/arrow/go/v14 digest to dbcb149 ([#1291](#1291)) ([7c634dc](7c634dc)) * **deps:** Update module github.com/cloudquery/plugin-pb-go to v1.12.3 ([#1289](#1289)) ([3e063bc](3e063bc)) --- This PR was generated with [Release Please](https://github.com/googleapis/release-please). See [documentation](https://github.com/googleapis/release-please#release-please).
This adds support for conditionally linking C libs when building plugin executables. CGO_ENABLED plugins (sqlite and duckdb) fail to run on Alpine Linux distributions because of missing C libraries needed to execute the code. See cloudquery/cloudquery#14465.
The fix is to include those missing libraries into the executable that is created by
go build:CGO_ENABLED=1 go build -ldflags="-linkmode external -extldflags=-static"