search

lima-vm / lima

Linux virtual machines, with a focus on running containers
https://lima-vm.io/
Apache License 2.0
15.36k stars 602 forks source link

Avoid importing multiple YAML libraries #2597

Open AkihiroSuda opened 2 months ago

AkihiroSuda commented 2 months ago

Currently, Lima imports 4 libraries of YAML 😓

https://github.com/lima-vm/lima/blob/52f5ad31e4c1f2a58fc37d1191ceebfdd5150dbe/go.mod#L24

https://github.com/lima-vm/lima/blob/52f5ad31e4c1f2a58fc37d1191ceebfdd5150dbe/go.mod#L48

https://github.com/lima-vm/lima/blob/52f5ad31e4c1f2a58fc37d1191ceebfdd5150dbe/go.mod#L123

https://github.com/lima-vm/lima/blob/52f5ad31e4c1f2a58fc37d1191ceebfdd5150dbe/go.mod#L130

Wish Go had provided stdlib for YAML...

afbjorklund commented 2 months ago

go mod why can be helpful for this.

Some of these dependencies are indirect:

# github.com/goccy/go-yaml
github.com/lima-vm/lima/pkg/instance
github.com/goccy/go-yaml
# gopkg.in/yaml.v3
github.com/lima-vm/lima/pkg/limayaml
gopkg.in/yaml.v3
# gopkg.in/yaml.v2
github.com/lima-vm/lima/pkg/guestagent/kubernetesservice
k8s.io/api/core/v1
k8s.io/apimachinery/pkg/runtime
sigs.k8s.io/structured-merge-diff/v4/value
gopkg.in/yaml.v2
# sigs.k8s.io/yaml
github.com/lima-vm/lima/pkg/guestagent/kubernetesservice
k8s.io/client-go/tools/clientcmd
k8s.io/client-go/tools/clientcmd.test
sigs.k8s.io/yaml

Where yaml.v3 was inherited from yq.

    github.com/goccy/go-yaml v1.12.0
    gopkg.in/yaml.v3 v3.0.1

But I guess you are not keen on a 5th?

afbjorklund commented 2 months ago

The "goccy" version of yqlib has this note:

NOTE this is still a WIP - not yet ready.

So I don't think it is ready to switch yet?

Wish Go had provided stdlib for YAML...

The informal standard is/was go-yaml (v3)

But it has the same lack of progress as json.

So that's the problem of it being "built-in"

afbjorklund commented 2 months ago

We could use yqlib for verification, instead of calling gopkg.in/yaml.v3 directly...

This way, if/when yqlib changes their yaml library it would also be changed in lima?

It would not make a huge difference, but...

@@ -45,7 +45,6 @@ require (
        google.golang.org/grpc v1.66.0
        google.golang.org/protobuf v1.34.2
        gopkg.in/op/go-logging.v1 v1.0.0-20160211212156-b2cb9fa56473
-       gopkg.in/yaml.v3 v3.0.1
        gotest.tools/v3 v3.5.1
        inet.af/tcpproxy v0.0.0-20221017015627-91f861402626
        k8s.io/api v0.31.0
@@ -121,6 +120,7 @@ require (
        gopkg.in/inf.v0 v0.9.1 // indirect
        gopkg.in/tomb.v1 v1.0.0-20141024135613-dd632973f1e7 // indirect
        gopkg.in/yaml.v2 v2.4.0 // indirect
+       gopkg.in/yaml.v3 v3.0.1 // indirect
        gvisor.dev/gvisor v0.0.0-20231023213702-2691a8f9b1cf // indirect
        k8s.io/klog/v2 v2.130.1 // indirect
        k8s.io/kube-openapi v0.0.0-20240228011516-70dd3763d340 // indirect
afbjorklund commented 2 months ago

The library https://github.com/kubernetes-sigs/yaml is a wrapper over yaml.v2 and yaml.v3

But it has forked those libraries, so doesn't depend on the original go modules any more:

https://github.com/kubernetes-sigs/yaml/commit/b96582ba35aa64170bc05e26895fa0a8a12e6d7c

That makes it a bit harder to see the patches, but it was related to compact sequences?

EDIT: Actually, after checking with go mod vendor there doesn't seem to be any code differences...

It uses gopkg.in/yaml.v2@v2.4.0 without any modifcations - except to the README.md and OWNERS

@@ -1,3 +1,13 @@
+# go-yaml fork
+
+This package is a fork of the go-yaml library and is intended solely for consumption
+by kubernetes projects. In this fork, we plan to support only critical changes required for
+kubernetes, such as small bug fixes and regressions. Larger, general-purpose feature requests
+should be made in the upstream go-yaml library, and we will reject such changes in this fork
+unless we are pulling them from upstream.
+
+This fork is based on v2.4.0: https://github.com/go-yaml/yaml/releases/tag/v2.4.0
+
 # YAML support for the Go language

 Introduction

And neither of the Kubernetes (SIG) libraries have migrated to use gopkg.in/yaml.v3 yet.

It even has some opinionated views that one should use the JSON libraries to do YAML!

In short, this library first converts YAML to JSON using go-yaml and then uses json.Marshal and json.Unmarshal to convert to or from the struct. This means that it effectively reuses the JSON struct tags as well as the custom JSON methods MarshalJSON and UnmarshalJSON unlike go-yaml