Added bazel in local execution environment

[priyawadhwa]

I added bazel as a builder plugin in the local execution environment.
This required moving DependenciesForArtifact into the build interface;
if people want to write their own plugins, they will need some way of
telling the skaffold watcher what their dependencies are.

fixes #1553

[codecov-io]

Codecov Report

Merging #1662 into master will decrease coverage by 0.71%.
The diff coverage is 10.25%.

@@            Coverage Diff             @@
##           master    #1662      +/-   ##
==========================================
- Coverage   47.41%   46.69%   -0.72%     
==========================================
  Files         122      123       +1     
  Lines        5448     5600     +152     
==========================================
+ Hits         2583     2615      +32     
- Misses       2604     2716     +112     
- Partials      261      269       +8

Impacted Files	Coverage Δ
pkg/skaffold/plugin/environments/gcb/types.go	0% <0%> (ø)	⬆️
pkg/skaffold/build/local/local.go	29.41% <0%> (-22.32%)	⬇️
pkg/skaffold/runner/dev.go	56.52% <0%> (ø)	⬆️
pkg/skaffold/plugin/environments/gcb/desc.go	56.52% <0%> (-2.57%)	⬇️
pkg/skaffold/build/kaniko/types.go	0% <0%> (ø)	⬆️
pkg/skaffold/util/util.go	46.04% <0%> (-2.45%)	⬇️
...kg/skaffold/plugin/environments/gcb/cloud_build.go	0% <0%> (ø)	⬆️
pkg/skaffold/build/plugin/plugin.go	15.06% <0%> (-1.35%)	⬇️
cmd/skaffold/app/cmd/diagnose.go	0% <0%> (ø)	⬆️
pkg/skaffold/build/kaniko/run.go	0% <0%> (ø)	⬆️
... and 11 more

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update acde29d...ff280e7. Read the comment docs.

[balopat]

It's looking good - Now that we introduced the DependenciesForArtifact in the Builder interface, I think we can break out the build.DependenciesForArtifact to the separate implementations now instead of keeping it a centralized switch statement. WDYT?

[priyawadhwa]

I think the switch statement may still be the cleanest option, since build.DependenciesForArtifact applies to the overall "local" builder instead of the different types of artifacts -- is that what you meant?

[balopat]

I think the switch statement may still be the cleanest option, since build.DependenciesForArtifact applies to the overall "local" builder instead of the different types of artifacts -- is that what you meant?

No, what I meant is that the builder itself now should know what the dependencies are for a given artifact. It is weird to see bazel.DependenciesForArtifact calling out to a global build.DependenciesForArtifact where it checks whether the artifact is docker, jib or bazel... Instead the logic should be something like this in bazel.DependenciesForArtifact:

if a.BazelArtifact == nil {
  return nil, fmt.Errorf("%s is not a bazel artifact", a)  
}
return bazel.GetDependencies(ctx, a.Workspace, a.BazelArtifact)

[priyawadhwa]

@balopat thanks for clarifying, that should be done now!

[balopat]

Great, almost there - I think we should go all the way, we can just nuke deps.go.

[priyawadhwa]

So removing deps.go would cause code duplication of the switch statement since we need it here for the local builder and here for the diagnose command.

I'm leaning towards keeping deps.go, WDYT?

[balopat]

This looks great!
1.) local.go will go away with all the builders turning into plugins. We can keep the switch logic there for now as it's an "obsolete" builder
2.) diagnose should actually delegate the call now to the builder that the runner created!

[priyawadhwa]

Ahh got it that makes sense, thanks! I updated diagnose to do that :)