Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Closed
bodduv opened this issue Aug 27, 2020 · 5 comments · Fixed by #9333
Closed

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

bodduv opened this issue Aug 27, 2020 · 5 comments · Fixed by #9333
Labels
kind/cleanup Categorizes issue or PR as related to cleaning up code, process, or technical debt. priority/important-longterm Important over the long term, but may not be staffed and/or may need multiple releases to complete.
Milestone

Comments

@bodduv
Copy link

bodduv commented Aug 27, 2020

Currently the Makefile uses an outdated and archived version of go-bindata:

which go-bindata || GO111MODULE=off GOBIN="$(GOPATH)$(DIRSEP)bin" go get github.com/jteeuwen/go-bindata/...

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

@afbjorklund
Copy link
Collaborator

See jteeuwen/go-bindata#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 afbjorklund added kind/cleanup Categorizes issue or PR as related to cleaning up code, process, or technical debt. priority/important-longterm Important over the long term, but may not be staffed and/or may need multiple releases to complete. labels Aug 29, 2020
@afbjorklund
Copy link
Collaborator

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
Copy link
Collaborator

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
Copy link
Collaborator

afbjorklund commented Aug 29, 2020

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
Copy link
Collaborator

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

kubernetes/kubernetes@df3f9f1

$ 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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind/cleanup Categorizes issue or PR as related to cleaning up code, process, or technical debt. priority/important-longterm Important over the long term, but may not be staffed and/or may need multiple releases to complete.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants