git clone git@github.com:gabolaev/gl-issue-reproduction.git&&cd$_golangci-lint cache cleangit checkout HEAD.; golangci-lint run; sed -i's/some text/SOME OTHER TEXT/g' somepackage/somefile.go; golangci-lint run# git checkout HEAD .; ./golangci-lint run; sed -i 's/some text/SOME OTHER TEXT/g' somepackage/somefile.go; ./golangci-lint run

If it works you should see:

somepackage/somefile.go:9:2: printf: (*github.com/sirupsen/logrus.Logger).Warnf format %v reads arg #1, but call has 0 args (govet)        log.Warnf("some text %v") // this line is supposed to be reported by govet printf linter        ^somepackage/somefile.go:9:2: printf: (*github.com/sirupsen/logrus.Logger).Warnf format %v reads arg #1, but call has 0 args (govet)        log.Warnf("SOME OTHER TEXT %v") // this line is supposed to be reported by govet printf linter        ^

but if it not works:

somepackage/somefile.go:9:2: printf: (*github.com/sirupsen/logrus.Logger).Warnf format %v reads arg #1, but call has 0 args (govet)        log.Warnf("some text %v") // this line is supposed to be reported by govet printf linter        ^

This will impact negatively the performance of a run with a cache but it will increase the stability of reports.

Fixes#4423

ldez added bug

Something isn't working

area: cache labels

Feb 27, 2024

ldez force-pushed thefix/cache-imports branch 2 times, most recently from849f21e toccf27b4Compare

February 27, 2024 21:29

ldez requested review fromAntonboom andbombsimon

February 28, 2024 22:24

bombsimon approved these changes

Feb 28, 2024

View reviewed changes

Copy link

Member

bombsimon left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others.Learn more.

Did you do any tests (manual or otherwise) on a somewhat larger codebase and/or with a lot of local imports? Are we talking millisecond increases or can this become a cardinality bomb and grow much bigger than that?

The fix looks good and is understandable so I'll approve this since I have full trust in your judgement here 😄

Copy link

MemberAuthor

ldez commentedFeb 28, 2024•
edited
Loading

Did you do any tests (manual or otherwise) on a somewhat larger codebase and/or with a lot of local imports?

I didn't do a benchmark because this depends on how many direct packages a package depends on, so it's difficult to create accurate benchmarks.

Are we talking millisecond increases or can this become a cardinality bomb and grow much bigger than that?

As it just adds the direct packages related to one analyzed package and not the indirect packages, the cardinality is limited.

But if a package depends on thousands of local packages (I hope nobody does that), the impact will be important on performances (only for the run after the first one) but I don't have a better solution.

EDIT: I have some doubts maybe the real problem is the linter 🤔