cue-lang / cue

The home of the CUE language! Validate and define text-based and dynamic configuration
https://cuelang.org
Apache License 2.0
4.9k stars 278 forks source link

pkg: slowly move the Go APIs for CUE builtin funcs under internal/pkg #3218

Open mvdan opened 3 weeks ago

mvdan commented 3 weeks ago

We have Go APIs in packages like https://pkg.go.dev/cuelang.org/go/encoding/yaml, which are meant to be used directly in Go, as well as similar Go APIs in pkg/... packages like https://pkg.go.dev/cuelang.org/go/pkg/encoding/yaml, which are meant to be used as builtins in CUE code.

This works fine today; at init time, packages in pkg/... register their builtin functions so that they may be called.

The Go APIs in pkg/... packages reflect what the builtins need to do to work on CUE values as part of an evaluation, so they may not be the most human-friendly APIs. Moreover, we may need to change them from time to time to improve the builtins or fix bugs in them, such as https://review.gerrithub.io/c/cue-lang/cue/+/1194425 swapping the signature of pkg/encoding/yaml.Validate so that it can handle incomplete CUE values. Note that changes like these would not break any CUE code, but they may break Go code.

Strictly speaking, there doesn't appear to be a good reason to expose the pkg/... packages; they could be under internal/pkg/.... Unfortunately, they have been exposed for years now, so there appear to be hundreds of existing users: https://github.com/search?q=import+cuelang.org%2Fgo%2Fpkg%2F+language%3AGo&type=code

Why are they using pkg/... Go APIs directly like pkg/encoding/yaml rather than their non-builtin counterparts like encoding/yaml? It could be simply an oversight, for example https://pkg.go.dev/cuelang.org/go/pkg/encoding/yaml#Marshal could be switched with https://pkg.go.dev/cuelang.org/go/encoding/yaml#Encode rather easily. However, other APIs like https://pkg.go.dev/cuelang.org/go/pkg/encoding/yaml#ValidatePartial don't have direct counterparts.

Keeping pkg/... public makes improving or iterating on the builtin functions harder in the future. Adding more CUE builtins forcibly means we have to expose and maintain public APIs, and if we were to remove or rename a CUE API which can be resolved with cue fix, we would still be left with the Go API breakage. And, as explained before, any changes to the Go APIs which don't break CUE users could still break Go API users.

As painful as it might be, I propose that we start deprecating and slowly moving pkg/... packages to internal/pkg/.... For example, in v0.10 we could deprecate all of those APIs, suggesting that users switch to the non-pkg package counterparts, and raising issues with us if they find any missing APIs so we can discuss and design to fill the gaps. And future releases like v0.11 and v0.12 would start moving some of the pkg packages to internal, perhaps three or four per release to deal with the churn in stages.

mvdan commented 3 weeks ago

In terms of API surface, it's worth noting that a significant amount is a direct copy of Go's API, such as https://pkg.go.dev/cuelang.org/go/pkg/strings being a relatively old snapshot of https://pkg.go.dev/strings. In such cases, there should be zero reason for any Go user to use our Go APIs over directly using Go's from its standard library. And there's also zero reason for our CUE module to have all of this extra API surface which duplicates with Go's own standard library.

mvdan commented 3 weeks ago

@myitcv points out that internal packages aren't treated very well by pkg.go.dev - they don't show up under searches (e.g. https://pkg.go.dev/search?q=astinternal not showing https://pkg.go.dev/cuelang.org/go@v0.9.0/internal/astinternal) and they don't show up in package lists by default (e.g. https://pkg.go.dev/cuelang.org/go#section-directories having "show internal").

That's a valid point, given that e.g. https://pkg.go.dev/cuelang.org/go/pkg/list is the only good place where a user can currently read the docs for those CUE builtins. So perhaps once we have a different place to render our CUE builtin docs (perhaps the central registry once rendering of CUE docs is finished), and especially on the way to v1, we should move pkg/... to internal/pkg/....

But until we have an alternative, moving the packages today doesn't seem appropriate.