[oauth2] Remove oauth2 go.mod and go.sum

[pgier]

Motivation

When the oauth2 sub-directory was added in PR #313 there wasn't any justification given (AFAICT) for creating a separate sub-module instead of just treating the oauth2 sources as a normal directory. Since the oauth2 module does not have a separate release cycle, treating it as a sub-module just adds unnecessary complexity.

Modifications

Removed go.mod and go.sum from the oauth2 sub-directory. Updated the main go.mod and go.sum to included the necessary dependencies.

[pgier]

The main module contained a newer version of golang.org/x/oauth2 dependency than the oauth2 sub-module, so I had to update some of the tests to match the output of the newer dependency. This type of dependency mismatch will be avoided in the future using a single module defined in the root directory.

[pgier]

I did a bit of testing just to make sure that this change doesn't break existing apps. AFAICT, a normal go get github.com/apache/pulsar-client-go@latest possibly followed by go mod tidy should work for most users without any additional changes needed.

In some cases, users may see an ambiguous import error after upgrading.

github.com/apache/pulsar-client-go/oauth2: ambiguous import: found package github.com/apache/pulsar-client-go/oauth2 in multiple modules

The fix for this is to remove the indirect dependency line from go.mod, and the run go mod tidy.

github.com/apache/pulsar-client-go/oauth2 v0.0.0-20220630195735-e95cf0633348 // indirect

[liangyuanpeng]

LGTM, and i believe that we need add notes for this problem when release.
In some cases, users may see an ambiguous import error after upgrading.

[pgier]

@liangyuanpeng Thanks for the feedback, I have added some info to the README about the possible error.

[zzzming]

LGTM

[michaeljmarshall]

LGTM

[tisonkun]

IIRC streamnative/pulsar-ctl uses oauth2 only and provides an admin API. It will be affected due to this patch and it proves that there're existing packages depends on the isolated releases.

cc @nodece

[tisonkun]

And we may add some doc (README.md under oauth2?) to say this situation.

[nodece]

IIRC streamnative/pulsar-ctl uses oauth2 only and provides an admin API. It will be affected due to this patch and it proves that there're existing packages depends on the isolated releases.

cc @nodece

Thanks for telling me, that changes should break the pulsarctl.

I verified the pulsarctl, it works fine.

[tisonkun]

@nodece thanks for your validation!

Interesting. I ever thought of you can only add a dep in the top level of go.mod.

[nodece]

@nodece thanks for your validation!

Interesting. I ever thought of you can only add a dep in the top level of go.mod.

When using the go get github.com/apache/pulsar-client-go/oauth2@master, the golang will add top level mod to our project, but it seems to have little effect on us:

❯ go get github.com/apache/pulsar-client-go/oauth2@master
go: added github.com/apache/pulsar-client-go v0.9.0-candidate-1.0.20220812163439-934c867b3727

[tisonkun]

@nodece as long as the whole codebase is small and it isn't polluted in the linking stage, it's fine.

Although, such usage may indicate that oauth2 deserves an isolated release lifecycle. You can trade off the choices. I tend to revert this patch and keep oauth2 a separate go module. Because we accept this patch due to "there wasn't any justification given (AFAICT) for creating a separate sub-module", now we have one.

[pgier]

@tisonkun I tested the pulsar-ctl build, and this change has no immediate effect on it. Go modules are tied to specific checksums, so this change would only be noticeable during the next upgrade. The pulsar-ctl go.mod currently also has a replace directive which prevents upgrade.

Here are the steps to upgrade, if you want to test:

1. remove these two lines from go.mod

   github.com/apache/pulsar-client-go/oauth2 v0.0.0-20211108044248-fe3b7c4e445b
   
   replace github.com/apache/pulsar-client-go/oauth2 => github.com/apache/pulsar-client-go/oauth2 v0.0.0-20211006154457-742f1b107403
   

2. Get the latest commit of pulsar-client-go

   go get github.com/apache/pulsar-client-go@934c867b3727fe231100715d04e363992ca5ea38
   

3. Run go mod tidy
4. Build as normal make pulsarctl

Although, such usage may indicate that oauth2 deserves an isolated release lifecycle. You can trade off the choices. I tend to revert this patch and keep oauth2 a separate go module. Because we accept this patch due to "there wasn't any justification given (AFAICT) for creating a separate sub-module", now we have one.

I don't think this justifies the need for a sub-module. Go is pretty efficient in terms of using only the code that it needs, so depending on the root module instead of the oauth2 module should not add any additional code/size to your binary. The oauth2 module has never actually had an isolated release lifecycle (separate release process and version), it's always just been released along with the main project.

[tisonkun]

@pgier thanks for your explanation! I'll try to learn how go.mod works nowadays in details :)

Actually, such an assumption prevents me reuse a package inner inside vitess and I'm happy to see there's no restriction.

[tisonkun]

@pgier I read your comment now. And it seems my statement above holds:

As long as the whole codebase is small and it isn't polluted in the linking stage, it's fine.

Out of topic, still, I can hardly reuse a package inner inside vitess since the top level mod is a relative large codebase to pull.