Multiple directories do not sum correctly
Kerollmops opened this issue · 7 comments
Describe the bug
When using scc on multiple directories doesn't sum both directories independently.
To Reproduce
Documents; scc meilisearch
───────────────────────────────────────────────────────────────────────────────
Language Files Lines Blanks Comments Code Complexity
───────────────────────────────────────────────────────────────────────────────
Rust 372 114690 11683 6453 96554 5299
TOML 27 1066 134 117 815 2
YAML 23 1911 158 122 1631 0
Markdown 15 1347 315 0 1032 0
JSON 11 4088 0 0 4088 0
SVG 5 108 0 1 107 0
Shell 5 308 29 32 247 10
JSONL 2 79 0 0 79 0
Docker ignore 1 4 0 0 4 0
Dockerfile 1 48 12 9 27 3
License 1 21 4 0 17 0
Python 1 25 6 7 12 4
───────────────────────────────────────────────────────────────────────────────
Total 464 123695 12341 6741 104613 5318
───────────────────────────────────────────────────────────────────────────────
Estimated Cost to Develop (organic) $3 565 826
Estimated Schedule Effort (organic) 22,30 months
Estimated People Required (organic) 14,21
───────────────────────────────────────────────────────────────────────────────
Processed 4607051 bytes, 4.607 megabytes (SI)
───────────────────────────────────────────────────────────────────────────────
Documents; scc heed
───────────────────────────────────────────────────────────────────────────────
Language Files Lines Blanks Comments Code Complexity
───────────────────────────────────────────────────────────────────────────────
Rust 40 10505 944 3770 5791 426
C 14 14631 1267 2142 11222 2785
TOML 7 207 21 47 139 1
License 3 89 19 0 70 0
C Header 2 1855 120 1467 268 2
Markdown 2 66 18 0 48 0
Plain Text 2 135 10 0 125 0
Makefile 1 119 20 21 78 7
YAML 1 96 7 1 88 0
───────────────────────────────────────────────────────────────────────────────
Total 72 27703 2426 7448 17829 3221
───────────────────────────────────────────────────────────────────────────────
Estimated Cost to Develop (organic) $556 261
Estimated Schedule Effort (organic) 11,01 months
Estimated People Required (organic) 4,49
───────────────────────────────────────────────────────────────────────────────
Processed 926376 bytes, 0.926 megabytes (SI)
───────────────────────────────────────────────────────────────────────────────
Documents; scc meilisearch heed
───────────────────────────────────────────────────────────────────────────────
Language Files Lines Blanks Comments Code Complexity
───────────────────────────────────────────────────────────────────────────────
Rust 80 21010 1888 7540 11582 852
C 28 29262 2534 4284 22444 5570
TOML 14 414 42 94 278 2
License 6 178 38 0 140 0
C Header 4 3710 240 2934 536 4
Markdown 4 132 36 0 96 0
Plain Text 4 270 20 0 250 0
Makefile 2 238 40 42 156 14
YAML 2 192 14 2 176 0
───────────────────────────────────────────────────────────────────────────────
Total 144 55406 4852 14896 35658 6442
───────────────────────────────────────────────────────────────────────────────
Estimated Cost to Develop (organic) $1 151 755
Estimated Schedule Effort (organic) 14,51 months
Estimated People Required (organic) 7,05
───────────────────────────────────────────────────────────────────────────────
Processed 1852752 bytes, 1.853 megabytes (SI)
───────────────────────────────────────────────────────────────────────────────Expected behavior
Giving multiple directories to scc must compute the sum of all the stats from the different directories.
Desktop (please complete the following information):
- OS: macOS
- Version scc version 3.3.0
Well that's embarrassing.
In short I used a new bit of Go functionality inside the code walker https://github.com/boyter/gocodewalker by using errgroup.Group{} without actually realizing that its eg.Go call is non blocking, so it counts in this case the second directory twice. Simple enough fix https://github.com/boyter/gocodewalker/blob/master/file.go#L160 but annoying.
Anyway pushed out 2 releases, the first which fixes that, and the second which fixes the version (small but I wanted it fixed) https://github.com/boyter/scc/releases/tag/v3.3.2
I had a feeling moving over to the new walker would cause an issue somewhere. Thanks for picking it up so quickly.
Hey @boyter
i think the go team fixed that behavior on go 1.22 https://go.dev/blog/loopvar-preview
maybe worth looking into upgrading go to 1.22, let me know what you think, I might have miss understood the underlying problem
@Tommy-42 oddly I am using Go 1.22 to compile it. I thought it would have resolved it as well, but not in this case. Although it could be falling back to older style of loop as its Go 1.20 in the go.mod
@boyter I did few test and it is kinda weird behavior
I removed your fix ( copy var ) from the gocodewolker :
if len(f.directories) != 0 {
eg := errgroup.Group{}
for _, directory := range f.directories {
eg.Go(func() error {
return f.walkDirectoryRecursive(directory, []gitignore.GitIgnore{}, []gitignore.GitIgnore{})
})
}
err = eg.Wait()
} else {
if f.directory != "" {
err = f.walkDirectoryRecursive(f.directory, []gitignore.GitIgnore{}, []gitignore.GitIgnore{})
}
}✗ go version
go version go1.22.2 darwin/arm64I tried : go run main.go ../meilisearch ../zconfig
it didn't sum both directory, I also tried to update the go.mod to 1.22 it didn't work as well.
then I looked at the link I sent you and they say that you can try it out, with :
GOEXPERIMENT=loopvar go run main.go ../meilisearch ../zconfig
and it worked, the sum of both directory was done
I found the proposal : golang/go#60078 it appears to be done and merged on 1.22 so I am not sure whats the underlying problem there
both following links state that it should work
https://tip.golang.org/ref/spec#For_range , https://pkg.go.dev/golang.org/x/tools/go/analysis/passes/loopclosure
I don't know if it is because of the go.mod of the gocodewalker repository that is not go 1.22 and could block the feature even if it is in vendor folder
Odd.... One I will want to revisit that's for sure because I knew I was using Go 1.22 hence not capturing the variable, but as you say perhaps all the go.mod files need to be on 1.22 in order to ensure it works.
Sounds like a great "gotcha" blog post that would get attention though.
Ok I tried that :
I cloned the gocodewalker repo and updated the go.mod to 1.22 and removed the fix.
I modified the go.mod of scc to use local version of gocodewalker
replace github.com/boyter/gocodewalker => /Users/tommy/Codes/github/gocodewalker
I deleted the vendor folder
run go run main.go ../meilisearch ../zconfig
and it worked.
I changed back the go.mod to 1.20 and it failed as expected.
So I guess we have our answer
Well that answers that. Thanks for the investigation.