search

m-lab / go

General purpose libraries / APIs for use in mlab code.
Apache License 2.0
5 stars 6 forks source link

Use go modules to fix upstream dependency breakage. #93

Closed gfr10598 closed 4 years ago

gfr10598 commented 4 years ago

See m-lab/dev-tracker#531

This change is Reviewable

coveralls commented 4 years ago

Pull Request Test Coverage Report for Build 734

Totals Coverage Status
Change from base Build 717: 0.0%
Covered Lines: 1074
Relevant Lines: 1267
💛 - Coveralls
gfr10598 commented 4 years ago

I'm adding all SWEs to review this, because this is a major issue, and I want to ensure everyone is aware and on-board. I anticipate there will be reluctance to start using modules, but I think this is a good reason to do so.

The particular issue may get resolved upstream, but I think we need to have options when this happens again, and my efforts to fix this today have made it clear that we have a number of issues that interfere with modules. We need to at least fix these module incompatibilities, even if we don't adopt modules widely.

I suggest we adopt modules specifically for our m-lab/go repo, and debate further whether to use them in other repos.

pboothe commented 4 years ago

I would prefer to live at head by default. Using modules for everything means that we need to explicitly track changes to ... (writes a quick shell script to count the number of non-m-lab github repos we depend on) .... more than 80 libraries used among our dozens of projects.

Being exposed to spontaneous upstream breakage is the price we pay for not having to do the toil of tracking changes in everything we depend on. I think go modules can be a way we temporarily remediate the pain from upstream breakage, but a system where we never get upstream fixes unless we explicitly ask for them sets us up on the wrong path. I would prefer occasional small breakage (like this) to finding out that "the world has moved on" and our systems can't be upgraded or made to work simply because we didn't make small changes along the way as upstream libraries changed.

Or, put another way: go modules decrease the likelihood our code will stop compiling, and increase the brittleness of the overall system. System brittleness is a wicked and hard problem, and library updates are an annoying small problem. I don't know how to solve both problems simultaneously, so I would prefer to err on the side of fixing whole-system brittleness, even if it means upstream libraries can break us with stupid updates (as happened here).

The code in question that is bringing up this problem literally has a note "NOTE: This code has alpha status. It may be subject to backwards-incompatible changes." I do not want to use Go modules to require a dependency on old code with that status. Tracking that code when it is at head feels mostly kind of okay. Depending on old versions is just asking for long-term problems.

gfr10598 commented 4 years ago

Perhaps we need to discuss what constitutes HEAD. The reality is that projects may push something to head that is never tagged with a version number, and not intended to be used by the general public. So I very much agree that we should be working at latest, but I think we need to think carefully about whether we should be working at HEAD.

BTW, it looks like the problem we have today is caused by grpc moving a feature from head to v2. https://github.com/grpc/grpc-go/commit/336cf8d76145dc5ebd517fd9c19e14c6822450b3 The bigquery and other libraries that use this are likely not broken for clients that use modules, because they will use the latest instead of the HEAD.

pboothe commented 4 years ago

This one change is LGTM. The new policy is not. We should not hold up an expedient fix based on a policy discussion.