Refactor go builder to stop using go/build

[jonjohnsonjr]

In #303 I added a big warning about GOROOT mismatches between ko and go. I think this is necessary given how ko works, but we should be able to remove that warning if we refactor the go builder a bit.

Historically, ko would attempt to Import every single string in a document to determine whether or not it should build it. If we succeeded, great. If we failed, just ignore it (i.e. we failed open). That code is here.

Per rsc this is bad. If ko embeds knowledge of how to import/build go code in its binary, if that logic changes in go, we have a mismatch. This happened with GOROOT, which is why we're here.

Unfortunately, shelling out to go would be too slow if we needed to do it for every string.

Now that #281 has landed, we no longer look at every string. We only consider ko:// strings, and we expect importing/building those targets will always succeed (i.e. we fail closed). Thus, we shouldn't really need to use go/build.Import. Shelling out to go should be fine, because we only do it a handful of times.

Similarly, we relied on strange heuristics to avoid shelling out to go to get module info than once or using the packages package because it was slow for large numbers of strings.

I think we should revisit some of these decisions, now that we require ko:// everywhere.

If I remember correctly, we should be able to just use packages for everything (it was build for this express purpose), but we should do some careful benchmarking to make sure we don't slow things down unnecessarily.

I'd like some numbers for tekton, knative, and maybe mink before merging any of this.

cc @mattmoor @imjasonh thoughts?

[imjasonh]

I'd like some numbers for tekton, knative, and maybe mink before merging any of this.
@imjasonh thoughts?

+1

What numbers can I provide?

[jonjohnsonjr]

What numbers

A matrix like this would give me confidence to merge:

cold|warm x before|after x project

We'd need the PR to exist before we test it, though. I don't intend to implement this anytime soon, so if someone else wants to jump on it, feel free.

[github-actions]

This issue is stale because it has been open for 90 days with no
activity. It will automatically close after 30 more days of
inactivity. Reopen the issue with /reopen. Mark the issue as
fresh by adding the comment /remove-lifecycle stale.

[benmoss]

i'm slowly taking a look at this 😄 , no promises