Skip to content

Commit

Permalink
plugin: Drop library version matching
Browse files Browse the repository at this point in the history
This change drops the check made by the ThriftRW Plugin system verifying
that the version of ThriftRW talking to the plugin is the same as the
version of ThriftRW used to compile the plugin.

This check is unnecessay because we already verify the API version,
which is defined in the Thrift file.

This check is more trouble than it's worth as it's constantly tripping
up users and we don't change the plugin API nearly often enough to need
this.
  • Loading branch information
abhinav committed Jan 22, 2019
1 parent 8f6f49f commit decb245
Show file tree
Hide file tree
Showing 4 changed files with 3 additions and 79 deletions.
3 changes: 2 additions & 1 deletion CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,8 @@ The format is based on [Keep a Changelog](http://keepachangelog.com/en/1.0.0/)
and this project adheres to [Semantic Versioning](http://semver.org/spec/v2.0.0.html).

## [Unreleased]
- No changes yet.
### Fixed
- plugin: Library version matching was dropped.

## [1.15.0] - 2019-01-14
### Changed
Expand Down
3 changes: 0 additions & 3 deletions internal/plugin/errors.go
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,6 @@
package plugin

import (
"errors"
"fmt"

"go.uber.org/thriftrw/internal/semver"
Expand Down Expand Up @@ -52,8 +51,6 @@ func (e errAPIVersionMismatch) Error() string {
return fmt.Sprintf("plugin API version mismatch: expected %v but got %v", e.Want, e.Got)
}

var errVersionIsRequired = errors.New("Version is required")

type errVersionMismatch struct {
Want semver.Range
Got string
Expand Down
40 changes: 1 addition & 39 deletions internal/plugin/transport.go
Original file line number Diff line number Diff line change
Expand Up @@ -27,28 +27,14 @@ import (

"go.uber.org/thriftrw/internal/envelope"
"go.uber.org/thriftrw/internal/multiplex"
"go.uber.org/thriftrw/internal/semver"
"go.uber.org/thriftrw/plugin/api"
"go.uber.org/thriftrw/protocol"
"go.uber.org/thriftrw/version"

"go.uber.org/atomic"
"go.uber.org/multierr"
)

var (
_proto = protocol.Binary
compatRange = computeCompatibleRange()
)

func computeCompatibleRange() semver.Range {
v, err := semver.Parse(version.Version)
if err != nil {
panic(err)
}

return semver.CompatibleRange(v)
}
var _proto = protocol.Binary

// transportHandle is a Handle to a plugin which is behind an envelope.Transport.
type transportHandle struct {
Expand Down Expand Up @@ -88,30 +74,6 @@ func NewTransportHandle(name string, t envelope.Transport) (Handle, error) {
}
}

// If we got here, the API version matches so the plugin must have
// provided the Version
if handshake.LibraryVersion == nil {
return nil, errHandshakeFailed{
Name: name,
Reason: errVersionIsRequired,
}
}

version, err := semver.Parse(*handshake.LibraryVersion)
if err != nil {
return nil, errHandshakeFailed{Name: name, Reason: err}
}

if !compatRange.Contains(version) {
return nil, errHandshakeFailed{
Name: name,
Reason: errVersionMismatch{
Want: compatRange,
Got: *handshake.LibraryVersion,
},
}
}

features := make(map[api.Feature]struct{}, len(handshake.Features))
for _, feature := range handshake.Features {
features[feature] = struct{}{}
Expand Down
36 changes: 0 additions & 36 deletions internal/plugin/transport_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -144,42 +144,6 @@ func TestTransportHandleHandshakeError(t *testing.T) {
wantError: `handshake with plugin "foo" failed: ` +
fmt.Sprintf("plugin API version mismatch: expected %d but got 42", api.APIVersion),
},
{
desc: "missing version",
name: "foo",
response: &api.HandshakeResponse{
Name: "foo",
APIVersion: api.APIVersion,
LibraryVersion: nil,
Features: []api.Feature{},
},
wantError: `handshake with plugin "foo" failed: Version is required`,
},
{
desc: "unparseable version",
name: "foo",
response: &api.HandshakeResponse{
Name: "foo",
APIVersion: api.APIVersion,
LibraryVersion: ptr.String("hello"),
Features: []api.Feature{},
},
wantError: `handshake with plugin "foo" failed: ` +
`cannot parse as semantic version: "hello"`,
},
{
desc: "semver mismatch",
name: "foo",
response: &api.HandshakeResponse{
Name: "foo",
APIVersion: api.APIVersion,
LibraryVersion: ptr.String("12.3.4"),
Features: []api.Feature{},
},
wantError: `handshake with plugin "foo" failed: ` +
"plugin compiled with the wrong version of ThriftRW: " +
fmt.Sprintf("expected >=%v and <%v but got 12.3.4", &compatRange.Begin, &compatRange.End),
},
}

for _, tt := range tests {
Expand Down

0 comments on commit decb245

Please sign in to comment.