Skip to content
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

fix: Add CGO_ENABLED=0 flag to Dataflow example #4406

Merged
merged 7 commits into from
Oct 10, 2024

Conversation

damondouglas
Copy link
Contributor

@damondouglas damondouglas commented Sep 24, 2024

Description

Fixes #4405 by adding CGO_ENABLED=0 to build instruction of Dataflow example.

Checklist

@damondouglas damondouglas requested review from a team as code owners September 24, 2024 23:37
@product-auto-label product-auto-label bot added the samples Issues that are directly related to samples. label Sep 24, 2024
@damondouglas damondouglas changed the title Bump go/Beam SDK Versions; Add CGO_ENABLED=0 flag Add CGO_ENABLED=0 flag to Dataflow example Sep 24, 2024
@iennae iennae changed the title Add CGO_ENABLED=0 flag to Dataflow example fix: Add CGO_ENABLED=0 flag to Dataflow example Sep 25, 2024
@iennae iennae added the kokoro:force-run Add this label to force Kokoro to re-run the tests. label Sep 25, 2024
@kokoro-team kokoro-team removed the kokoro:force-run Add this label to force Kokoro to re-run the tests. label Sep 25, 2024
@iennae
Copy link
Contributor

iennae commented Sep 25, 2024

@damondouglas can you provide a bit more context on this change?

This disables the mechanism for working with c code which might be the right path if no c libraries are needed, and the reason it's failing in some places might be due to the different environments (containers or on a mac where glibc libraries aren't available). I'm a little worried this might cause a potential issue based on assumptions about where users might run the code. I haven't dug into the existing code to verify what is needed so if you can give some feedback on the context that would be reassuring to merge this.

@davidcavazos
Copy link
Contributor

I'm not too familiar with the Go compilation process and how Cgo affects it. From a quick search, it looks like Cgo allows access to C's stdlib in the target environment, which is useful in some targets like Android. This, however, introduces stdlib as a dynamically linked library, so the resulting Go binary is no longer statically isolated (one of the best features of Go). From some reading, it looks like if Cgo is enabled, it'll use stdlib functions, but if it's disabled then the Go compiler introduces its own versions of those functions and keeps the binary completely independent from the host system.

Here's a great blog post that goes into some more detail.

From what I can gather, this is not only safe, but will guarantee consistent binaries that don't depend on the system stdlib (which can be different on different operating systems and Docker images like alpine). I'm actually a bit surprised that Cgo is enabled by default, it would seem like a feature that should be opt-in only if you need it. It is (inconsistently) disabled by default when cross-compiling.

Copy link
Collaborator

@telpirion telpirion left a comment

Choose a reason for hiding this comment

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

I don't believe that these samples include any C code. Let's go ahead and set this flag.

@telpirion telpirion merged commit 2756e38 into GoogleCloudPlatform:main Oct 10, 2024
9 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
samples Issues that are directly related to samples.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

GLIB not found error when running Go Dataflow Flex template example
5 participants