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 build constraints since windows does not depend on cgo #47

Merged
merged 1 commit into from
May 20, 2023

Conversation

gapra-msft
Copy link
Contributor

@gapra-msft gapra-msft commented Feb 1, 2023

The build constraints in ieproxy_unix and pac_unix are incorrect.

The windows implementation does not depend on cgo, only the darwin one does.

Based off the following logic this should be the build constraint on this file, otherwise Windows architectures with cgo disabled by default will not build correctly, resulting in an error like this

# github.com/mattn/go-ieproxy
../../../go/pkg/mod/github.com/mattn/[email protected]/ieproxy_windows.go:22:6: getConf redeclared in this block
	../../../go/pkg/mod/github.com/mattn/[email protected]/ieproxy_unix.go:6:6: other declaration of getConf
../../../go/pkg/mod/github.com/mattn/[email protected]/ieproxy_windows.go:28:6: reloadConf redeclared in this block
	../../../go/pkg/mod/github.com/mattn/[email protected]/ieproxy_unix.go:10:6: other declaration of reloadConf
../../../go/pkg/mod/github.com/mattn/[email protected]/ieproxy_windows.go:133:6: overrideEnvWithStaticProxy redeclared in this block
	../../../go/pkg/mod/github.com/mattn/[email protected]/ieproxy_unix.go:14:6: other declaration of overrideEnvWithStaticProxy
../../../go/pkg/mod/github.com/mattn/[email protected]/pac_windows.go:9:29: ProxyScriptConf.findProxyForURL redeclared in this block
	../../../go/pkg/mod/github.com/mattn/[email protected]/pac_unix.go:6:29: other declaration of findProxyForURL

!(windows || (darwin && cgo)) = !windows && !(darwin && cgo) = !windows && (!darwin || !cgo) = !windows,!darwin !cgo

//go:build !windows && (!darwin || !cgo)
// +build !windows,!darwin !cgo

I believe this should resolve #45 as well

@gesellix
Copy link

This would be great to be merged and released. I'm already using the patch in my projects, but I'd prefer not to use vendored modules :)

@gesellix
Copy link

@mattn do you think you could merge and release this change?

@mattn mattn merged commit 9ef99e2 into mattn:master May 20, 2023
@mattn
Copy link
Owner

mattn commented May 20, 2023

Thank you

@gapra-msft gapra-msft deleted the gapra/fixbuildconstraints branch May 22, 2023 16:20
andrzej-stencel added a commit to andrzej-stencel/opentelemetry-collector-releases that referenced this pull request Sep 25, 2024
…package

The replacement for the package `github.com/mattn/go-ieproxy` is currently in effect
and replaces `v0.0.11` of the package with version `v0.0.1`.
I believe this replacement is not necessary anymore,
as the issue mattn/go-ieproxy#45 was fixed mattn/go-ieproxy#47
in v0.0.11.
andrzej-stencel added a commit to open-telemetry/opentelemetry-collector-releases that referenced this pull request Oct 7, 2024
…package (#678)

The replacement for the package `github.com/mattn/go-ieproxy` is currently in effect
and replaces `v0.0.11` of the package with version `v0.0.1`.
I believe this replacement is not necessary anymore,
as the issue mattn/go-ieproxy#45 was fixed mattn/go-ieproxy#47
in v0.0.11.

Co-authored-by: Alex Boten <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

findProxyForURL with cgo on windows
3 participants