Skip to content

travis: enforce generated code, add 1.x #521

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.

Sign up for GitHub

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

Merged
merged 1 commit into from
Feb 21, 2018
Merged

travis: enforce generated code, add 1.x #521

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
19 changes: 17 additions & 2 deletions .travis.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,14 +5,29 @@ go:
- 1.7.x
- 1.8.x
- 1.9.x
- 1.10.x
- 1.x

install:
- go get -v -d -t github.com/golang/protobuf/...
- curl -L https://github.com/google/protobuf/releases/download/v3.5.1/protoc-3.5.1-linux-x86_64.zip -o /tmp/protoc.zip
- unzip /tmp/protoc.zip -d $HOME/protoc
- unzip /tmp/protoc.zip -d "$HOME"/protoc
- mkdir -p "$HOME"/src && ln -s "$HOME"/protoc "$HOME"/src/protobuf

env:
- PATH=$HOME/protoc/bin:$PATH

script:
- make all test
- make all
- make regenerate
Copy link
Contributor

Choose a reason for hiding this comment

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

This doesn't seem right--if we run regenerate before test, the golden tests will always pass even if something has unexpectedly changed.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'm happy to rearrange this, but note that if anything at all changes as a result of make regenerate, the stanza immediately following will fail.

Copy link
Contributor

Choose a reason for hiding this comment

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

Ahh, I see what you're doing; I hadn't quite followed the purpose of that stanza.

FWIW, the tests in protoc-gen-go should already pass independent of the Go version (they ignore the compressed portion of the generated files). Would it make more sense to fix any of the tests which don't have that behavior than to change the Travis config?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

FWIW, the tests in protoc-gen-go should already pass independent of the Go version (they ignore the compressed portion of the generated files). Would it make more sense to fix any of the tests which don't have that behavior than to change the Travis config?

Again, while those tests might pass, the files will have changed and the aforementioned stanza will fail.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Also note that no tests are being skipped here; make test is run unconditionally.

Copy link
Contributor

Choose a reason for hiding this comment

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

I guess my question is: If this is doing something that make test doesn't already do, should we just fix make test?

("Fixing make test is hard" is a valid answer. :) )

# TODO(tamird): When https://github.com/travis-ci/gimme/pull/130 is
# released, make this look for "1.x".
- if [[ "$TRAVIS_GO_VERSION" == 1.10* ]]; then
if [[ "$(git status --porcelain 2>&1)" != "" ]]; then
git status >&2;
git diff -a >&2;
exit 1;
fi;
echo "git status is clean.";
fi;
- make test
3 changes: 1 addition & 2 deletions Make.protobuf
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,5 @@
#
# include $(GOROOT)/src/pkg/github.com/golang/protobuf/Make.protobuf

%.pb.go: %.proto
%.pb.go: %.proto
protoc --go_out=. $<

1 change: 0 additions & 1 deletion Makefile
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,6 @@
# (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE
# OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.


all: install

install:
Expand Down
4 changes: 2 additions & 2 deletions README.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -114,9 +114,9 @@ Consider file test.proto, containing
```proto
syntax = "proto2";
package example;

enum FOO { X = 17; };

message Test {
required string label = 1;
optional int32 type = 2 [default=77];
Expand Down
1 change: 0 additions & 1 deletion proto/Makefile
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,6 @@ install:
test: install generate-test-pbs
go test


generate-test-pbs:
make install
make -C test_proto
Expand Down
4 changes: 2 additions & 2 deletions protoc-gen-go/descriptor/Makefile
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,5 +33,5 @@
# at src/google/protobuf/descriptor.proto
regenerate:
@echo WARNING! THIS RULE IS PROBABLY NOT RIGHT FOR YOUR INSTALLATION
cp $(HOME)/src/protobuf/src/google/protobuf/descriptor.proto .
protoc --go_out=../../../../.. -I$(HOME)/src/protobuf/src $(HOME)/src/protobuf/src/google/protobuf/descriptor.proto
cp $(HOME)/src/protobuf/include/google/protobuf/descriptor.proto .
protoc --go_out=../../../../.. -I$(HOME)/src/protobuf/include $(HOME)/src/protobuf/include/google/protobuf/descriptor.proto
410 changes: 244 additions & 166 deletions protoc-gen-go/descriptor/descriptor.pb.go
View file
Open in desktop

Large diffs are not rendered by default.

27 changes: 25 additions & 2 deletions protoc-gen-go/descriptor/descriptor.proto
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,7 @@ option java_package = "com.google.protobuf";
option java_outer_classname = "DescriptorProtos";
option csharp_namespace = "Google.Protobuf.Reflection";
option objc_class_prefix = "GPB";
option cc_enable_arenas = true;

// descriptor.proto must be optimized for speed because reflection-based
// algorithms don't work during bootstrapping.
Expand Down Expand Up @@ -225,6 +226,26 @@ message EnumDescriptorProto {
repeated EnumValueDescriptorProto value = 2;

optional EnumOptions options = 3;

// Range of reserved numeric values. Reserved values may not be used by
// entries in the same enum. Reserved ranges may not overlap.
//
// Note that this is distinct from DescriptorProto.ReservedRange in that it
// is inclusive such that it can appropriately represent the entire int32
// domain.
message EnumReservedRange {
optional int32 start = 1; // Inclusive.
optional int32 end = 2; // Inclusive.
}

// Range of reserved numeric values. Reserved numeric values may not be used
// by enum values in the same enum declaration. Reserved ranges may not
// overlap.
repeated EnumReservedRange reserved_range = 4;

// Reserved enum value names, which may not be reused. A given name may only
// be reserved once.
repeated string reserved_name = 5;
}

// Describes a value within an enum.
Expand Down Expand Up @@ -396,10 +417,12 @@ message FileOptions {
// determining the namespace.
optional string php_namespace = 41;

// The parser stores options it doesn't recognize here. See above.
// The parser stores options it doesn't recognize here.
// See the documentation for the "Options" section above.
repeated UninterpretedOption uninterpreted_option = 999;

// Clients can define custom options in extensions of this message. See above.
// Clients can define custom options in extensions of this message.
// See the documentation for the "Options" section above.
extensions 1000 to max;

reserved 38;
Expand Down
4 changes: 2 additions & 2 deletions protoc-gen-go/plugin/Makefile
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -34,9 +34,9 @@
# Also we need to fix an import.
regenerate:
@echo WARNING! THIS RULE IS PROBABLY NOT RIGHT FOR YOUR INSTALLATION
cp $(HOME)/src/protobuf/src/google/protobuf/compiler/plugin.proto .
cp $(HOME)/src/protobuf/include/google/protobuf/compiler/plugin.proto .
protoc --go_out=Mgoogle/protobuf/descriptor.proto=github.com/golang/protobuf/protoc-gen-go/descriptor:../../../../.. \
-I$(HOME)/src/protobuf/src $(HOME)/src/protobuf/src/google/protobuf/compiler/plugin.proto
-I$(HOME)/src/protobuf/include $(HOME)/src/protobuf/include/google/protobuf/compiler/plugin.proto

restore:
cp plugin.pb.golden plugin.pb.go
Expand Down