Add note to README on responsible CI usage. #765

edwarnicke · 2018-11-28T13:54:55Z

This is in the hopes of easing folks hitting #763 in getting
to a proper use of CI.

such as using append where it makes the code shorter and more natural.

It has almost zero impact on performance in practice, and permits this code to keep working with Go 1.4. Fixes golang#146.

Signed-off-by: David Symonds <dsymonds@golang.org>

…fields. Fixes golang#148.

Clarify the unsupported nature of this interface in the README. Fixes golang#147.

Signed-off-by: David Symonds <dsymonds@golang.org>

Like App Engine, GopherJS does not support unsafe. This change adds a 'js' build tag (set by GopherJS) alongside the existing 'appengine' tags. Fixes golang#154.

…required field. Signed-off-by: David Symonds <dsymonds@golang.org>

Enums used in maps is still broken, but commented-out test data is now there, together with TODOs. Fixes golang#164.

…ion. See grpc/grpc-go#642 for the corresponding grpc-go change. Signed-off-by: David Symonds <dsymonds@golang.org>

Signed-off-by: David Symonds <dsymonds@golang.org>

Unmarshaling Any is a TODO. Fixes golang#170.

Methods that manipulate protos with extensions will now take proto.Message instead of the internal extendableProto interface. A ClearExtensions method is added to clear all extensions on protos.

While this check could be done before marshalling, it shouldn't occur frequently and thus would slow down proto encoding on average.

Turn generated message struct field XXX_Extensions map[int32]Extension into an embedded proto.InternalExtensions struct InternalExtensions is a struct without any exported fields and methods. This effectively makes the representation of the extension map private. The proto package can access InternalExtensions by checking that the generated struct has the method 'extmap() proto.InternalExtensions'. Also lock accesses to the extension map. This change bumps the Go protobuf generated code version number. Any .pb.go files generated with this version of the proto package or later will require this version or later of the proto package to compile.

When the new V2 generated extension format was introduced, we mistakenly dropped support for comparing V1 generated extensions for equality. Add that back.

This is meant to fix golang#188. There are no tests because we don't guarantee that we're going to maintain this behavior in the future. Fixes golang#188

Otherwise invalid JSON can be produced.

Regenerated .pb.go files

…or bytes

Bump support package version number from 2 to 3.

Add tests for merging a map field. When src contains a duplicate key, its value message replaces (not merges with) the value message in dst.

// GetAllExtensionDescs returns a slice of all the descriptors // extensions present in pb. // // If an extension is not registered, a descriptor with only the // 'field' value set will // // be returned instead of a full descriptor. // // The returned slice is not guaranteed to be in any given order. // func GetAllExtensionDescs(pb Message) (extensions []*ExtensionDesc, // err error)

A test depends on the new 'Cute' field.

Don't generate identifiers that conflict with predeclared identifiers, prepend with "_" instead. Signed-off-by: Arjun Sreedharan <asreedharan@pivotal.io>

…names (golang#724) Permit creating a generated package named e.g., "string". Apply disambiguation to prevent creating a local import name that conflicts with predefined identifiers; importing package "string" will be done as: import string1 "string"

Avoid turning comments like "//////" into "// ////".

Parse floating-point default values and format them with fmt.Sprint. Eliminates a minor point of inconsistency with the v2 generator.

Convert all the import_public protos to proto2 to allow testing more features. Add usage of groups, extensions, and default values.

Groups were being excluded from the set of messages we generate forwarding declarations for. Include them.

Test the following case: File 1 defines a symbol S. File 2 publicly imports file 1. File 3 imports file 2 and uses S. All files in different Go packages: P1, P2, P3. Note that the .pb.go for file 3 could import P1 or P2, since the symbol is defined in both.

Consider this case: File 1 defines M. File 2 publicly imports file 1. File 3 imports file 2, and references M. Each file is in a different Go package: P1, P2, P3. Should the generated Go code for file 3 reference P1.M, or P2.M? The two should be equivalent, since file 2 will contain a forwarding declaration such as "type M = P1.M". Historically, we've gone with the latter (P2.M). This does make sense: A generated file only imports the packages of the files that it directly imports. However, this does have some mildly surprising effects. If File 3 imports files 2a and 2b, each of which publicly imports file 1, we need to arbitrarily pick one of P2a.M or P2b.M. (Admittedly an obscure case.) Simplify the generator a little bit (and, more importantly, make it consistent with the v2 generator) and change to referencing public imports directly.

Despite the naming, these are "internal-only" functions that are intended to only be called from generated code for MessageSets. Furthermore, MessageSets are themselves a deprecated feature from proto1 days, such that descriptor.proto even warns against their use since the initial open-source release of protocol buffers in 2008. Within Google, there are no direct usages of these functions, and all existing usages of MessageSets go through the new table-driven implementation. In addition to deprecating the {Unm,M}arshalMessageSet{JSON} top-level functions, also modify the generator to stop emitting MarshalJSON and UnmarshalJSON methods for messages sets. The UnmarshalJSON method is not implemented and the MarshalJSON method does not seem to be called anywhere in Google (verified by making the method panic and doing a global test). The jsonpb package continues to work with MessageSets. I should note that when the table-driven implementation was open sourced in late 2017 (see 8cc9e46), it accidentally removed generation of the Marshal and Unmarshal method. However, no one seemed to have noticed that those methods were no longer generated.

…g#746) The current API represents scalar extension fields as *T and repeated extension fields as []T. However, this is not an accurate reflection of the protobuf data model. For scalars, pointers are usually used to represent nullability. However, in the case of extension scalars, there is no need to do so since presence information is captured by checking whether the field is in the extension map. For this reason, presence on extension scalars is not determined by checking whether the returned pointer is nil, but whether HasExtension reports true. For repeated fields, using []T means that the returned value is only a partially mutable value. You can swap out elements, but you cannot change the length of the original field value. On the other hand, the reflective API provides methods on repeated field values that permit operations that do change the length. Thus, using *[]T is a closer match to the protobuf data model. This CL changes the implementation of extension fields to always store T for scalars, and *[]T for repeated fields. However, for backwards compatibility, the API continues to provide *T and []T. In theory, this could break anyone that relies on memory aliasing for *T. However, this is unlikely since: * use of extensions themselves are relatively rare * if extensions are used, it is recommended practice to use a message as the field extension and not a scalar * relying on memory aliasing is generally not a good idiom to follow. The expected pattern is to call SetExtension to make it explicit that a mutation is happening on the message. * analysis within Google demonstrates that no one is relying on this behavior.

Update all proto files that is obtained from the upstream repository to v3.6.1.

…olang#760) The marshaler, unmarshaler, and sizer functions are unused ever since the underlying implementation was switched to be table-driven. Change the function to only return the wrapper structs. This change: * enables generated protos to drop dependencies on certain proto types * reduces the size of generated protos * simplifies the implementation of oneofs in protoc-gen-go Updates golang#708

This is in the hopes of easing folks hitting golang#763 in getting to a proper use of CI. Signed-off-by: Ed Warnicke <hagbard@gmail.com>

googlebot · 2018-11-28T13:54:58Z

Thanks for your pull request. It looks like this may be your first contribution to a Google open source project (if not, look below for help). Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

📝 Please visit https://cla.developers.google.com/ to sign.

Once you've signed (or fixed any issues), please reply here (e.g. I signed it!) and we'll verify it.

What to do if you already signed the CLA

Individual signers

It's possible we don't have your GitHub username or you're using a different email address on your commit. Check your existing CLA data and verify that your email is set on your git commits.

Corporate signers

Your company has a Point of Contact who decides which employees are authorized to participate. Ask your POC to be added to the group of authorized contributors. If you don't know who your Point of Contact is, direct the Google project maintainer to go/cla#troubleshoot (Public version).
The email used to register you as an authorized contributor must be the email used for the Git commit. Check your existing CLA data and verify that your email is set on your git commits.
The email used to register you as an authorized contributor must also be attached to your GitHub account.

googlebot · 2018-11-28T13:54:58Z

Thanks for your pull request. It looks like this may be your first contribution to a Google open source project (if not, look below for help). Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

📝 Please visit https://cla.developers.google.com/ to sign.

Once you've signed (or fixed any issues), please reply here (e.g. I signed it!) and we'll verify it.

What to do if you already signed the CLA

Individual signers

It's possible we don't have your GitHub username or you're using a different email address on your commit. Check your existing CLA data and verify that your email is set on your git commits.

Corporate signers

Your company has a Point of Contact who decides which employees are authorized to participate. Ask your POC to be added to the group of authorized contributors. If you don't know who your Point of Contact is, direct the Google project maintainer to go/cla#troubleshoot (Public version).
The email used to register you as an authorized contributor must be the email used for the Git commit. Check your existing CLA data and verify that your email is set on your git commits.
The email used to register you as an authorized contributor must also be attached to your GitHub account.

edwarnicke · 2018-11-28T13:57:39Z

@sjdweb would something like this in the docs have helped you?

googlebot · 2018-11-28T13:57:41Z

CLAs look good, thanks!

googlebot · 2018-11-28T13:57:41Z

CLAs look good, thanks!

edwarnicke · 2018-11-28T14:22:20Z

Is CI broken? I'm a little surprised that a readme change broke CI...

srenatus

Nitpicked some typos. I'm just an interested bystander, and while I appreciate the clarity, I wonder if it makes sense to include this. After all, the same applies to any binary used in a build system, protoc-gen-go is not special in any way here. 🤔

srenatus · 2018-11-29T09:38:56Z

README.md

+CI systems should produce repeatable builds.  Installing proto-gen-go via:
+
+```bash
+go get go get github.com/golang/protobuf/protoc-gen-go


one go get too much here

srenatus · 2018-11-29T09:39:11Z

README.md

+
+## Using protoc-gen-go in Continuous Integration (CI) Systems
+
+CI systems should produce repeatable builds.  Installing proto-gen-go via:


protoc-gen-go (missing c)

srenatus · 2018-11-29T09:40:35Z

README.md

+go get go get github.com/golang/protobuf/protoc-gen-go
+```
+
+within a CI system produces inherently non-reproducible builds, because 


[nit] "reproducible" is a stronger property than repeatable, I'd think. (See this definition, for example, https://reproducible-builds.org/docs/definition/)

srenatus · 2018-11-29T09:41:16Z

README.md

+This can lead to unexpected build breakage in the CI system.
+
+If you are using vendoring in go, and wish to have a repeatable stable build process,
+it is better to install protoc-ge-go in your CI via:


protoc-gen-go (missing n)

sjdweb · 2018-11-29T09:57:23Z

Would have helped yes, probably in my case including dep instructions (and go modules I assume when it's production ready).

edwarnicke · 2018-12-03T15:50:43Z

@srenatus Thanks for the typo checks. I'm not super strongly opinionated about where this goes... but I suspect a lot of folks are about to have this issue due to recent changes in master which cause things to break if you are being cavalier about your CI.

Do you have other suggestions about how to get the word out?

dsymonds and others added 30 commits March 9, 2016 13:16

A few small bits of style modernisation,

b9504f2

such as using append where it makes the code shorter and more natural.

Switch use of strings.LastIndexByte to strings.LastIndex.

0fd8c90

It has almost zero impact on performance in practice, and permits this code to keep working with Go 1.4. Fixes golang#146.

Add support for decoding a stream of JSON objects.

9951127

Signed-off-by: David Symonds <dsymonds@golang.org>

jsonpb: Accept both camelCase and orig_name as field names for oneof …

62e4364

…fields. Fixes golang#148.

Expose the gRPC code generation plugin.

2fea9e1

Clarify the unsupported nature of this interface in the README. Fixes golang#147.

Add Any helper funcs to the ptypes package.

d120e2f

Add specific error for oneof with nil element.

8d92cf5

Signed-off-by: David Symonds <dsymonds@golang.org>

Use reflection-driven field accesses under GopherJS.

dda510a

Like App Engine, GopherJS does not support unsafe. This change adds a 'js' build tag (set by GopherJS) alongside the existing 'appengine' tags. Fixes golang#154.

Improve error message when text unmarshaling a nested message with a …

78550bb

…required field. Signed-off-by: David Symonds <dsymonds@golang.org>

jsonpb: Fix handling of repeated enums.

f0a097d

Enums used in maps is still broken, but commented-out test data is now there, together with TODOs. Fixes golang#164.

Update the grpc plugin to support the grpc-go interceptor implementat…

331aba2

…ion. See grpc/grpc-go#642 for the corresponding grpc-go change. Signed-off-by: David Symonds <dsymonds@golang.org>

Correctly set OrigName for oneof fields.

2ebff28

Signed-off-by: David Symonds <dsymonds@golang.org>

jsonpb: Marshal Any according to the spec.

bf531ff

Unmarshaling Any is a TODO. Fixes golang#170.

Use grpcPkg for a bit of code gen that overlooked it.

7cc19b7

proto: clean up proto API for extensions

a4ab9ec

Methods that manipulate protos with extensions will now take proto.Message instead of the internal extendableProto interface. A ClearExtensions method is added to clear all extensions on protos.

proto: return error from Marshal if a message encodes to > 2GiB

cd85f19

While this check could be done before marshalling, it shouldn't occur frequently and thus would slow down proto encoding on average.

proto: fix equality to work with V1 generated format

9e6977f

When the new V2 generated extension format was introduced, we mistakenly dropped support for comparing V1 generated extensions for equality. Add that back.

proto: allow text proto map keys to be omitted or order swapped

1111461

net/proto2/go: make a slight change to how we handle []byte fields

3b06fc7

This is meant to fix golang#188. There are no tests because we don't guarantee that we're going to maintain this behavior in the future. Fixes golang#188

Add missing error check in jsonpb's marshal implementation.

f43dfb5

Otherwise invalid JSON can be produced.

Fix marshalAny's handling of indentation.

c27503d

Make proto.RegisterFile public for gRPC

ba6f978

Regenerated .pb.go files

protoc-gen-go: export the generated variable name of the FileDescript…

545732f

…or bytes

ptypes: regen and remove transforms from regen.sh

8616e8e

proto: s/Printf/Print/ where there's no format verb

5386fff

protoc-gen-go/grpc: pass file descriptor for service as Metadata

0c1f6d6

Bump support package version number from 2 to 3.

proto: import change from Google.

f6b4231

Add tests for merging a map field. When src contains a duplicate key, its value message replaces (not merges with) the value message in dst.

proto: regenerate proto3.pb.go from .proto file

b3e26a4

A test depends on the new 'Cute' field.

sesmith177 and others added 18 commits October 3, 2018 12:35

protoc-gen-go: predeclared identifiers in cleanPackageName (golang#722)

31e0d06

Don't generate identifiers that conflict with predeclared identifiers, prepend with "_" instead. Signed-off-by: Arjun Sreedharan <asreedharan@pivotal.io>

.travis.yml: include go1.11 (golang#731)

5e707b2

protoc-gen-go/generator: fix misspelling (golang#732)

7be3631

protoc-gen-go: don't force space after "//" in comments (golang#725)

d7fc201

Avoid turning comments like "//////" into "// ////".

protoc-gen-go: add test case for deprecated oneof field

3ed0fc6

protoc-gen-go: generate deprecation comments for oneof fields

5e0eda4

protoc-gen-go: normalize floating-point default values (golang#737)

d3de96c

Parse floating-point default values and format them with fmt.Sprint. Eliminates a minor point of inconsistency with the v2 generator.

protoc-gen-go: expand import_public test files

d18af0d

Convert all the import_public protos to proto2 to allow testing more features. Add usage of groups, extensions, and default values.

protoc-gen-go: include groups in public import forwarders

0382e76

Groups were being excluded from the set of messages we generate forwarding declarations for. Include them.

all: increase minimum supported version to go1.9 (golang#742)

75dceb1

all: update to v3.6.1 (golang#757)

882cf97

Update all proto files that is obtained from the upstream repository to v3.6.1.

Add note to README on responsible CI usage.

eb11628

This is in the hopes of easing folks hitting golang#763 in getting to a proper use of CI. Signed-off-by: Ed Warnicke <hagbard@gmail.com>

srenatus reviewed Nov 29, 2018

View reviewed changes

gopherbot closed this Apr 16, 2019

gopherbot force-pushed the master branch from e91709a to 4419fab Compare April 16, 2019 15:07

golang locked and limited conversation to collaborators Jun 26, 2020

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add note to README on responsible CI usage. #765

Add note to README on responsible CI usage. #765

edwarnicke commented Nov 28, 2018

googlebot commented Nov 28, 2018

googlebot commented Nov 28, 2018

edwarnicke commented Nov 28, 2018

googlebot commented Nov 28, 2018

googlebot commented Nov 28, 2018

edwarnicke commented Nov 28, 2018

srenatus left a comment

srenatus Nov 29, 2018

srenatus Nov 29, 2018

srenatus Nov 29, 2018

srenatus Nov 29, 2018

sjdweb commented Nov 29, 2018

edwarnicke commented Dec 3, 2018


		## Using protoc-gen-go in Continuous Integration (CI) Systems

		CI systems should produce repeatable builds. Installing proto-gen-go via:

Add note to README on responsible CI usage. #765

Add note to README on responsible CI usage. #765

Conversation

edwarnicke commented Nov 28, 2018

googlebot commented Nov 28, 2018

What to do if you already signed the CLA

Individual signers

Corporate signers

googlebot commented Nov 28, 2018

What to do if you already signed the CLA

Individual signers

Corporate signers

edwarnicke commented Nov 28, 2018

googlebot commented Nov 28, 2018

googlebot commented Nov 28, 2018

edwarnicke commented Nov 28, 2018

srenatus left a comment

Choose a reason for hiding this comment

srenatus Nov 29, 2018

Choose a reason for hiding this comment

srenatus Nov 29, 2018

Choose a reason for hiding this comment

srenatus Nov 29, 2018

Choose a reason for hiding this comment

srenatus Nov 29, 2018

Choose a reason for hiding this comment

sjdweb commented Nov 29, 2018

edwarnicke commented Dec 3, 2018