Add home directory to config file search paths #1325

System-Glitch · 2020-08-21T14:48:55Z

Guidance required: tests don't pass if you have a configuration file in your home directory. This is because some tests check that config is not found. I need assistance on how to update tests properly.

boring-cyborg · 2020-08-21T14:48:57Z

Hey, thank you for opening your first Pull Request !

CLAassistant · 2020-08-21T14:49:01Z

All committers have signed the CLA.

SVilgelm · 2020-08-21T14:57:36Z

So, dumb question, but I must ask it: have you test it on your env? Since we cannot write the unit test for this. And I cannot test it on my laptop, I don't have external adds or usb sticks

System-Glitch · 2020-08-21T15:00:27Z

I have tested it but have not written automated tests for it.

From another drive (/mnt/6C161AE6161AB156), I ran it and got

$ ./golangci-lint run -v
INFO [config_reader] Config search paths: [./ /mnt/6C161AE6161AB156 /mnt / /home/jeremy] 
INFO [config_reader] Used config file ../../home/jeremy/.golangci.yml 
INFO [lintersdb] Active 14 linters: [deadcode errcheck gocyclo gofmt golint gosimple govet ineffassign misspell staticcheck structcheck typecheck unused varcheck] 
INFO [loader] Go packages loading at mode 575 (compiled_files|imports|types_sizes|deps|exports_file|files|name) took 446.118075ms 
ERRO Running error: context loading failed: no go files to analyze 
INFO Memory: 6 samples, avg is 72.8MB, max is 72.8MB 
INFO Execution took 449.689682ms

With v1.30.0:

$ golangci-lint run -v
INFO [config_reader] Config search paths: [./ /mnt/6C161AE6161AB156 /mnt /] 
INFO [lintersdb] Active 10 linters: [deadcode errcheck gosimple govet ineffassign staticcheck structcheck typecheck unused varcheck] 
INFO [loader] Go packages loading at mode 575 (imports|compiled_files|exports_file|files|name|types_sizes|deps) took 580.5053ms 
ERRO Running error: context loading failed: no go files to analyze 
INFO Memory: 7 samples, avg is 72.1MB, max is 72.1MB 
INFO Execution took 583.579037ms

/home/jeremy/.golanci.yml:

run:
  skip-dirs:
    - docs_src
    - docs
    - .github

linters-settings:
  gocyclo:
    min-complexity: 15
  gofmt:
    simplify: true
  misspell:
    locale: US

linters:
  enable:
    - gofmt
    - golint
    - gocyclo
    - misspell
  disable-all: false
  fast: false

issues:
  exclude-use-default: false

You can notice the additional linters I enabled are correctly enabled.

SVilgelm · 2020-08-21T15:05:27Z

Cool, thanks

System-Glitch · 2020-08-21T15:07:17Z

So, we leave it like that and write no tests?

bombsimon · 2020-08-21T15:12:17Z

Another question just to be sure since I haven't dug into the code, is the search order like this now so /home doesn't come after /mnt/path/go but before /mnt/path? Because I think that could be breaking for some users.

Given code in /mnt/path/go

/mnt/path/go
/mnt/path
/mnt
/
/home

SVilgelm · 2020-08-21T15:12:26Z

We can write the test, but it will require some refactoring. viper.SetConfigName(".golangci") must be changed and the ".golangci" must be moved to a variable, so it's not a constant. Then unit tests can change that variable and generate the config files and put them to the all possible folders and test that the expected file is used.

SVilgelm · 2020-08-21T15:14:37Z

it will work in this order:

/mnt/path/go
/mnt/path
/mnt
/
/home

the /home is the latest folder

System-Glitch · 2020-08-21T15:15:56Z

@bombsimon The home directory is added last, so it should be checked last.
See my comment above:

INFO [config_reader] Config search paths: [./ /mnt/6C161AE6161AB156 /mnt / /home/jeremy]

However, there is a possibility for duplicate paths (effectively checking the same path twice). I can add a check to prevent this if needed.

Proof:

$ ./golangci-lint run -v
INFO [config_reader] Config search paths: [./ /home/jeremy/Projects/go/randomproject /home/jeremy/Projects/go /home/jeremy/Projects /home/jeremy /home / /home/jeremy]

@SVilgelm I am still a bit lost on how I could write those tests. Maybe it can be merged and tests added later by the core team?

System-Glitch · 2020-08-21T15:24:48Z

Duplicate paths fixed:

$ ./golangci-lint run -v
INFO [config_reader] Config search paths: [./ /home/jeremy/Projects/go/randomproject /home/jeremy/Projects/go /home/jeremy/Projects /home/jeremy /home /]

SVilgelm

Looks good to me. Thank you

sayboras · 2020-08-22T03:18:04Z

pkg/config/reader.go

 	r.log.Infof("Config search paths: %s", configSearchPaths)
 	viper.SetConfigName(".golangci")
 	for _, p := range configSearchPaths {
 		viper.AddConfigPath(p)
 	}
 }

+func sliceContains(slice []string, value string) bool {


I can't find where this function is used.

https://github.com/golangci/golangci-lint/pull/1325/files/b5ca52de4a96d08869900d387607b8244d5e9986#diff-e85ca4ec86b591a1db43740ce03c8773R187

I think it would be better to move to slice utils or so, so we can reuse it later

oops, my bad :(

it's needed to avoid duplications in the list of folders

Should I create a new package pkg/sliceutil and put it there?

yeah, it would be better if you move it to pkg/sliceutil

pkg/config/reader.go

sayboras

LGTM

bombsimon

LGTM

golangci-automator · 2020-08-24T06:44:39Z

Hey, @System-Glitch — we just merged your PR to golangci-lint! 🔥🚀

golangci-lint is built by awesome people like you. Let us say “thanks”: we just invited you to join the GolangCI organization on GitHub.
This will add you to our team of maintainers. Accept the invite by visiting this link.

By joining the team, you’ll be able to label issues, review pull requests, and merge approved pull requests.
More information about contributing is here.

Thanks again!

System-Glitch added 2 commits August 21, 2020 16:41

Add home directory to config file search paths #1324

48f9b94

Document home directory config search path

4255b97

System-Glitch changed the title ~~WIP: Add home directory to config file search paths~~ Add home directory to config file search paths Aug 21, 2020

Prevent duplicate home path in config search paths

b5ca52d

SVilgelm approved these changes Aug 22, 2020

View reviewed changes

sayboras reviewed Aug 22, 2020

View reviewed changes

sayboras requested changes Aug 22, 2020

View reviewed changes

pkg/config/reader.go Outdated Show resolved Hide resolved

sayboras reviewed Aug 22, 2020

View reviewed changes

pkg/config/reader.go Outdated Show resolved Hide resolved

SVilgelm reviewed Aug 22, 2020

View reviewed changes

pkg/config/reader.go Outdated Show resolved Hide resolved

Print home directory error instead of empty string

2481cdf

sayboras approved these changes Aug 23, 2020

View reviewed changes

bombsimon approved these changes Aug 23, 2020

View reviewed changes

sayboras merged commit 13c2a34 into golangci:master Aug 24, 2020

System-Glitch deleted the home-global-config branch August 24, 2020 08:29

ldez added this to the v1.31 milestone Mar 6, 2024

Uh oh!

Add home directory to config file search paths #1325

Add home directory to config file search paths #1325

Uh oh!

Conversation

System-Glitch commented Aug 21, 2020

Uh oh!

boring-cyborg bot commented Aug 21, 2020

Uh oh!

CLAassistant commented Aug 21, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

SVilgelm commented Aug 21, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

System-Glitch commented Aug 21, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

SVilgelm commented Aug 21, 2020

Uh oh!

System-Glitch commented Aug 21, 2020

Uh oh!

bombsimon commented Aug 21, 2020

Uh oh!

SVilgelm commented Aug 21, 2020

Uh oh!

SVilgelm commented Aug 21, 2020

Uh oh!

System-Glitch commented Aug 21, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

System-Glitch commented Aug 21, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

SVilgelm left a comment

Choose a reason for hiding this comment

Uh oh!

sayboras Aug 22, 2020

Choose a reason for hiding this comment

Uh oh!

SVilgelm Aug 22, 2020

Choose a reason for hiding this comment

Uh oh!

SVilgelm Aug 22, 2020

Choose a reason for hiding this comment

Uh oh!

sayboras Aug 22, 2020

Choose a reason for hiding this comment

Uh oh!

SVilgelm Aug 22, 2020

Choose a reason for hiding this comment

Uh oh!

System-Glitch Aug 22, 2020

Choose a reason for hiding this comment

Uh oh!

SVilgelm Aug 23, 2020

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

sayboras left a comment

Choose a reason for hiding this comment

Uh oh!

bombsimon left a comment

Choose a reason for hiding this comment

Uh oh!

golangci-automator bot commented Aug 24, 2020

Uh oh!

Uh oh!

CLAassistant commented Aug 21, 2020 •

edited

Loading

SVilgelm commented Aug 21, 2020 •

edited

Loading

System-Glitch commented Aug 21, 2020 •

edited

Loading

System-Glitch commented Aug 21, 2020 •

edited

Loading

System-Glitch commented Aug 21, 2020 •

edited

Loading