Skip to content

Commit

Permalink
Separating tools dependencies from main dependencies (uber#4895)
Browse files Browse the repository at this point in the history
Adding gowrap in uber#4879 pulled in an additional *30MB* of dependencies, which is
pretty ridiculous.  It's happening because, while gowrap uses a tools-tagged `tools.go`,
it's in the same module as everything else, so its dependencies are pulled in too, because
Go has no way to tell that it's not needed by callers (it needs to consider ALL tags).

We're doing the same thing.  Granted, we're an application and not a library, but it still
seems kinda unnecessary.  And it forces upgrades we may not want to force (yet).

So this moves tools.go to a new package, sets up a new go.mod file just for it, and adds
a bit of automation to prevent it from drifting too far from the main module.
With this, `go mod tidy` in the top level does not consider tools-dependencies any more,
so we actually *remove* some, and do not add tons more for gowrap/etc in the future.

---

internal/tools is just because it needs to be in a separate package, and "tools" was taken.
plus nobody should ever import it, so it should be a clear "dependency dead-end" for tools.
  • Loading branch information
Groxx committed Jul 21, 2022
1 parent cbb16b7 commit 094b5f2
Show file tree
Hide file tree
Showing 8 changed files with 1,157 additions and 308 deletions.
46 changes: 33 additions & 13 deletions Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -39,8 +39,9 @@ $(BUILD)/proto-lint:
$(BUILD)/fmt: $(BUILD)/copyright # formatting must occur only after all other go-file-modifications are done
$(BUILD)/copyright: $(BUILD)/codegen # must add copyright to generated code, sometimes needs re-formatting
$(BUILD)/codegen: $(BUILD)/thrift $(BUILD)/protoc
$(BUILD)/thrift:
$(BUILD)/protoc:
$(BUILD)/thrift: $(BUILD)/go_mod_check
$(BUILD)/protoc: $(BUILD)/go_mod_check
$(BUILD)/go_mod_check:

# ====================================
# helper vars
Expand Down Expand Up @@ -125,7 +126,19 @@ LINT_SRC := $(filter-out %_test.go ./.gen/%, $(ALL_SRC))
# downloads and builds a go-gettable tool, versioned by go.mod, and installs
# it into the build folder, named the same as the last portion of the URL.
define go_build_tool
$Q echo "building $(or $(2), $(notdir $(1))) from $(1)..."
$Q echo "building $(or $(2), $(notdir $(1))) from internal/tools/go.mod..."
$Q go build -mod=readonly -modfile=internal/tools/go.mod -o $(BIN)/$(or $(2), $(notdir $(1))) $(1)
endef

# same as go_build_tool, but uses our main module file, not the tools one.
# this is necessary / useful for tools that we are already importing in the repo, e.g. yarpc.
# versions here are checked to make sure the tools version matches the service version.
#
# this is an imperfect check as it only checks the top-level version.
# checks of other packages are handled in $(BUILD)/go_mod_check
define go_mod_build_tool
$Q echo "building $(or $(2), $(notdir $(1))) from go.mod..."
$Q ./scripts/check-gomod-version.sh $(1) $(if $(verbose),-v)
$Q go build -mod=readonly -o $(BIN)/$(or $(2), $(notdir $(1))) $(1)
endef

Expand All @@ -135,35 +148,42 @@ $(BIN) $(BUILD):
$Q mkdir -p $@

$(BIN)/thriftrw: go.mod
$(call go_build_tool,go.uber.org/thriftrw)
$(call go_mod_build_tool,go.uber.org/thriftrw)

$(BIN)/thriftrw-plugin-yarpc: go.mod
$(call go_build_tool,go.uber.org/yarpc/encoding/thrift/thriftrw-plugin-yarpc)
$(call go_mod_build_tool,go.uber.org/yarpc/encoding/thrift/thriftrw-plugin-yarpc)

$(BIN)/mockgen: go.mod
$(BIN)/mockgen: internal/tools/go.mod
$(call go_build_tool,github.com/golang/mock/mockgen)

$(BIN)/mockery: go.mod
$(BIN)/mockery: internal/tools/go.mod
$(call go_build_tool,github.com/vektra/mockery/v2,mockery)

$(BIN)/enumer: go.mod
$(BIN)/enumer: internal/tools/go.mod
$(call go_build_tool,github.com/dmarkham/enumer)

$(BIN)/goimports: go.mod
$(BIN)/goimports: internal/tools/go.mod
$(call go_build_tool,golang.org/x/tools/cmd/goimports)

$(BIN)/revive: go.mod
$(BIN)/revive: internal/tools/go.mod
$(call go_build_tool,github.com/mgechev/revive)

$(BIN)/protoc-gen-gogofast: go.mod | $(BIN)
$(call go_build_tool,github.com/gogo/protobuf/protoc-gen-gogofast)
$(call go_mod_build_tool,github.com/gogo/protobuf/protoc-gen-gogofast)

$(BIN)/protoc-gen-yarpc-go: go.mod | $(BIN)
$(call go_build_tool,go.uber.org/yarpc/encoding/protobuf/protoc-gen-yarpc-go)
$(call go_mod_build_tool,go.uber.org/yarpc/encoding/protobuf/protoc-gen-yarpc-go)

$(BIN)/goveralls: go.mod
$(BIN)/goveralls: internal/tools/go.mod
$(call go_build_tool,github.com/mattn/goveralls)

$(BUILD)/go_mod_check: go.mod internal/tools/go.mod
$Q # ensure both have the same apache/thrift replacement
$Q ./scripts/check-gomod-version.sh github.com/apache/thrift/lib/go/thrift $(if $(verbose),-v)
$Q # generated == used is occasionally important for gomock / mock libs in general. this is not a definite problem if violated though.
$Q ./scripts/check-gomod-version.sh github.com/golang/mock/gomock $(if $(verbose),-v)
$Q touch $@

# copyright header checker/writer. only requires stdlib, so no other dependencies are needed.
$(BIN)/copyright: cmd/tools/copyright/licensegen.go
$Q go build -o $@ ./cmd/tools/copyright/licensegen.go
Expand Down
117 changes: 0 additions & 117 deletions common/thrift_util.go

This file was deleted.

28 changes: 2 additions & 26 deletions go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -6,14 +6,12 @@ require (
cloud.google.com/go/storage v1.6.0
github.com/Shopify/sarama v1.23.0
github.com/VividCortex/mysqlerr v1.0.0
github.com/apache/thrift v0.13.0
github.com/aws/aws-sdk-go v1.34.13
github.com/cactus/go-statsd-client/statsd v0.0.0-20191106001114-12b4e2b38748
github.com/cch123/elasticsql v0.0.0-20190321073543-a1a440758eb9
github.com/cristalhq/jwt/v3 v3.1.0
github.com/davecgh/go-spew v1.1.1
github.com/dgryski/go-farm v0.0.0-20200201041132-a6ae2369ad13
github.com/dmarkham/enumer v1.5.1
github.com/emirpasic/gods v0.0.0-20190624094223-e689965507ab
github.com/fatih/color v1.13.0
github.com/go-sql-driver/mysql v1.5.0
Expand All @@ -27,8 +25,6 @@ require (
github.com/jonboulle/clockwork v0.1.0
github.com/lib/pq v1.2.0
github.com/m3db/prometheus_client_golang v0.8.1
github.com/mattn/goveralls v0.0.7
github.com/mgechev/revive v1.0.3
github.com/olekukonko/tablewriter v0.0.4
github.com/olivere/elastic v6.2.21+incompatible
github.com/olivere/elastic/v7 v7.0.21
Expand All @@ -42,10 +38,8 @@ require (
github.com/uber/cadence-idl v0.0.0-20220223020740-f2f5b7fc2bbd
github.com/uber/ringpop-go v0.8.5
github.com/uber/tchannel-go v1.22.0
github.com/uber/tcheck v1.1.0
github.com/urfave/cli v1.22.4
github.com/valyala/fastjson v1.4.1
github.com/vektra/mockery/v2 v2.0.3
github.com/xdg/scram v0.0.0-20180814205039-7eeb5667e42c
github.com/xwb1989/sqlparser v0.0.0-20180606152119-120387863bf2
go.mongodb.org/mongo-driver v1.7.3
Expand All @@ -61,7 +55,6 @@ require (
golang.org/x/oauth2 v0.0.0-20200107190931-bf48bf16ab8d
golang.org/x/sync v0.0.0-20210220032951-036812b2e83c
golang.org/x/time v0.0.0-20191024005414-555d28b269f0
golang.org/x/tools v0.1.11
gonum.org/v1/gonum v0.7.0
google.golang.org/api v0.26.0
google.golang.org/grpc v1.29.1
Expand All @@ -76,6 +69,7 @@ require (
github.com/BurntSushi/toml v0.3.1 // indirect
github.com/DataDog/zstd v1.4.0 // indirect
github.com/anmitsu/go-shlex v0.0.0-20161002113705-648efa622239 // indirect
github.com/apache/thrift v0.13.0 // indirect
github.com/benbjohnson/clock v0.0.0-20161215174838-7dc76406b6d3 // indirect
github.com/beorn7/perks v1.0.1 // indirect
github.com/cespare/xxhash/v2 v2.1.1 // indirect
Expand All @@ -85,7 +79,6 @@ require (
github.com/eapache/queue v1.1.0 // indirect
github.com/facebookgo/clock v0.0.0-20150410010913-600d898af40a // indirect
github.com/fatih/structtag v1.2.0 // indirect
github.com/fsnotify/fsnotify v1.4.7 // indirect
github.com/go-stack/stack v1.8.0 // indirect
github.com/gogo/googleapis v1.3.2 // indirect
github.com/gogo/status v1.1.0 // indirect
Expand All @@ -96,8 +89,6 @@ require (
github.com/googleapis/gax-go/v2 v2.0.5 // indirect
github.com/hailocab/go-hostpool v0.0.0-20160125115350-e80d13ce29ed // indirect
github.com/hashicorp/go-uuid v1.0.1 // indirect
github.com/hashicorp/hcl v1.0.0 // indirect
github.com/inconshreveable/mousetrap v1.0.0 // indirect
github.com/jcmturner/gofork v1.0.0 // indirect
github.com/jessevdk/go-flags v1.4.0 // indirect
github.com/jmespath/go-jmespath v0.3.0 // indirect
Expand All @@ -109,18 +100,12 @@ require (
github.com/m3db/prometheus_client_model v0.1.0 // indirect
github.com/m3db/prometheus_common v0.1.0 // indirect
github.com/m3db/prometheus_procfs v0.8.1 // indirect
github.com/magiconair/properties v1.8.1 // indirect
github.com/mailru/easyjson v0.7.6 // indirect
github.com/mattn/go-colorable v0.1.9 // indirect
github.com/mattn/go-isatty v0.0.14 // indirect
github.com/mattn/go-runewidth v0.0.7 // indirect
github.com/mattn/go-sqlite3 v1.11.0 // indirect
github.com/matttproud/golang_protobuf_extensions v1.0.2-0.20181231171920-c182affec369 // indirect
github.com/mgechev/dots v0.0.0-20190921121421-c36f7dcfbb81 // indirect
github.com/mitchellh/go-homedir v1.1.0 // indirect
github.com/mitchellh/mapstructure v1.1.2 // indirect
github.com/pascaldekloe/name v1.0.0 // indirect
github.com/pelletier/go-toml v1.7.0 // indirect
github.com/pierrec/lz4 v0.0.0-20190701081048-057d66e894a4 // indirect
github.com/pkg/errors v0.9.1 // indirect
github.com/pmezard/go-difflib v1.0.0 // indirect
Expand All @@ -129,17 +114,9 @@ require (
github.com/prometheus/common v0.9.1 // indirect
github.com/prometheus/procfs v0.0.9 // indirect
github.com/rcrowley/go-metrics v0.0.0-20181016184325-3113b8401b8a // indirect
github.com/rs/zerolog v1.18.0 // indirect
github.com/russross/blackfriday/v2 v2.0.1 // indirect
github.com/shurcooL/sanitized_anchor_name v1.0.0 // indirect
github.com/spf13/afero v1.1.2 // indirect
github.com/spf13/cast v1.3.0 // indirect
github.com/spf13/cobra v1.0.0 // indirect
github.com/spf13/jwalterweatherman v1.0.0 // indirect
github.com/spf13/pflag v1.0.3 // indirect
github.com/spf13/viper v1.7.0 // indirect
github.com/stretchr/objx v0.2.0 // indirect
github.com/subosito/gotenv v1.2.0 // indirect
github.com/uber-common/bark v1.2.1 // indirect
github.com/uber-go/mapdecode v1.0.0 // indirect
github.com/xdg-go/pbkdf2 v1.0.0 // indirect
Expand All @@ -155,13 +132,12 @@ require (
golang.org/x/lint v0.0.0-20200302205851-738671d3881b // indirect
golang.org/x/mod v0.6.0-dev.0.20220419223038-86c51ed26bb4 // indirect
golang.org/x/sys v0.0.0-20211019181941-9d821ace8654 // indirect
golang.org/x/term v0.0.0-20201126162022-7de9c90e9dd1 // indirect
golang.org/x/text v0.3.7 // indirect
golang.org/x/tools v0.1.11 // indirect
google.golang.org/appengine v1.6.7 // indirect
google.golang.org/genproto v0.0.0-20201201144952-b05cb90ed32e // indirect
google.golang.org/protobuf v1.25.0 // indirect
gopkg.in/inf.v0 v0.9.1 // indirect
gopkg.in/ini.v1 v1.51.0 // indirect
gopkg.in/jcmturner/aescts.v1 v1.0.1 // indirect
gopkg.in/jcmturner/dnsutils.v1 v1.0.1 // indirect
gopkg.in/jcmturner/goidentity.v3 v3.0.0 // indirect
Expand Down
Loading

0 comments on commit 094b5f2

Please sign in to comment.