go-github icon indicating copy to clipboard operation
go-github copied to clipboard

Modules: Sub-Dependency is using v33, main project v35 - Type incompatibility + Module replacement

Open andygrunwald opened this issue 4 years ago • 14 comments

Hey everyone, thanks for maintaining this library, keeping everything in shape, and continuously release new versions. I am facing the challenge of using the new versions:

Sub-Dependency is using v33, main project v35 - Type incompatibility

Imagine Application A that is using

  • this library in v35
  • another library (e.g., https://github.com/palantir/go-githubapp) which is using v33

Independent of the breaking changes in versions (lets keep them out for now), when using both libraries, errors like

applicationA/file.go:129:89: cannot use listReposOpts (type *"github.com/google/go-github/v35/github".ListOptions) as type *"github.com/google/go-github/v33/github".ListOptions in argument to installationClient.Apps.ListRepos

What is the preferred way to solve this?

I can think of multiple solutions:

1) Updating dependency in sub-library If the sub-library is from a different vendor/user, you don't have control over it and might lead to forking.

2) Cast the type v33/github".ListOptions to v35/github".ListOptions This might work if the two types are the same. If this type is part of a breaking change, this might not work that simple and the code would be bloated with transferring the one type into another.

3) Module replacement Here I am not sure. Is there a way on how Application A can force a module replacement for all other sublibraries?

This might not be an issue with the library itself, but rather to the usage. However, I would like to get your thoughts on this and if you see a solution to this challenge. Happy to provide further information.

andygrunwald avatar Apr 27 '21 09:04 andygrunwald

My knee-jerk reaction is to fork https://github.com/palantir/go-githubapp, update it to v35, and submit a PR (or use the forked version).

I'll leave this open as others might have better responses since my knee is not always correct. :smile:

gmlewis avatar Apr 27 '21 11:04 gmlewis

Thanks, @gmlewis for the quick response. That is, indeed, what I did anyway, see https://github.com/palantir/go-githubapp/pull/81 However, I suppose this is not the only case or I am not the only one facing this.

andygrunwald avatar Apr 27 '21 11:04 andygrunwald

My understanding is that this is a known consequence of how Go modules work (specifically, the fact that the major version appears in the import path and the import path is what determines if two modules are the same) and @gmlewis's suggestion to fork the library is the expected solution.

I believe the replace directive won't work in this case because Go considers v33 and v35 to be completely separate modules and therefore not eligible for replacement with each other.


As a maintainer of palantir/go-githubapp and some other libraries that export google/go-github types, I've run into this same problem and for now have settled on trying to keep up with the new versions released here. Unfortunately, those upgrades are sometimes complicated by the need to support specific versions of GitHub Enterprise, which might not be compatible with the latest library version. For example, v33 removed the preview headers for the Checks API, since they are no longer needed on GitHub.com, but GitHub Enterprise 2.22.x and earlier still require them.

Enterprise compatibility aside, I think many of the issues with combining two versions come from not being able to convert a client of one version to a client of the other. If you could do this, applications could include both library versions and convert between them as needed without re-authenticating or invalidating caches associated with a client. Unfortunately, I haven't thought of a way to implement this without big changes to the github.Client type, which are probably not worth it for this relatively niche use case.

bluekeyes avatar Apr 29 '21 03:04 bluekeyes

Good to know that I am not the only one facing these issues. Thanks, @bluekeyes!

Your comment about how modules are handled totally makes sense.

About the Preview-Headers in GitHub Enterprise: @gmlewis Is there a compatibility rule for go-github which Enterprise Versions this lib supports? Because It might be an easy fix to re-introduce these preview headers with a comment. They don't harm Github.com

andygrunwald avatar Apr 29 '21 07:04 andygrunwald

About the Preview-Headers in GitHub Enterprise: @gmlewis Is there a compatibility rule for go-github which Enterprise Versions this lib supports? Because It might be an easy fix to re-introduce these preview headers with a comment. They don't harm Github.com

Historically, since there are so many versions of GitHub Enterprise "in the wild", this repo has strived to maintain compatibility with the latest public API, but you make an excellent point...

I have no problem re-introducing preview headers with a clear comment that they are there for GitHub Enterprise support as long as they do not interfere with public GitHub functionality. Feel free to submit PRs as long as @willnorris has no objections.

gmlewis avatar Apr 29 '21 11:04 gmlewis

I don't run GitHub Enterprise On-Premise, hence I would like to redirect the Preview-Header-Reintroduction-PR to @bluekeyes.

andygrunwald avatar Apr 29 '21 12:04 andygrunwald

I don't have any objection as long as is doesn't impact other users.

I wonder how difficult it would be to capture and track which versions of GHE a given version of go-github supports? That might end up being a fair amount of work, I honestly don't know.

willnorris avatar Apr 29 '21 21:04 willnorris

I'm running into this problem as well. A solution would be to export the http.Client in github.Client. This would allow easy conversions of clients whenever you have just the client, keeping whatever auth is attached to the http.Client. Then you could do something like:

import (
	"dependency"
	"github.com/google/go-github/v37/github"    // My code uses latest
	gh32 "github.com/google/go-github/v32/github"  // Dependency is on old version
)

func doWork(c *github.Client) {
	gh32c := gh32.NewClient(c.Client)
	dependency.Function(gh32c)
}

jeffmendoza avatar Jul 20 '21 18:07 jeffmendoza

@jeffmendoza - could you please test out #2015 and see if it actually solves your problem?

gmlewis avatar Jul 20 '21 18:07 gmlewis

@gmlewis @jeffmendoza I am not sure if I understand the approach to the original issue. Would someone of you mind explaining this a bit more in detail?

andygrunwald avatar Jul 22 '21 17:07 andygrunwald

@gmlewis @jeffmendoza I am not sure if I understand the approach to the original issue. Would someone of you mind explaining this a bit more in detail?

@andygrunwald - hmmm... looking more closely at the docs for https://pkg.go.dev/github.com/palantir/[email protected]/githubapp I'm not so sure that #2016 actually solved that problem.

Do you agree that this ticket should be re-opened?

gmlewis avatar Jul 22 '21 18:07 gmlewis

Yeah, in my situation I was doing work with a client, then needed to call a dependency that takes a client as well. Easy client version conversion solves that. It doesn't solve any conversion of data types.

jeffmendoza avatar Jul 22 '21 18:07 jeffmendoza

@gmlewis

Do you agree that this ticket should be re-opened?

Yes, I do agree. I am personally not even sure how this can be fixed or if there is a solution at all (after re-reading @bluekeyes comment from above again).

andygrunwald avatar Jul 22 '21 18:07 andygrunwald

The idea is that as long as the thing creating GitHub clients uses a version of go-github with the fix from #2016, you can then transform those clients into any other newer or older version. I'll need to tag a release of palantir/go-githubapp that uses go-github/v37 (and we'll need a release here), but then going forward, you'll have more options to solve this problem.

In your application using version X of go-github:

  1. Create a GitHub client using githubapp.ClientCreator. This has version N (N >= v37.1.0)
  2. Use github.NewClient(clientFromGitHubApp.Client()) to convert the client to version X
  3. Optionally, copy the BaseURL field if you support GitHub Enterprise
  4. Now, all other parts of your app can use data types and clients from version X

Your application should use data types from version X as much as possible. If you need to interface with a library that uses version Y, you can either convert the structs (with a method that copies each relevant field, possibly using reflection if you don't want to write this method for each type) or use the conversion process above to make a client of version Y and then only use that client for the parts that interact with the library.

This is admittedly still awkward and requires extra work for authors, but does avoid forking libraries. I think additional issues will have to be taken up with library maintainers. For example, go-githubapp takes *github.Repository types in some functions. We could maybe change those functions to accept interfaces, allowing them to work with multiple versions of go-github without conversion. GetInstallationIDFromEvent is an example of a function that already does this.

bluekeyes avatar Jul 22 '21 18:07 bluekeyes