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

build: do not produce Windows binaries with netgo build tags #4672

Merged
merged 2 commits into from
Aug 1, 2023

Conversation

rfratto
Copy link
Member

@rfratto rfratto commented Aug 1, 2023

Go 1.19 adds Windows support for the native Go network stack, but doesn't include support for resolving DNS short names.

Other projects, including Prometheus, have updated their build process to exclude the netgo build tags when producing Windows binaries to work around this behavior.

Fixes #4665

@rfratto rfratto requested review from a team as code owners August 1, 2023 12:35
@ptodev
Copy link
Contributor

ptodev commented Aug 1, 2023

I think you mean that it fixes #4665 and not #4465 ?

Go 1.19 adds Windows support for the native Go network stack, but
doesn't include support for resolving DNS short names.

Other projects, including Prometheus, have updated their build process
to exclude the netgo build tags when producing Windows binaries to work
around this behavior.

Fixes grafana#4665.
@rfratto rfratto force-pushed the remove-netgo-windows branch from 91c6de1 to 3f89af3 Compare August 1, 2023 12:40
@rfratto
Copy link
Member Author

rfratto commented Aug 1, 2023

I think you mean that it fixes #4665 and not #4465 ?

🤦 Yes, thank you

dist/grafana-agent-darwin-arm64: GOOS := darwin
dist/grafana-agent-darwin-arm64: GOARCH := arm64
dist/grafana-agent-darwin-arm64: generate-ui
$(PACKAGING_VARS) AGENT_BINARY=$@ $(MAKE) -f $(PARENT_MAKEFILE) agent

# NOTE(rfratto): do not use netgo when building Windows binaries, which
# prevents DNS short names from being resovable. See grafana/agent#4665.
Copy link
Contributor

Choose a reason for hiding this comment

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

Do we need to add a TODO to revert this change when the underlying Go issue is resolved?
golang/go#57757

Copy link
Member Author

Choose a reason for hiding this comment

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

I don't think that's the same issue, is it? I don't see any mention of the hosts file in the issue or the Prometheus issue.

Copy link
Member Author

Choose a reason for hiding this comment

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

I added a generic TODO comment, since I don't think this is the same issue.

Copy link
Contributor

Choose a reason for hiding this comment

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

Ah yes, I think you're right and it's not the same issue. But it's probably another issue which needs to be resolved before we use netgo on windows. Anyway, thank you for adding the TODO.

@rfratto rfratto merged commit 6680f50 into grafana:main Aug 1, 2023
@rfratto rfratto deleted the remove-netgo-windows branch August 1, 2023 12:56
clayton-cornell pushed a commit that referenced this pull request Aug 14, 2023
Go 1.19 adds Windows support for the native Go network stack, but
doesn't include support for resolving DNS short names.

Other projects, including Prometheus, have updated their build process
to exclude the netgo build tags when producing Windows binaries to work
around this behavior.

Fixes #4665.
clayton-cornell pushed a commit that referenced this pull request Aug 14, 2023
Go 1.19 adds Windows support for the native Go network stack, but
doesn't include support for resolving DNS short names.

Other projects, including Prometheus, have updated their build process
to exclude the netgo build tags when producing Windows binaries to work
around this behavior.

Fixes #4665.
@github-actions github-actions bot added the frozen-due-to-age Locked due to a period of inactivity. Please open new issues or PRs if more discussion is needed. label Feb 22, 2024
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Feb 22, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
frozen-due-to-age Locked due to a period of inactivity. Please open new issues or PRs if more discussion is needed.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Agent does not resolve DNS short names on Windows
2 participants