kubernetes / minikube

Run Kubernetes locally
https://minikube.sigs.k8s.io/
Apache License 2.0
29.49k stars 4.89k forks source link

Use maintained go-bindata instead of an archived version #9107

Closed bodduv closed 4 years ago

bodduv commented 4 years ago

Currently the Makefile uses an outdated and archived version of go-bindata: https://github.com/kubernetes/minikube/blob/214ff5b33f20bc08ee3037ebca66b5bda2eb15a4/Makefile#L311

Someone else has volunteered to maintain this package, see influxdata/chronograf#2785 and here.

afbjorklund commented 4 years ago

See https://github.com/jteeuwen/go-bindata/issues/5 for background (account went "missing")

Most of the forking happened around what kevinburke calls 3.2-3.3

* 1cc2e96 (tag: v3.3.0, tag: test) 3.3.0
* 1a5fad7 (kevinburke/run-differ) Check whether a testdata diff has been generated
* f092eb0 (kevinburke/add-tests) Add safefile tests and update LICENSE
* 001dbf2 (kevinburke/authors) Add AUTHORS file
* d92efe3 (kevinburke/atomic-write) Implement atomic file write
* caa3edd (kevinburke/fix-typo) Fix spelling mistake
* e338d3e (kevinburke/return-right-error) Fix error in bindataRead
| * 6025e8d (HEAD -> master, jteeuwen/master, jteeuwen/HEAD) Add notice for discussion area
| | *   df38da1 (a-urth/master) Merge pull request #3 from paralax/patch-2
| | |\  
| | | * 3b92035 spelling fixes, no content changes
| | |/  
| | *   616077d Merge pull request #2 from RyanBrushett/update-readme
| | |\  
| | | * 959d767 Update README.md install instructions
| | |/  
| | * 300725a Change package name
| |/  
| | * 40153cf (kevinburke/bazel) Add Bazel support
| |/  
|/|   
* | 318b9e4 (tag: v3.2.0) 3.2.0
* | b9c70f2 (kevinburke/version) Clean up version printing
* | 12449d9 (kevinburke/patch-1) Remove useless conditional
* | c65a1b1 (kevinburke/matt-master) Make the output file compatible with gofmt
* | 5a0b4d6 (kevinburke/dim13-master) Use formalized DO NOT EDIT marker
* | ee7c487 Removed a duplicate word
* | 1950f31 (kevinburke/travis) Add Travis CI
|/  
| * f70fbe2 (kevinburke/simplify) Simplify RestoreAsset
|/  
*   a0ff256 Merge pull request #115 from grobie/fix-go-fmt

Hopefully it would make for a drop-in replacement for generated data.

afbjorklund commented 4 years ago

Need to look at the differences, seems to be mostly whitespace etc.

 assets/assets.go          |  544 ++++++++++++++++++++++++++++++++++++++++++++++++------------------------------------------
 translate/translations.go |  136 ++++++++++++++--------
 2 files changed, 380 insertions(+), 300 deletions(-)
afbjorklund commented 4 years ago

Comparing with the older version (and ignoring whitespace and blank lines) makes diff smaller. Looks OK.

--- pkg/minikube/assets/assets.go.orig  2020-08-29 15:12:15.403044261 +0200
+++ pkg/minikube/assets/assets.go   2020-08-29 15:17:16.804595218 +0200
@@ -1,4 +1,4 @@
-// Code generated by go-bindata.
+// Code generated by go-bindata. DO NOT EDIT.
 // sources:
 // deploy/addons/ambassador/ambassador-operator-crds.yaml
 // deploy/addons/ambassador/ambassador-operator.yaml
@@ -76,7 +76,6 @@
 // deploy/addons/storage-provisioner-gluster/storage-gluster-ns.yaml.tmpl
 // deploy/addons/storage-provisioner-gluster/storage-provisioner-glusterfile.yaml.tmpl
 // deploy/addons/storageclass/storageclass.yaml.tmpl
-// DO NOT EDIT!

 package assets

@@ -100,13 +99,14 @@

    var buf bytes.Buffer
    _, err = io.Copy(&buf, gz)
-   clErr := gz.Close()

    if err != nil {
        return nil, fmt.Errorf("Read %q: %v", name, err)
    }
+
+   clErr := gz.Close()
    if clErr != nil {
-       return nil, err
+       return nil, clErr
    }

    return buf.Bytes(), nil
@@ -1667,8 +1667,8 @@
 // It returns an error if the asset could not be found or
 // could not be loaded.
 func Asset(name string) ([]byte, error) {
-   cannonicalName := strings.Replace(name, "\\", "/", -1)
-   if f, ok := _bindata[cannonicalName]; ok {
+   canonicalName := strings.Replace(name, "\\", "/", -1)
+   if f, ok := _bindata[canonicalName]; ok {
        a, err := f()
        if err != nil {
            return nil, fmt.Errorf("Asset %s can't read by error: %v", name, err)
@@ -1693,8 +1693,8 @@
 // It returns an error if the asset could not be found or
 // could not be loaded.
 func AssetInfo(name string) (os.FileInfo, error) {
-   cannonicalName := strings.Replace(name, "\\", "/", -1)
-   if f, ok := _bindata[cannonicalName]; ok {
+   canonicalName := strings.Replace(name, "\\", "/", -1)
+   if f, ok := _bindata[canonicalName]; ok {
        a, err := f()
        if err != nil {
            return nil, fmt.Errorf("AssetInfo %s can't read by error: %v", name, err)
@@ -1809,8 +1884,8 @@
 func AssetDir(name string) ([]string, error) {
    node := _bintree
    if len(name) != 0 {
-       cannonicalName := strings.Replace(name, "\\", "/", -1)
-       pathList := strings.Split(cannonicalName, "/")
+       canonicalName := strings.Replace(name, "\\", "/", -1)
+       pathList := strings.Split(canonicalName, "/")
        for _, p := range pathList {
            node = node.Children[p]
            if node == nil {
@@ -1996,11 +2071,7 @@
    if err != nil {
        return err
    }
-   err = os.Chtimes(_filePath(dir, name), info.ModTime(), info.ModTime())
-   if err != nil {
-       return err
-   }
-   return nil
+   return os.Chtimes(_filePath(dir, name), info.ModTime(), info.ModTime())
 }

 // RestoreAssets restores an asset under the given directory recursively
@@ -2021,6 +2092,6 @@
 }

 func _filePath(dir, name string) string {
-   cannonicalName := strings.Replace(name, "\\", "/", -1)
-   return filepath.Join(append([]string{dir}, strings.Split(cannonicalName, "/")...)...)
+   canonicalName := strings.Replace(name, "\\", "/", -1)
+   return filepath.Join(append([]string{dir}, strings.Split(canonicalName, "/")...)...)
 }
--- pkg/minikube/translate/translations.go.orig 2020-08-29 15:12:15.479043643 +0200
+++ pkg/minikube/translate/translations.go  2020-08-29 15:17:16.916594307 +0200
@@ -1,4 +1,4 @@
-// Code generated by go-bindata.
+// Code generated by go-bindata. DO NOT EDIT.
 // sources:
 // translations/de.json
 // translations/es.json
@@ -8,7 +8,6 @@
 // translations/pl.json
 // translations/strings.txt
 // translations/zh-CN.json
-// DO NOT EDIT!

 package translate

@@ -32,13 +31,14 @@

    var buf bytes.Buffer
    _, err = io.Copy(&buf, gz)
-   clErr := gz.Close()

    if err != nil {
        return nil, fmt.Errorf("Read %q: %v", name, err)
    }
+
+   clErr := gz.Close()
    if clErr != nil {
-       return nil, err
+       return nil, clErr
    }

    return buf.Bytes(), nil
@@ -239,8 +239,8 @@
 // It returns an error if the asset could not be found or
 // could not be loaded.
 func Asset(name string) ([]byte, error) {
-   cannonicalName := strings.Replace(name, "\\", "/", -1)
-   if f, ok := _bindata[cannonicalName]; ok {
+   canonicalName := strings.Replace(name, "\\", "/", -1)
+   if f, ok := _bindata[canonicalName]; ok {
        a, err := f()
        if err != nil {
            return nil, fmt.Errorf("Asset %s can't read by error: %v", name, err)
@@ -265,8 +265,8 @@
 // It returns an error if the asset could not be found or
 // could not be loaded.
 func AssetInfo(name string) (os.FileInfo, error) {
-   cannonicalName := strings.Replace(name, "\\", "/", -1)
-   if f, ok := _bindata[cannonicalName]; ok {
+   canonicalName := strings.Replace(name, "\\", "/", -1)
+   if f, ok := _bindata[canonicalName]; ok {
        a, err := f()
        if err != nil {
            return nil, fmt.Errorf("AssetInfo %s can't read by error: %v", name, err)
@@ -313,8 +320,8 @@
 func AssetDir(name string) ([]string, error) {
    node := _bintree
    if len(name) != 0 {
-       cannonicalName := strings.Replace(name, "\\", "/", -1)
-       pathList := strings.Split(cannonicalName, "/")
+       canonicalName := strings.Replace(name, "\\", "/", -1)
+       pathList := strings.Split(canonicalName, "/")
        for _, p := range pathList {
            node = node.Children[p]
            if node == nil {
@@ -368,11 +375,7 @@
    if err != nil {
        return err
    }
-   err = os.Chtimes(_filePath(dir, name), info.ModTime(), info.ModTime())
-   if err != nil {
-       return err
-   }
-   return nil
+   return os.Chtimes(_filePath(dir, name), info.ModTime(), info.ModTime())
 }

 // RestoreAssets restores an asset under the given directory recursively
@@ -393,6 +396,6 @@
 }

 func _filePath(dir, name string) string {
-   cannonicalName := strings.Replace(name, "\\", "/", -1)
-   return filepath.Join(append([]string{dir}, strings.Split(cannonicalName, "/")...)...)
+   canonicalName := strings.Replace(name, "\\", "/", -1)
+   return filepath.Join(append([]string{dir}, strings.Split(canonicalName, "/")...)...)
 }
afbjorklund commented 4 years ago

There is also a https://github.com/go-bindata/go-bindata organization, that has an older fork...

They call their releases "3.1", which I suppose was why the numbering started over with "3.2".

*   dcdd2e5 (tag: v3.1.0) Merge pull request #18 from alimy/master
|\  
| * 22f7e2b support generate instance http.FileSystem interface code
* |   892c73e Merge pull request #10 from krhubert/master
|\ \  
| |/  
|/|   
| * 0561df6 Satisfy gofmt + golint
|/  
*   41975c0 Merge pull request #8 from csimons/master
|\  
| * 59fc9bc make output conform to gofmt standard
* |   6518908 Merge pull request #2 from nathanjordan/nathan/fix_generated_comment
|\ \  
| |/  
|/|   
| * fbb4dc4 move @generated comment to respect go standard
|/  
*   d266f3a Merge pull request #1 from nathanjordan/nathan/add_phabricator_generated_tag
|\  
| * e1b4998 Add @generated tag to generated bindata file
|/  
*   987f604 Merge branch 'master' of https://github.com/go-bindata/go-bindata
|\  
| * 89a54e7 Update README.md
* | efcd773 format code
|/  
* 4b0d0c5 add goreportcard badge
* 5ebd2c0 (tag: v1.0.0) Update README.md
* 2605bf7 fix install
* 867b676 Fix golint fail
| * 6025e8d (jteeuwen/master, jteeuwen/HEAD, master) Add notice for discussion area
|/  
| * 318b9e4 (tag: v3.2.0) 3.2.0
| * b9c70f2 (kevinburke/version) Clean up version printing
| * 12449d9 (kevinburke/patch-1) Remove useless conditional
| * c65a1b1 (kevinburke/matt-master) Make the output file compatible with gofmt
| * 5a0b4d6 (kevinburke/dim13-master) Use formalized DO NOT EDIT marker
| * ee7c487 Removed a duplicate word
| * 1950f31 (kevinburke/travis) Add Travis CI
|/  
* a0ff256 Merge pull request #115 from grobie/fix-go-fmt
afbjorklund commented 4 years ago

I think we will end up going with github.com/go-bindata/go-bindata since that is what kubernetes is doing:

https://github.com/kubernetes/kubernetes/commit/df3f9f1047ec4a3fcb53b241b63489df72308d0c

$ grep go-bindata go.sum
github.com/go-bindata/go-bindata v3.1.1+incompatible h1:tR4f0e4VTO7LK6B2YWyAoVEzG9ByG1wrXB4TL9+jiYg=
github.com/go-bindata/go-bindata v3.1.1+incompatible/go.mod h1:xK8Dsgwmeed+BBsSy2XTopBn/8uK2HWuGSnA11C3Joo=
$ go-bindata version
go-bindata 3.1.3 (Go runtime go1.14.6).
Copyright (c) 2010-2013, Jim Teeuwen.