Skip to content

Commit

Permalink
all: rename tag and flags for legacy support
Browse files Browse the repository at this point in the history
Rename build tag "proto1_legacy" -> "protolegacy"
to be consistent with the "protoreflect" tag.

Rename flag constant "Proto1Legacy" -> "ProtoLegacy" since
it covers more than simply proto1 legacy features.
For example, it covers alpha-features of proto3 that
were eventually removed from the final proto3 release.

Change-Id: I0f4fcbadd4b5a61c87645e2e5be11d187e59157c
Reviewed-on: https://go-review.googlesource.com/c/protobuf/+/189345
Reviewed-by: Damien Neil <dneil@google.com>
  • Loading branch information
dsnet committed Aug 8, 2019
1 parent 7164af5 commit 1799d11
Show file tree
Hide file tree
Showing 25 changed files with 56 additions and 55 deletions.
2 changes: 1 addition & 1 deletion encoding/protojson/decode.go
Expand Up @@ -139,7 +139,7 @@ func (o UnmarshalOptions) unmarshalMessage(m pref.Message, skipTypeURL bool) err
// unmarshalFields unmarshals the fields into the given protoreflect.Message.
func (o UnmarshalOptions) unmarshalFields(m pref.Message, skipTypeURL bool) error {
messageDesc := m.Descriptor()
if !flags.Proto1Legacy && messageset.IsMessageSet(messageDesc) {
if !flags.ProtoLegacy && messageset.IsMessageSet(messageDesc) {
return errors.New("no support for proto1 MessageSets")
}

Expand Down
12 changes: 6 additions & 6 deletions encoding/protojson/decode_test.go
Expand Up @@ -1334,7 +1334,7 @@ func TestUnmarshal(t *testing.T) {
})
return m
}(),
skip: !flags.Proto1Legacy,
skip: !flags.ProtoLegacy,
}, {
desc: "not real MessageSet 1",
inputMessage: &pb2.FakeMessageSet{},
Expand All @@ -1350,7 +1350,7 @@ func TestUnmarshal(t *testing.T) {
})
return m
}(),
skip: !flags.Proto1Legacy,
skip: !flags.ProtoLegacy,
}, {
desc: "not real MessageSet 2",
inputMessage: &pb2.FakeMessageSet{},
Expand All @@ -1360,7 +1360,7 @@ func TestUnmarshal(t *testing.T) {
}
}`,
wantErr: true,
skip: !flags.Proto1Legacy,
skip: !flags.ProtoLegacy,
}, {
desc: "not real MessageSet 3",
inputMessage: &pb2.MessageSet{},
Expand All @@ -1376,7 +1376,7 @@ func TestUnmarshal(t *testing.T) {
})
return m
}(),
skip: !flags.Proto1Legacy,
skip: !flags.ProtoLegacy,
}, {
desc: "Empty",
inputMessage: &emptypb.Empty{},
Expand Down Expand Up @@ -2457,13 +2457,13 @@ func TestUnmarshal(t *testing.T) {
m.SetWeakMessage1(&weakpb.WeakImportMessage1{A: proto.Int32(1)})
return m
}(),
skip: !flags.Proto1Legacy,
skip: !flags.ProtoLegacy,
}, {
desc: "weak fields; unknown field",
inputMessage: &testpb.TestWeak{},
inputText: `{"weak_message1":{"a":1}, "weak_message2":{"a":1}}`,
wantErr: true, // weak_message2 is unknown since the package containing it is not imported
skip: !flags.Proto1Legacy,
skip: !flags.ProtoLegacy,
}}

for _, tt := range tests {
Expand Down
2 changes: 1 addition & 1 deletion encoding/protojson/encode.go
Expand Up @@ -88,7 +88,7 @@ func (o MarshalOptions) marshalMessage(m pref.Message) error {
// marshalFields marshals the fields in the given protoreflect.Message.
func (o MarshalOptions) marshalFields(m pref.Message) error {
messageDesc := m.Descriptor()
if !flags.Proto1Legacy && messageset.IsMessageSet(messageDesc) {
if !flags.ProtoLegacy && messageset.IsMessageSet(messageDesc) {
return errors.New("no support for proto1 MessageSets")
}

Expand Down
6 changes: 3 additions & 3 deletions encoding/protojson/encode_test.go
Expand Up @@ -1034,7 +1034,7 @@ func TestMarshal(t *testing.T) {
"optString": "not a messageset extension"
}
}`,
skip: !flags.Proto1Legacy,
skip: !flags.ProtoLegacy,
}, {
desc: "not real MessageSet 1",
input: func() proto.Message {
Expand All @@ -1049,7 +1049,7 @@ func TestMarshal(t *testing.T) {
"optString": "not a messageset extension"
}
}`,
skip: !flags.Proto1Legacy,
skip: !flags.ProtoLegacy,
}, {
desc: "not real MessageSet 2",
input: func() proto.Message {
Expand All @@ -1064,7 +1064,7 @@ func TestMarshal(t *testing.T) {
"optString": "another not a messageset extension"
}
}`,
skip: !flags.Proto1Legacy,
skip: !flags.ProtoLegacy,
}, {
desc: "BoolValue empty",
input: &wrapperspb.BoolValue{},
Expand Down
2 changes: 1 addition & 1 deletion encoding/prototext/decode.go
Expand Up @@ -76,7 +76,7 @@ func (o UnmarshalOptions) Unmarshal(b []byte, m proto.Message) error {
// unmarshalMessage unmarshals a [][2]text.Value message into the given protoreflect.Message.
func (o UnmarshalOptions) unmarshalMessage(tmsg [][2]text.Value, m pref.Message) error {
messageDesc := m.Descriptor()
if !flags.Proto1Legacy && messageset.IsMessageSet(messageDesc) {
if !flags.ProtoLegacy && messageset.IsMessageSet(messageDesc) {
return errors.New("no support for proto1 MessageSets")
}

Expand Down
12 changes: 6 additions & 6 deletions encoding/prototext/decode_test.go
Expand Up @@ -1310,7 +1310,7 @@ opt_int32: 42
})
return m
}(),
skip: !flags.Proto1Legacy,
skip: !flags.ProtoLegacy,
}, {
desc: "not real MessageSet 1",
inputMessage: &pb2.FakeMessageSet{},
Expand All @@ -1326,7 +1326,7 @@ opt_int32: 42
})
return m
}(),
skip: !flags.Proto1Legacy,
skip: !flags.ProtoLegacy,
}, {
desc: "not real MessageSet 2",
inputMessage: &pb2.FakeMessageSet{},
Expand All @@ -1336,7 +1336,7 @@ opt_int32: 42
}
`,
wantErr: true,
skip: !flags.Proto1Legacy,
skip: !flags.ProtoLegacy,
}, {
desc: "not real MessageSet 3",
inputMessage: &pb2.MessageSet{},
Expand All @@ -1351,7 +1351,7 @@ opt_int32: 42
})
return m
}(),
skip: !flags.Proto1Legacy,
skip: !flags.ProtoLegacy,
}, {
desc: "Any not expanded",
inputMessage: &anypb.Any{},
Expand Down Expand Up @@ -1501,13 +1501,13 @@ type_url: "pb2.Nested"
m.SetWeakMessage1(&weakpb.WeakImportMessage1{A: proto.Int32(1)})
return m
}(),
skip: !flags.Proto1Legacy,
skip: !flags.ProtoLegacy,
}, {
desc: "weak fields; unknown field",
inputMessage: &testpb.TestWeak{},
inputText: `weak_message1:{a:1} weak_message2:{a:1}`,
wantErr: true, // weak_message2 is unknown since the package containing it is not imported
skip: !flags.Proto1Legacy,
skip: !flags.ProtoLegacy,
}}

for _, tt := range tests {
Expand Down
2 changes: 1 addition & 1 deletion encoding/prototext/encode.go
Expand Up @@ -75,7 +75,7 @@ func (o MarshalOptions) Marshal(m proto.Message) ([]byte, error) {
// marshalMessage converts a protoreflect.Message to a text.Value.
func (o MarshalOptions) marshalMessage(m pref.Message) (text.Value, error) {
messageDesc := m.Descriptor()
if !flags.Proto1Legacy && messageset.IsMessageSet(messageDesc) {
if !flags.ProtoLegacy && messageset.IsMessageSet(messageDesc) {
return text.Value{}, errors.New("no support for proto1 MessageSets")
}

Expand Down
6 changes: 3 additions & 3 deletions encoding/prototext/encode_test.go
Expand Up @@ -1078,7 +1078,7 @@ opt_int32: 42
opt_string: "not a messageset extension"
}
`,
skip: !flags.Proto1Legacy,
skip: !flags.ProtoLegacy,
}, {
desc: "not real MessageSet 1",
input: func() proto.Message {
Expand All @@ -1092,7 +1092,7 @@ opt_int32: 42
opt_string: "not a messageset extension"
}
`,
skip: !flags.Proto1Legacy,
skip: !flags.ProtoLegacy,
}, {
desc: "not real MessageSet 2",
input: func() proto.Message {
Expand All @@ -1106,7 +1106,7 @@ opt_int32: 42
opt_string: "another not a messageset extension"
}
`,
skip: !flags.Proto1Legacy,
skip: !flags.ProtoLegacy,
}, {
desc: "Any not expanded",
mo: prototext.MarshalOptions{
Expand Down
10 changes: 5 additions & 5 deletions integration_test.go
Expand Up @@ -48,8 +48,8 @@ func Test(t *testing.T) {

if *regenerate {
t.Run("Generate", func(t *testing.T) {
fmt.Print(mustRunCommand(t, "go", "run", "-tags", "proto1_legacy", "./internal/cmd/generate-types", "-execute"))
fmt.Print(mustRunCommand(t, "go", "run", "-tags", "proto1_legacy", "./internal/cmd/generate-protos", "-execute"))
fmt.Print(mustRunCommand(t, "go", "run", "-tags", "protolegacy", "./internal/cmd/generate-types", "-execute"))
fmt.Print(mustRunCommand(t, "go", "run", "-tags", "protolegacy", "./internal/cmd/generate-protos", "-execute"))
files := strings.Split(strings.TrimSpace(mustRunCommand(t, "git", "ls-files", "*.go")), "\n")
mustRunCommand(t, append([]string{"gofmt", "-w"}, files...)...)
})
Expand Down Expand Up @@ -79,7 +79,7 @@ func Test(t *testing.T) {
runGo("PureGo", workDir, "go", "test", "-race", "-tags", "purego", "./...")
runGo("Reflect", workDir, "go", "test", "-race", "-tags", "protoreflect", "./...")
if goVersion == golangLatest {
runGo("Proto1Legacy", workDir, "go", "test", "-race", "-tags", "proto1_legacy", "./...")
runGo("ProtoLegacy", workDir, "go", "test", "-race", "-tags", "protolegacy", "./...")
runGo("ProtocGenGo", "cmd/protoc-gen-go/testdata", "go", "test")
runGo("ProtocGenGoGRPC", "cmd/protoc-gen-go-grpc/testdata", "go", "test")
}
Expand All @@ -94,11 +94,11 @@ func Test(t *testing.T) {
mustRunCommand(t, runner, "--failure_list", failureList, "--enforce_recommended", driver)
})
t.Run("GeneratedGoFiles", func(t *testing.T) {
diff := mustRunCommand(t, "go", "run", "-tags", "proto1_legacy", "./internal/cmd/generate-types")
diff := mustRunCommand(t, "go", "run", "-tags", "protolegacy", "./internal/cmd/generate-types")
if strings.TrimSpace(diff) != "" {
t.Fatalf("stale generated files:\n%v", diff)
}
diff = mustRunCommand(t, "go", "run", "-tags", "proto1_legacy", "./internal/cmd/generate-protos")
diff = mustRunCommand(t, "go", "run", "-tags", "protolegacy", "./internal/cmd/generate-protos")
if strings.TrimSpace(diff) != "" {
t.Fatalf("stale generated files:\n%v", diff)
}
Expand Down
2 changes: 1 addition & 1 deletion internal/encoding/text/text_test.go
Expand Up @@ -615,7 +615,7 @@ func Test(t *testing.T) {
}, {
in: `nums: [0xbeefbeef,0xbeefbeefbeefbeef]`,
wantVal: V(Msg{{ID("nums"), func() Value {
if flags.Proto1Legacy {
if flags.ProtoLegacy {
return V(Lst{V(int32(-1091584273)), V(int64(-4688318750159552785))})
} else {
return V(Lst{V(uint32(0xbeefbeef)), V(uint64(0xbeefbeefbeefbeef))})
Expand Down
2 changes: 1 addition & 1 deletion internal/encoding/text/value.go
Expand Up @@ -192,7 +192,7 @@ func (v Value) Int(b64 bool) (x int64, ok bool) {
}
// C++ accepts large positive hex numbers as negative values.
// This feature is here for proto1 backwards compatibility purposes.
if flags.Proto1Legacy && len(v.raw) > 1 && v.raw[0] == '0' && v.raw[1] == 'x' {
if flags.ProtoLegacy && len(v.raw) > 1 && v.raw[0] == '0' && v.raw[1] == 'x' {
if !b64 {
return int64(int32(n)), n <= math.MaxUint32
}
Expand Down
2 changes: 1 addition & 1 deletion internal/encoding/wire/wire.go
Expand Up @@ -494,7 +494,7 @@ func SizeGroup(num Number, n int) int {
// Other than overflow, this does not check for field number validity.
func DecodeTag(x uint64) (Number, Type) {
// NOTE: MessageSet allows for larger field numbers than normal.
if flags.Proto1Legacy {
if flags.ProtoLegacy {
if x>>3 > uint64(math.MaxInt32) {
return -1, 0
}
Expand Down
11 changes: 6 additions & 5 deletions internal/flags/flags.go
Expand Up @@ -5,12 +5,13 @@
// Package flags provides a set of flags controlled by build tags.
package flags

// Proto1Legacy specifies whether to enable support for legacy proto1
// functionality such as MessageSets, weak fields, and various other obscure
// behavior that is necessary to maintain backwards compatibility with proto1.
// ProtoLegacy specifies whether to enable support for legacy functionality
// such as MessageSets, weak fields, and various other obscure behavior
// that is necessary to maintain backwards compatibility with proto1 or
// the pre-release variants of proto2 and proto3.
//
// This is disabled by default unless built with the "proto1_legacy" tag.
// This is disabled by default unless built with the "protolegacy" tag.
//
// WARNING: The compatibility agreement covers nothing provided by this flag.
// As such, functionality may suddenly be removed or changed at our discretion.
const Proto1Legacy = proto1Legacy
const ProtoLegacy = protoLegacy
Expand Up @@ -2,8 +2,8 @@
// Use of this source code is governed by a BSD-style
// license that can be found in the LICENSE.md file.

// +build proto1_legacy
// +build !protolegacy

package flags

const proto1Legacy = true
const protoLegacy = false
Expand Up @@ -2,8 +2,8 @@
// Use of this source code is governed by a BSD-style
// license that can be found in the LICENSE.md file.

// +build !proto1_legacy
// +build protolegacy

package flags

const proto1Legacy = false
const protoLegacy = true
4 changes: 2 additions & 2 deletions internal/impl/codec_messageset.go
Expand Up @@ -45,7 +45,7 @@ func sizeMessageSet(mi *MessageInfo, p pointer, tagsize int, opts marshalOptions
}

func marshalMessageSet(mi *MessageInfo, b []byte, p pointer, wiretag uint64, opts marshalOptions) ([]byte, error) {
if !flags.Proto1Legacy {
if !flags.ProtoLegacy {
return b, errors.New("no support for message_set_wire_format")
}
ext := *p.Extensions()
Expand Down Expand Up @@ -97,7 +97,7 @@ func marshalMessageSetField(mi *MessageInfo, b []byte, x ExtensionField, opts ma
}

func unmarshalMessageSet(mi *MessageInfo, b []byte, p pointer, wtyp wire.Type, opts unmarshalOptions) (int, error) {
if !flags.Proto1Legacy {
if !flags.ProtoLegacy {
return 0, errors.New("no support for message_set_wire_format")
}
if wtyp != wire.StartGroupType {
Expand Down
2 changes: 1 addition & 1 deletion internal/impl/message_field.go
Expand Up @@ -288,7 +288,7 @@ func fieldInfoForScalar(fd pref.FieldDescriptor, fs reflect.StructField, x expor
}

func fieldInfoForWeakMessage(fd pref.FieldDescriptor, weakOffset offset) fieldInfo {
if !flags.Proto1Legacy {
if !flags.ProtoLegacy {
panic("no support for proto1 weak fields")
}

Expand Down
2 changes: 1 addition & 1 deletion internal/strs/strings.go
Expand Up @@ -15,7 +15,7 @@ import (

// EnforceUTF8 reports whether to enforce strict UTF-8 validation.
func EnforceUTF8(fd protoreflect.FieldDescriptor) bool {
if flags.Proto1Legacy {
if flags.ProtoLegacy {
if fd, ok := fd.(interface{ EnforceUTF8() bool }); ok {
return fd.EnforceUTF8()
}
Expand Down
4 changes: 2 additions & 2 deletions proto/decode_test.go
Expand Up @@ -98,9 +98,9 @@ func TestDecodeNoEnforceUTF8(t *testing.T) {
got := reflect.New(reflect.TypeOf(want).Elem()).Interface().(proto.Message)
err := proto.Unmarshal(test.wire, got)
switch {
case flags.Proto1Legacy && err != nil:
case flags.ProtoLegacy && err != nil:
t.Errorf("Unmarshal returned unexpected error: %v\nMessage:\n%v", err, marshalText(want))
case !flags.Proto1Legacy && err == nil:
case !flags.ProtoLegacy && err == nil:
t.Errorf("Unmarshal did not return expected error for invalid UTF8: %v\nMessage:\n%v", err, marshalText(want))
}
})
Expand Down
4 changes: 2 additions & 2 deletions proto/encode_test.go
Expand Up @@ -104,9 +104,9 @@ func TestEncodeNoEnforceUTF8(t *testing.T) {
t.Run(fmt.Sprintf("%s (%T)", test.desc, want), func(t *testing.T) {
_, err := proto.Marshal(want)
switch {
case flags.Proto1Legacy && err != nil:
case flags.ProtoLegacy && err != nil:
t.Errorf("Marshal returned unexpected error: %v\nMessage:\n%v", err, marshalText(want))
case !flags.Proto1Legacy && err == nil:
case !flags.ProtoLegacy && err == nil:
t.Errorf("Marshal did not return expected error for invalid UTF8: %v\nMessage:\n%v", err, marshalText(want))
}
})
Expand Down
4 changes: 2 additions & 2 deletions proto/messageset.go
Expand Up @@ -25,7 +25,7 @@ func sizeMessageSet(m protoreflect.Message) (size int) {
}

func marshalMessageSet(b []byte, m protoreflect.Message, o MarshalOptions) ([]byte, error) {
if !flags.Proto1Legacy {
if !flags.ProtoLegacy {
return b, errors.New("no support for message_set_wire_format")
}
var err error
Expand Down Expand Up @@ -53,7 +53,7 @@ func marshalMessageSetField(b []byte, fd protoreflect.FieldDescriptor, value pro
}

func unmarshalMessageSet(b []byte, m protoreflect.Message, o UnmarshalOptions) error {
if !flags.Proto1Legacy {
if !flags.ProtoLegacy {
return errors.New("no support for message_set_wire_format")
}
md := m.Descriptor()
Expand Down
2 changes: 1 addition & 1 deletion proto/messageset_test.go
Expand Up @@ -14,7 +14,7 @@ import (
)

func init() {
if flags.Proto1Legacy {
if flags.ProtoLegacy {
testProtos = append(testProtos, messageSetTestProtos...)
}
}
Expand Down

0 comments on commit 1799d11

Please sign in to comment.