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

overhaul how we load module info to format Go files #211

Merged
merged 1 commit into from Mar 18, 2022
Merged

Commits on Mar 18, 2022

  1. overhaul how we load module info to format Go files

    We would call "go mod edit -json" for each Go file we formatted,
    as each file may be in a different directory,
    and thus inside a different module.
    
    However, the first mistake is that we always ran the command in the
    directory where gofumpt is run, not the directory containing each Go
    file. Our tests weren't strict enough to catch this; now
    octal-literal.txt is, via its run of gofmt before it calls cd.
    
    The second mistake, and a pretty embarrassing one, is that since v0.3.0
    made gofumpt concurrent, it has been racy in how it writes to globals
    like langVersion from multiple goroutines. octal-literals.txt now tests
    for this by adding a nested Go module.
    
    Finally, we could also run into open file limits,
    because spawning a child process and grabbing its output opens files of
    its own such as named pipes.
    The added test shows this with a limit of 256 and 10k tiny Go files:
    
    	--- FAIL: TestWithLowOpenFileLimit (0.30s)
    		ulimit_unix_test.go:82: writing 10000 tiny Go files
    		ulimit_unix_test.go:104: running with 1 paths
    		ulimit_unix_test.go:104: running with 10000 paths
    		ulimit_unix_test.go:112:
    			error:
    			  got non-nil error
    			comment:
    			  stderr:
    			  open /tmp/TestWithLowOpenFileLimit2753748366/001/p003/014.go: too many open files
    			  open /tmp/TestWithLowOpenFileLimit2753748366/001/p003/017.go: too many open files
    			  open /tmp/TestWithLowOpenFileLimit2753748366/001/p004/000.go: too many open files
    			  open /tmp/TestWithLowOpenFileLimit2753748366/001/p004/019.go: too many open files
    
    Instead, only call "go mod edit -json" once per directory,
    and do it in the main thread to reduce its parallelism.
    Also make it grab fdSem as well, for good measure.
    
    This may not be a complete fix, as we're not sure how many files are
    open by an exec.Command.Output call. However, we are no longer able to
    reproduce a failure, so leave that as a TODO.
    
    Fixes #208.
    mvdan committed Mar 18, 2022
    Copy the full SHA
    62e7ea0 View commit details
    Browse the repository at this point in the history