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

Compile errors with Go 1.20 #1775

Closed
scottmason88 opened this issue Feb 13, 2023 · 2 comments
Closed

Compile errors with Go 1.20 #1775

scottmason88 opened this issue Feb 13, 2023 · 2 comments
Labels
kind/bug Categorizes issue or PR as related to a bug.

Comments

@scottmason88
Copy link
Contributor

Bug report

Using Go 1.20 and building from source, I'm getting several errors.

pkg/printers/tablegenerator.go:77:2: S1011: should replace loop with `columns = append(columns, handler.columnDefinitions...)` (gosimple)
	for i := range handler.columnDefinitions {
	^
pkg/errors/factory_test.go:172:46: loopclosure: loop variable tc captured by func literal (govet)
			assert.Equal(t, IsForbiddenError(GetError(tc.Error)), tc.Forbidden)
			                                          ^
pkg/errors/factory_test.go:212:56: loopclosure: loop variable tc captured by func literal (govet)
			assert.Equal(t, api_errors.IsInternalError(GetError(tc.Error)), tc.Internal)
			                                                    ^
pkg/errors/factory_test.go:245:21: loopclosure: loop variable tc captured by func literal (govet)
			assert.Assert(t, tc.ErrorType(GetError(tc.Error)))
			                 ^
cmd/kn/main.go:33:2: SA1019: rand.Seed has been deprecated since Go 1.20 and an alternative has been available since Go 1.0: Programs that call Seed and then expect a specific sequence of results from the global random source (using functions such as Int) can be broken when a dependency changes how much it consumes from the global random source. To avoid such breakages, programs that need a specific result sequence should use NewRand(NewSource(seed)) to obtain a random generator that other packages cannot access. (staticcheck)
	rand.Seed(time.Now().UnixNano())
	^
pkg/serving/service_test.go:33:2: SA1019: rand.Seed has been deprecated since Go 1.20 and an alternative has been available since Go 1.0: Programs that call Seed and then expect a specific sequence of results from the global random source (using functions such as Int) can be broken when a dependency changes how much it consumes from the global random source. To avoid such breakages, programs that need a specific result sequence should use NewRand(NewSource(seed)) to obtain a random generator that other packages cannot access. (staticcheck)
	rand.Seed(1)
	^
pkg/serving/service_test.go:48:3: SA1019: rand.Seed has been deprecated since Go 1.20 and an alternative has been available since Go 1.0: Programs that call Seed and then expect a specific sequence of results from the global random source (using functions such as Int) can be broken when a dependency changes how much it consumes from the global random source. To avoid such breakages, programs that need a specific result sequence should use NewRand(NewSource(seed)) to obtain a random generator that other packages cannot access. (staticcheck)
		rand.Seed(1)
		^
exit status 1
--- FAIL: golangci-lint failed please fix the reported errors

Steps to reproduce the problem

Install Go 1.20, and run build.sh

kn version

Version:      v1.9.0
Build Date:   2023-01-26 20:34:03
Git Revision: df40f5a3
Supported APIs:
* Serving
  - serving.knative.dev/v1 (knative-serving v1.9.0)
* Eventing
  - sources.knative.dev/v1 (knative-eventing v1.9.0)
  - eventing.knative.dev/v1 (knative-eventing v1.9.0)

I would like to start contributing to the project, and found this while attempting to build from source for the first time. I would like to be assigned the issue so I can work through the fix.

@scottmason88 scottmason88 added the kind/bug Categorizes issue or PR as related to a bug. label Feb 13, 2023
@scottmason88
Copy link
Contributor Author

I have all but the error in cmd/kn/main.go fixed, and was looking for some direction as to what the best course of action would be. The relevant code is below:

func init() {
	rand.Seed(time.Now().UnixNano())
}

Since this has been deprecated, the 'new' way to do this is rand.New(rand.NewSource(seed)) which returns a Rand object. There are probably a few ways to fix this, and I'm also probably not aware of most of them as this is my first experience with golang. As this is also my first jump into this code, I wanted to have a discussion on the best way to move forward. My two thoughts are:

  1. Use a global, and change all the calls that used this seed to use it.
  2. Have the linter ignore this

Paul Schweigert is mentoring me through this process, and suggested that I tag @rhuss and @dsimansk in this to get their opinion.

Thanks in advance!

@rhuss
Copy link
Contributor

rhuss commented Feb 15, 2023

Hey @scottmason88 , welcome on board :-)

Actually, I'd prefer a third option: Check where rand is used and seed it locally. Afais we are using a random number only in pkg/serving/service.go when creating revision numbers. We can create a package local variable revisionNameRand holding that seeded random variable number, in this package init() method in service.go.

The tricky part Is to update the test in service_test.go as this compares random numbers with the same seed (1), but instead of seeding it, you could replace the revisionNameRand variable with a 1-seeded rand before there tests.

Does this make sense ?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind/bug Categorizes issue or PR as related to a bug.
Projects
None yet
Development

No branches or pull requests

2 participants