proposal: format all shell scripts in Go repo with shfmt

[ghost]

What version of Go are you using (go version)?

$ go version
go version go1.15.9 linux/amd64

Does this issue reproduce with the latest release?

Yes.

What operating system and processor architecture are you using (go env)?

go env Output

$ go env
GO111MODULE=""
GOARCH="amd64"
GOBIN=""
GOCACHE="/home/prajwal/.cache/go-build"
GOENV="/home/prajwal/.config/go/env"
GOEXE=""
GOFLAGS=""
GOHOSTARCH="amd64"
GOHOSTOS="linux"
GOINSECURE=""
GOMODCACHE="/home/prajwal/go/pkg/mod"
GONOPROXY=""
GONOSUMDB=""
GOOS="linux"
GOPATH="/home/prajwal/go"
GOPRIVATE=""
GOPROXY="https://proxy.golang.org,direct"
GOROOT="/usr/lib/go-1.15"
GOSUMDB="sum.golang.org"
GOTMPDIR=""
GOTOOLDIR="/usr/lib/go-1.15/pkg/tool/linux_amd64"
GCCGO="gccgo"
AR="ar"
CC="gcc"
CXX="g++"
CGO_ENABLED="1"
GOMOD=""
CGO_CFLAGS="-g -O2"
CGO_CPPFLAGS=""
CGO_CXXFLAGS="-g -O2"
CGO_FFLAGS="-g -O2"
CGO_LDFLAGS="-g -O2"
PKG_CONFIG="pkg-config"
GOGCCFLAGS="-fPIC -m64 -pthread -fmessage-length=0 -fdebug-prefix-map=/tmp/go-build076073519=/tmp/go-build -gno-record-gcc-switches"

What did you do?

Update the bash script, format it all.

[gopherbot]

Change https://golang.org/cl/303272 mentions this issue: all: Fixed bash

[ianlancetaylor]

Is there some well known formatter for bash scripts? If this just relies on manual formatting, I don't think it's worth doing. It will just get out of date as people edit the scripts.

[zikaeroh]

@mvdan's shfmt is one. https://github.com/mvdan/sh

[ghost]

@ianlancetaylor
https://marketplace.visualstudio.com/items?itemName=foxundermoon.shell-format
Completely automated.

[zikaeroh]

marketplace.visualstudio.com/items?itemName=foxundermoon.shell-format
Completely automated.

FWIW, this is a shfmt wrapper. Even better. 🙂

[ianlancetaylor]

Thanks, turning this into a proposal.

[ghost]

package main

import (
	"fmt"
	"log"
	"os"
	"os/exec"
	"path/filepath"
)

func main() {
	if len(os.Args) > 1 {
		systemRequrmentsCheck()
		formatShellFiles()
	} else {
		os.Exit(0)
	}
}

func systemRequrmentsCheck() {
	if !commandExists("shfmt") {
		log.Fatal("Error: shfmt not found on the system.")
	}
}

func formatShellFiles() {
	if fileExists(os.Args[1]) {
		if filepath.Ext(os.Args[1]) == ".sh" {
			fmt.Println(os.Args[1])
			cmd := exec.Command("shfmt", "-l", "-w", os.Args[1])
			cmd.Run()
		} else if filepath.Ext(os.Args[1]) == ".bash" {
			fmt.Println(os.Args[1])
			cmd := exec.Command("shfmt", "-l", "-w", os.Args[1])
			cmd.Run()
		}
	}

	if folderExists(os.Args[1]) {
		filepath.Walk(os.Args[1], func(path string, info os.FileInfo, err error) error {
			if filepath.Ext(path) == ".sh" {
				fmt.Println(path)
				cmd := exec.Command("shfmt", "-l", "-w", path)
				cmd.Run()
			} else if filepath.Ext(path) == ".bash" {
				fmt.Println(path)
				cmd := exec.Command("shfmt", "-l", "-w", path)
				cmd.Run()
			}
			return nil
		})
	}
}

func folderExists(foldername string) bool {
	info, err := os.Stat(foldername)
	if os.IsNotExist(err) {
		return false
	}
	return info.IsDir()
}

func fileExists(filename string) bool {
	info, err := os.Stat(filename)
	if os.IsNotExist(err) {
		return false
	}
	return !info.IsDir()
}

func commandExists(cmd string) bool {
	cmd, err := exec.LookPath(cmd)
	if err != nil {
		return false
	}
	return true
}

[tklauser]

If we decide to use a common format for shell code, how do we ensure future code changes and additions are properly formatted?

For Go code we can usually assume contributors to run gofmt (or have their editor run it automatically on save). However, there are still cases where non-gofmted code enters the repo. For shell code I'd assume way fewer contributors to know about and run shfmt before submitting code. If this proposal is accepted, we should clearly document the need to shell code to be formatted and ideally enforce it at TryBot level, e.g. as part of #18548 (if it is ever implemented).

EDIT: Or instead of/in addition to a check at TryBot level, provide a pre-commit hook for git-codereview checking for shfmted shell code the same way it already does for gofmted Go code.

[rsc]

There is a significant cost here: all developers working in the repo must install shfmt and keep it updated, we have to expand git-codereview's formatting purview beyond Go files in some configurable way (we can't impose shfmt on all git-codereview users), and so on.

To what benefit? I don't see any. There are basically no arguments over shell syntax today. And there are almost no shell scripts. And perhaps most importantly we don't have tools like gofix or gofmt -r that need to edit shell scripts without making spurious formatting changes. So the three primary benefits of gofmt enforcement do not apply, yet the costs are higher (everyone has gofmt automatically).

It looks to me like we should not do this.

[ghost]

@rsc
Hello,
I agree with you and I don't think we should require users to install shfmt but there are a bunch of benefits to using shfmt.
How about once in a while a maintainer just formats it using shfmt manually, if you go through the changes there are some changes that the shfmt made, that people forgot to make.

https://github.com/golang/go/pull/45125/files#diff-5b8253f08bb9def8ccedef35c577b0af4948354c9f31486cadcf58fa978ccac1R111
https://github.com/golang/go/pull/45125/files#diff-4da5e818e617143528e287539d42b234591b14deab38f58d75120b303ae3fadeR11
https://github.com/golang/go/pull/45125/files#diff-62fb832ef70212fc784967c6ac2c161968a6dfd41f6cb52e2d2a3fb94b78e3eeR34

[rsc]

This proposal has been added to the active column of the proposals project
and will now be reviewed at the weekly proposal review meetings.
— rsc for the proposal review group

[rsc]

Based on the discussion above, this proposal seems like a likely decline.
— rsc for the proposal review group

[rsc]

No change in consensus, so declined.
— rsc for the proposal review group