refraction-networking/conjure

Change the import rather than relying on the replace directive for dtls modifications

cohosh opened this issue · 3 comments

cohosh commented

There is a quirk with Go's replace directive where it is only applied if the directive appears in the go.mod file of the main program, but not if it is in the go.mod file of a dependency.

In a scenario where another Go program is using the conjure/pkg/registration library (as in the Tor Conjure PT), a build will fail with the following errors:

# github.com/refraction-networking/conjure/pkg/dtls
../../../../go/pkg/mod/github.com/refraction-networking/conjure@v0.6.7/pkg/dtls/dial.go:64:3: unknown field CustomClientHelloRandom in struct literal of type "github.com/pion/dtls/v2".Config
../../../../go/pkg/mod/github.com/refraction-networking/conjure@v0.6.7/pkg/dtls/listener.go:63:24: connState.RemoteRandomBytes undefined (type "github.com/pion/dtls/v2".State has no field or method RemoteRandomBytes)
../../../../go/pkg/mod/github.com/refraction-networking/conjure@v0.6.7/pkg/dtls/listener.go:137:34: state.RemoteRandomBytes undefined (type *"github.com/pion/dtls/v2".State has no field or method RemoteRandomBytes)
../../../../go/pkg/mod/github.com/refraction-networking/conjure@v0.6.7/pkg/dtls/listener.go:236:40: clientHello.RandomBytes undefined (type *"github.com/pion/dtls/v2".ClientHelloInfo has no field or method RandomBytes)

Because the directive

replace github.com/pion/dtls/v2 => github.com/mingyech/dtls/v2 v2.0.0

fails to apply.

This can be fixed by adding the directive to the go.mod file of the calling program, but it would be cleaner to just change the import where it's used, especially since this is a long-term change.

This isn't urgent, just a nice to have :)

We are working on a better solution to this issue. The decision to use the replace over modified imports was made so that we wouldn't have to fork (and keep up to date) more of the pion modules relating to dtls. The use of the replace is intended to be a temporary fix.

The plan is to attempt to upstream our changes to the respective pion repositories, at which point the replace would not be necessary at all. Failing that though, we will fork the remaining pion libraries and switch to imports.

cohosh commented

Thanks for the update! That sounds like a good plan.

pion/dtls#583 should let us remove the replace if it gets merged! The remaining replace for pion/transport will be unneeded and removed as well.