Skip to content

Fix imports of generated go proto bindings - #40752

Closed
ndbls wants to merge 1 commit into
tensorflow:masterfrom
ndbls:fix_imports
Closed

Fix imports of generated go proto bindings#40752
ndbls wants to merge 1 commit into
tensorflow:masterfrom
ndbls:fix_imports

Conversation

@ndbls

@ndbls ndbls commented Jun 24, 2020

Copy link
Copy Markdown

Fixes

#39744

Summary

After running the following in accordance with the installation instructions for go:

$ go get -d github.com/tensorflow/tensorflow/tensorflow/go
$ go generate github.com/tensorflow/tensorflow/tensorflow/go/op
$ go test github.com/tensorflow/tensorflow/tensorflow/go

The first command generates error message unrelated to this PR, a fix for which is ready to pull. That error message is relatively low impact and should not impact the functionality of the library after the go generate command is run. However, the third command generates the following error:

$ go test github.com/tensorflow/tensorflow/tensorflow/go
../../../go/src/github.com/tensorflow/tensorflow/tensorflow/go/saved_model.go:25:2: cannot find package "github.com/tensorflow/tensorflow/tensorflow/go/core/protobuf/for_core_protos_go_proto" in any of:
	/home/abls/go/src/github.com/tensorflow/tensorflow/tensorflow/go/vendor/github.com/tensorflow/tensorflow/tensorflow/go/core/protobuf/for_core_protos_go_proto (vendor tree)
	/usr/local/go/src/github.com/tensorflow/tensorflow/tensorflow/go/core/protobuf/for_core_protos_go_proto (from $GOROOT)
	/home/abls/go/src/github.com/tensorflow/tensorflow/tensorflow/go/core/protobuf/for_core_protos_go_proto (from $GOPATH)

This is caused by a bad import that seemed to be from an accident during a refactor. The problematic import that occurs in several files is:

import corepb "github.com/tensorflow/tensorflow/tensorflow/go/core/protobuf/for_core_protos_go_proto"

This line is supposed to point to the generated files, however the generated files are actually placed in github.com/tensorflow/tensorflow/tensorflow/go/core/core_protos_go_proto due to the following line in several proto files in tensorflow/core/protobuf read by protoc during generation:

option go_package = "github.com/tensorflow/tensorflow/tensorflow/go/core/core_protos_go_proto";

The fix is simply to correct the import lines to point to the actual location of the files.

@google-ml-butler google-ml-butler Bot added the size:XS CL Change Size: Extra Small label Jun 24, 2020
@gbaned gbaned self-assigned this Jun 24, 2020
@gbaned
gbaned requested a review from jhseu June 24, 2020 11:52
philpearl
philpearl approved these changes Jun 24, 2020
View reviewed changes

@philpearl philpearl left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Review from random stranger here, which unfortunately won't help this get merged. But yes! yes! yes! Thank-you!

@google-ml-butler google-ml-butler Bot added kokoro:force-run Tests on submitted change ready to pull PR ready for merge process labels Jun 24, 2020
@kokoro-team kokoro-team removed kokoro:force-run Tests on submitted change labels Jun 24, 2020
@google-ml-butler google-ml-butler Bot added the kokoro:force-run Tests on submitted change label Jun 24, 2020
@kokoro-team kokoro-team removed the kokoro:force-run Tests on submitted change label Jun 24, 2020
@gbaned gbaned added ready to pull PR ready for merge process and removed ready to pull PR ready for merge process labels Jul 2, 2020
@jhseu

jhseu commented Jul 22, 2020

Copy link
Copy Markdown
Contributor

Needed to change this a bit to match the BUILD rule, which is required for bazel compatibility:
e5e495d

@jhseu jhseu closed this Jul 22, 2020
@rubens21

rubens21 commented Aug 29, 2020

Copy link
Copy Markdown

Needed to change this a bit to match the BUILD rule, which is required for bazel compatibility:
e5e495d

@jhseu Any idea when it will be in a release?

@rubens21 rubens21 mentioned this pull request Aug 29, 2020
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

2 more reviewers
@jhseu jhseu jhseu approved these changes
@philpearl philpearl philpearl approved these changes
Reviewers whose approvals may not affect merge requirements

Assignees

@gbaned gbaned

Labels

cla: yes ready to pull PR ready for merge process size:XS CL Change Size: Extra Small

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

7 participants

@ndbls @jhseu @rubens21 @philpearl @googlebot @kokoro-team @gbaned