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 #530 Hanging behavior with ASDF installed Go on Darwin #548

Merged
merged 1 commit into from
Dec 15, 2021

Conversation

StevenACoffman
Copy link
Contributor

So this resolves the issue with ko hanging on Darwin when Go is installed using ASDF, however, I'm not sure if this is the final fix that still works for everyone else.

@jonjohnsonjr
Signed-off-by: Steve Coffman steve@khanacademy.org

Signed-off-by: Steve Coffman <steve@khanacademy.org>
Copy link
Contributor Author

@StevenACoffman StevenACoffman left a comment

Choose a reason for hiding this comment

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

Are there places where we actually want dir to be . instead of ""?

There are still a few more references to this g.dir where we don't do the song and dance like:
https://github.com/google/ko/blob/1ec35c80cf6964d950868a626374743d75cae169/pkg/build/gobuild.go#L690

Also, I wonder about just lifting the song and dance to where the creation of the struct in the first place:
https://github.com/google/ko/blob/1ec35c80cf6964d950868a626374743d75cae169/pkg/build/gobuild.go#L131

Or here:
https://github.com/google/ko/blob/1ec35c80cf6964d950868a626374743d75cae169/pkg/build/gobuild.go#L151

@jonjohnsonjr
Copy link
Collaborator

There are still a few more references to this g.dir where we don't do the song and dance

I think it's fine to leave those alone for now. This seems like a weird interaction with packages.Load that we need to workaround, but just execing go directly works as expected. I'll want to refactor some of this stuff regardless, so we can just merge this to fix things until I can look at it more closely.

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.

2 participants