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

Fix: Buildifier cannot find .buildifier-tables.json when run from a subdirectory #1312

Open
wants to merge 7 commits into
base: main
Choose a base branch
from
38 changes: 34 additions & 4 deletions buildifier/config/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -67,6 +67,28 @@ func FindConfigPath(rootDir string) string {
return filepath.Join(dirname, buildifierJSONFilename)
}

// findTablesPath locates the specified table file starting from the process's
// current working directory. It searches upward through the directory tree
// until the file is found or the root of the workspace is reached.
func findTablesPath(file string) (string, error) {
if wspace.IsRegularFile(file) {
return file, nil
}
rootDir, _ := os.Getwd()
dirname, err := wspace.Find(
rootDir,
map[string]func(os.FileInfo) bool{
file: func(fi os.FileInfo) bool {
return fi.Mode()&os.ModeType == 0
},
},
)
if err != nil {
return file, err
}
return filepath.Join(dirname, file), nil
}

// Config is used to configure buildifier
type Config struct {
// InputType determines the input file type: build (for BUILD files), bzl
Expand Down Expand Up @@ -190,14 +212,22 @@ func (c *Config) Validate(args []string) error {
}

if c.TablesPath != "" {
if err := tables.ParseAndUpdateJSONDefinitions(c.TablesPath, false); err != nil {
return fmt.Errorf("failed to parse %s for -tables: %w", c.TablesPath, err)
foundTablesPath, err := findTablesPath(c.TablesPath)
if err != nil {
return fmt.Errorf("failed to find %s for -tables: %w", c.TablesPath, err)
}
if err := tables.ParseAndUpdateJSONDefinitions(foundTablesPath, false); err != nil {
return fmt.Errorf("failed to parse %s for -tables: %w", foundTablesPath, err)
}
}

if c.AddTablesPath != "" {
if err := tables.ParseAndUpdateJSONDefinitions(c.AddTablesPath, true); err != nil {
return fmt.Errorf("failed to parse %s for -add_tables: %w", c.AddTablesPath, err)
foundTablesPath, err := findTablesPath(c.AddTablesPath)
if err != nil {
return fmt.Errorf("failed to find %s for -add_tables: %w", c.AddTablesPath, err)
}
if err := tables.ParseAndUpdateJSONDefinitions(foundTablesPath, true); err != nil {
return fmt.Errorf("failed to parse %s for -add_tables: %w", foundTablesPath, err)
}
}

Expand Down
93 changes: 93 additions & 0 deletions buildifier/config/config_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -581,3 +581,96 @@ func TestFindConfigPath(t *testing.T) {
})
}
}

func TestFindTablePath(t *testing.T) {
tests := []struct {
name string
file string
files []string
wd string
want string
wantErr error
}{
{
name: "default",
file: ".buildifier-tables.json",
files: []string{".buildifier-tables.json"},
wd: "",
want: ".buildifier-tables.json",
wantErr: nil,
},
{
name: "working-dir-is-subdir",
file: ".buildifier-tables.json",
files: []string{".buildifier-tables.json", "foo/BUILD.bazel"},
wd: "foo",
want: ".buildifier-tables.json",
wantErr: nil,
},
{
name: "relative-subdir",
file: "bar/.buildifier-tables.json",
files: []string{"bar/.buildifier-tables.json", "foo/BUILD.bazel"},
wd: "foo",
want: "bar/.buildifier-tables.json",
wantErr: nil,
},
{
name: "file-not-found",
file: "nonexistentFile.json",
files: []string{".buildifier-tables.json"},
wd: "",
want: "nonexistentFile.json",
wantErr: os.ErrNotExist,
},
}

for _, tc := range tests {
t.Run(tc.name, func(t *testing.T) {
tmp := t.TempDir()

// On MacOS "/tmp" is a symlink to "/private/tmp". Resolve it to make the testing easier
tmp, err := filepath.EvalSymlinks(tmp)
if err != nil {
t.Fatalf("failed to resolve symlink for temporary directory: %v", err)
}
t.Log("tmp:", tmp)

if tc.wd != "" {
if err := os.MkdirAll(filepath.Join(tmp, tc.wd), os.ModePerm); err != nil {
t.Fatalf("failed to create working directory: %v", err)
}
if err := os.Chdir(filepath.Join(tmp, tc.wd)); err != nil {
t.Fatalf("failed to change working directory: %v", err)
}
} else {
if err := os.Chdir(tmp); err != nil {
t.Fatalf("failed to change working directory: %v", err)
}
}

for _, file := range tc.files {
filePath := filepath.Join(tmp, file)
dir := filepath.Dir(filePath)
if err := os.MkdirAll(dir, os.ModePerm); err != nil {
t.Fatalf("failed to create directory %v: %v", dir, err)
}
if err := os.WriteFile(filePath, []byte("{}"), 0644); err != nil {
t.Fatalf("failed to create file %v: %v", filePath, err)
}
}

got, err := findTablesPath(tc.file)
got = strings.TrimPrefix(got, tmp)
got = strings.TrimPrefix(got, "/")

if (err != nil) != (tc.wantErr != nil) || (err != nil && tc.wantErr.Error() != err.Error()) {
t.Errorf("FindTablePath wantErr = %q, error = %q", tc.wantErr, err)
}

if tc.want != got {
t.Errorf("FindTablePath want = %q, got = %q", tc.want, got)
}
})
}
}