-
-
Notifications
You must be signed in to change notification settings - Fork 61
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
root expression: Use pwd argument to parse go.mod #93
Conversation
This will ensure that the Go version selector picks the very latest compatible compiler version.
default.nix
Outdated
@@ -42,7 +42,7 @@ buildGoApplication { | |||
--fish <($out/bin/gomod2nix completion fish) \ | |||
--zsh <($out/bin/gomod2nix completion zsh) | |||
'' + '' | |||
wrapProgram $out/bin/gomod2nix --prefix PATH : ${lib.makeBinPath [ go ]} | |||
wrapProgram $out/bin/gomod2nix --prefix PATH : $(dirname $(which go)) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why not just remove the wrapper script then?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We still need go
on $PATH
since we call out to it, but come to think of it that could just as well be a compiler flag that so we could avoid the wrapper entirely.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
But when we run the gomod2nix
binary in shell, it'll find the go
in PATH
anyway, right?
I've been run the binary directly without problem, it'll execute the go in my system environment.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not right now, no. It will use the wrapped Go, we could change it to suffix path instead of prefixing though.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think I'll do that, it's more in line with what you would expect as a user.
It's strictly less pure but more pragmatical.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I mean the binary built with go build
directly, works out of the box, as long as the system PATH
contains the go
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I've added a commit suffixing $PATH.
As suffixing makes gomod2nix inherit any system Go compiler.
@@ -42,7 +42,7 @@ buildGoApplication { | |||
--fish <($out/bin/gomod2nix completion fish) \ | |||
--zsh <($out/bin/gomod2nix completion zsh) | |||
'' + '' | |||
wrapProgram $out/bin/gomod2nix --prefix PATH : ${lib.makeBinPath [ go ]} | |||
wrapProgram $out/bin/gomod2nix --suffix PATH : $(dirname $(which go)) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Still confused by this wrapping logic, $(which go)
just find the go from current PATH
, and we put the path back into PATH
, what's the point of that?
This will ensure that the Go version selector picks the very latest compatible compiler version.
This is a potential alternative fix to #91, and I think worthwhile regardless of #92.
cc @yihuang