Fix proto gen #108365
Fix proto gen #108365
Conversation
Change-Id: I2a563514955d7fc7559ceb7afb73df08ace8fd8b
k8s-ci-robot
added
release-note-none
| @@ -260,6 +260,7 @@ func Run(g *Generator) { | |||
| if len(g.Conditional) > 0 { | |||
| fmt.Fprintf(buf, "// +build %s\n\n", g.Conditional) | |||
| } | |||
| buf.Write(boilerplate) | |||
There was a problem hiding this comment.
this was incorrectly removed in #108358
There was a problem hiding this comment.
should we create an issue to have some tests for these files ?
There was a problem hiding this comment.
with the fix to actually write to the checked in files, this bug was apparent (the copyright header wasn't present)
k8s-ci-robot
added
approved
| // read input | ||
| request := command.Read() | ||
|
|
||
| // if we're given paths as inputs, generate .pb.go files based on those paths |
There was a problem hiding this comment.
this is conditional because some generation scripts (kms and runtime) invoke without full paths to proto files, and some generation scripts (like the API type generation) pass full paths
|
|
||
| // if we're given paths as inputs, generate .pb.go files based on those paths | ||
| for _, file := range request.FileToGenerate { | ||
| if strings.Contains(file, "/") { |
There was a problem hiding this comment.
Contains or prefix? strings.HasPrefix(file, "/")
There was a problem hiding this comment.
contains. when given paths, it looks like k8s.io/kubernetes/vendor/k8s.io/api/core/v1/generated.proto
There was a problem hiding this comment.
when given files instead of paths, it's just api.proto, service.proto, etc
There was a problem hiding this comment.
ah, is not about relative or absolute paths, forget it
There was a problem hiding this comment.
should we put in the comment, an example of these expected paths to make it easier for other people looking at this code ?
|
/lgtm |
k8s-ci-robot
added
the
lgtm
|
/retest |
|
/approve that line did look suspicious :) thanks Jordan! |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: dims, liggitt The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
|
/triage accepted |
k8s-ci-robot
added
triage/accepted
What type of PR is this?
/kind bug
What this PR does / why we need it:
#108358 made proto files contain full go import paths, but broke actual generation of the generated.pb.go files.
This PR fixes that by telling gogo to generate pb.go files next to the proto files when we're given full paths as input.
Which issue(s) this PR fixes:
Fixes #108358
Special notes for your reviewer:
Does this PR introduce a user-facing change?