Skip to content

Conversation

@maaarcelino
Copy link
Contributor

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"

@maaarcelino maaarcelino changed the title Add support for conditional static linking of C lib to builds feat: Add support for conditional static linking of C lib to builds Oct 16, 2023
@github-actions github-actions bot added the feat label Oct 16, 2023
@github-actions
Copy link

github-actions bot commented Oct 16, 2023

⏱️ Benchmark results

Comparing with 7c634dc

  • Glob-8 ns/op: 206.8 ⬆️ 56.10% increase vs. 7c634dc

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" {
Copy link
Member

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

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done in 50caf3a

Copy link
Member

@erezrokah erezrokah left a 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?

Copy link
Member

@erezrokah erezrokah left a 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 🚀

@maaarcelino maaarcelino merged commit 7c27065 into main Oct 16, 2023
@maaarcelino maaarcelino deleted the feat/static-linking branch October 16, 2023 09:19
kodiakhq bot pushed a commit that referenced this pull request Oct 16, 2023
🤖 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).
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants