search

ugorji / go

idiomatic codec and rpc lib for msgpack, cbor, json, etc. msgpack.org[Go]
MIT License
1.85k stars 295 forks source link

Remove circular dependency on codec submodule #319

Closed dlaguerta closed 3 years ago

dlaguerta commented 4 years ago

Requiring the root module in the codec submodule creates a circular dependency, which causes go mod issues. By removing it as required in codec/go.mod, I believe people should not see issues.

dlaguerta commented 4 years ago

@ugorji -- could you take a look?

ugorji commented 4 years ago

I can't accept this PR without understanding what it did, exactly what it solves, or the implications.

1995parham commented 4 years ago

@ugorji I think this PR resolve #318 by removing the circular dependency between Go modules that are defined in the project.

dlaguerta commented 4 years ago

Like @1995parham mentioned, clients using this module are getting errors due to the circular dependency within this project (#318). The circular dependency defined in the go.mod mod file of go/codec also causes inconsistent changes to a client's go.mod file when nothing has actually changed. For example, when running go test, clients will see this change:

-   github.com/ugorji/go/codec v1.1.7 // indirect
+   github.com/ugorji/go v1.1.7 // indirect

running go mod tidy will revert this change, but clients would have to always run go mod tidy after, for example, running go test. The better fix would be to just remove the dependency on the root module (ugoriji/go) from the codec mod. This PR resolves #318.

ChrisRx commented 4 years ago

This change would be greatly appreciated. The Go modules defined within the project are setup incorrectly and this would go a long way to helping with some of the downstream problems intrinsic to Go modules themselves that people are experiencing.

With that said, after reading through a lot of the issues to get backstory, I think a overall better solution would be to also only use one Go modules at the project root, place a doc.go file (or whatever), and have codec be a sub-package (and bumping the module to v2, which would be very important). All of the headaches experienced with the project with regard to Go modules are due to the incorrect usage and subsequent definition of multiple Go modules also, so I think if the v2 could just eliminate the unnecessary use of multiple modules (and the unnecessary folder structure) it would help to guard against these issues in the future.

In any case, I would really like to see this PR get merged to address the go mod tidy issue described and would like to help in any way that I can. Thank you for your time and patience.

ugorji commented 3 years ago

Please see thread #299

This gives the context and history, explaining what influenced the decision to make the committed fix.

It seems that things are working well now. Closing. Please ping/reopen if needed.