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

Remove replace directives from go.mod #7024

Closed
mmannerm opened this issue Sep 13, 2024 · 4 comments · Fixed by #7056
Closed

Remove replace directives from go.mod #7024

mmannerm opened this issue Sep 13, 2024 · 4 comments · Fixed by #7056

Comments

@mmannerm
Copy link

What is the underlying problem you're trying to solve?

go run github.com/open-policy-agent/opa@v0.68.0 or go install github.com/open-policy-agent/opa@v0.68.0 will not work and results in to an error

go: downloading github.com/open-policy-agent-opa v0.68.0
go: github.com/open-policy-agent/opa@v0.68.0 (in github.com/open-policy-agent/opa@v0.68.0):
    The go.mod file for the module providing named packages contains one or
    more replace directives. It must not contain directives that would cause
    it to be interpreted differently than if it were the main module.

The replace statement in question is go.mod#L120.

replace github.com/golang/glog => ./build/replacements/github.com/golang/glog

Describe the ideal solution

The ideal solution would be that you can go run the opa policy agent directly via go run or you can install it directly with go install instead of relying on package manager.

Describe a "Good Enough" solution

Creating RPM packages and other packages for easier installation of opa executable is more work than fixing the replace statement.

Additional Context

The golang team is not willing to support replace statements with go install or go run: golang/go#44840

@anderseknert
Copy link
Member

Hi there! And thanks for filing this issue. And I agree, that would be nice. The trade-off we're looking at seems to be between supporting go install vs. providing a poor experience for Windows users on the official binaries. And supporting our distributed executables are likely always going to be prioritized higher than go install support. But obviously, if we can do both, we should!

@jamietanna
Copy link
Contributor

Related: StyraInc/regal#490

@anderseknert
Copy link
Member

anderseknert commented Sep 24, 2024

Indeed. In that case it was a no-brainer to just get rid of the replace, while it solves (or at least solved!) an actual issue here. But looking at the git history, that replace is from 2022, and I don't think anyone looked at it since then. So it could very well be that the issue has been fixed upstream. I don't have much time to look into it this week, and no windows machine to test with 😅 But if someone has the bandwidth to investigate, that would be much appreciated!

@srenatus
Copy link
Contributor

srenatus commented Sep 24, 2024

But looking at the git history, that replace is from 2022, and I don't think anyone looked at it since then. So it could very well be that the issue has been fixed upstream.

I'll have a quick look.

This looks promising -- two months ago 😎 -- golang/glog@9730314

srenatus added a commit to srenatus/opa that referenced this issue Sep 24, 2024
Just recently, glog introduced a fix for the potentially very expensive network
call on windows that had been troubling us before:

golang/glog@9730314

It's become the v1.2.2 release.

The release is now in vendor, and the replacement has been removed.

Fixes open-policy-agent#7024.

Signed-off-by: Stephan Renatus <stephan@styra.com>
srenatus added a commit that referenced this issue Sep 24, 2024
Just recently, glog introduced a fix for the potentially very expensive network
call on windows that had been troubling us before:

golang/glog@9730314

It's become the v1.2.2 release.

The release is now in vendor, and the replacement has been removed.

Fixes #7024.

Signed-off-by: Stephan Renatus <stephan@styra.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants