Skip to content

Commit

Permalink
fix(stdlib): generate placeholder.go file when needed (#4954)
Browse files Browse the repository at this point in the history
Go's vendoring semantics mean we need all Flux packages to also be Go
packages. This change handles that logic. This is similar to the
flux_gen.go solution we had previously, but different in one key way.
The contents of placeholder.go are static. This means we do not need to
treat them as special binary files or otherwise. Additionally the new
name of placeholder.go is clear what its purpose is.
  • Loading branch information
nathanielc authored Jul 1, 2022
1 parent 97adb3e commit 64ae871
Show file tree
Hide file tree
Showing 44 changed files with 305 additions and 36 deletions.
6 changes: 6 additions & 0 deletions cmd/flux/cmd/test.go
Original file line number Diff line number Diff line change
Expand Up @@ -971,6 +971,12 @@ func (t *TestReporter) ReportTestRun(test *Test) {
fmt.Fprintf(t.out, "%s...success\n", test.FullName())
}
} else {
if test.skip {
// Do not print full output of skipped tests
// Using verbosity >=3 is about debugging a running test,
// we do not need information about skipped tests
return
}
fmt.Fprintf(t.out, "Testcase: %s\n", test.FullName())
fmt.Fprintf(t.out, "Tags: %v\n", test.tags)
source, err := test.SourceCode()
Expand Down
48 changes: 24 additions & 24 deletions cmd/flux/test_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -214,10 +214,10 @@ func runForPath(t *testing.T, path string, wantErr error, args ...string) Summar

func Test_TestCmd(t *testing.T) {
want := Summary{
Found: 6,
Found: 7,
Passed: 1,
Failed: 0,
Skipped: 5,
Skipped: 6,
}
got := runAll(t, nil)
for name, got := range got {
Expand All @@ -229,10 +229,10 @@ func Test_TestCmd(t *testing.T) {

func Test_TestCmd_TestName(t *testing.T) {
want := Summary{
Found: 6,
Found: 7,
Passed: 1,
Failed: 0,
Skipped: 5,
Skipped: 6,
}
got := runAll(t, nil, "--test", "a")
for name, got := range got {
Expand All @@ -243,10 +243,10 @@ func Test_TestCmd_TestName(t *testing.T) {
}
func Test_TestCmd_Fails(t *testing.T) {
want := Summary{
Found: 6,
Found: 7,
Passed: 1,
Failed: 1,
Skipped: 4,
Skipped: 5,
}
got := runAll(t, errors.New("tests failed"), "--tags", "fail")
for name, got := range got {
Expand All @@ -268,56 +268,56 @@ func Test_TestCmd_Tags(t *testing.T) {
{
tags: []string{"a"},
want: Summary{
Found: 6,
Found: 7,
Passed: 2,
Skipped: 4,
Skipped: 5,
},
},
{
tags: []string{"a", "b"},
want: Summary{
Found: 6,
Found: 7,
Passed: 3,
Skipped: 3,
Skipped: 4,
},
},
{
tags: []string{"a", "b", "c"},
want: Summary{
Found: 6,
Found: 7,
Passed: 4,
Skipped: 2,
Skipped: 3,
},
},
{
tags: []string{"b", "c"},
want: Summary{
Found: 6,
Found: 7,
Passed: 1,
Skipped: 5,
Skipped: 6,
},
},
{
tags: []string{"c"},
want: Summary{
Found: 6,
Found: 7,
Passed: 1,
Skipped: 5,
Skipped: 6,
},
},
{
tags: []string{"b"},
want: Summary{
Found: 6,
Found: 7,
Passed: 1,
Skipped: 5,
Skipped: 6,
},
},
{
tags: []string{"foo"},
want: Summary{
Found: 6,
Passed: 2,
Found: 7,
Passed: 3,
Skipped: 4,
},
},
Expand All @@ -337,9 +337,9 @@ func Test_TestCmd_Tags(t *testing.T) {
}
func Test_TestCmd_Skip(t *testing.T) {
want := Summary{
Found: 6,
Found: 7,
Passed: 0,
Skipped: 6,
Skipped: 7,
}
got := runAll(t, nil, "--skip", "untagged")
for name, got := range got {
Expand All @@ -351,9 +351,9 @@ func Test_TestCmd_Skip(t *testing.T) {

func Test_TestCmd_SkipUntagged(t *testing.T) {
want := Summary{
Found: 6,
Found: 7,
Passed: 0,
Skipped: 6,
Skipped: 7,
}
got := runAll(t, nil, "--skip-untagged")
for name, got := range got {
Expand Down
6 changes: 6 additions & 0 deletions cmd/flux/testdata/pkga/pkga_test.flux
Original file line number Diff line number Diff line change
Expand Up @@ -9,3 +9,9 @@ option testing.tags = ["foo"]
testcase bar {
array.from(rows: [{}])
}

testcase untagged_extends extends "testdata/test_test.untagged" {
// Note this test is tagged with foo because of the package level
// option statement
super()
}
51 changes: 40 additions & 11 deletions internal/cmd/builtin/cmd/generate.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ package cmd

import (
"io/ioutil"
"os"
"path"
"path/filepath"
"strings"
Expand All @@ -15,7 +16,8 @@ var generateCmd = &cobra.Command{
Use: "generate",
Short: "Generate Go source from Flux source",
Long: `This utility generates a Go source file that imports the Flux packages.
Only Flux packages that contain Go code will be imported.`,
All Flux packages need to also be a Go package.
A placeholder file will be added when needed.`,
RunE: generate,
}

Expand All @@ -29,17 +31,31 @@ func init() {
rootCmd.AddCommand(generateCmd)
generateCmd.Flags().StringVar(&pkgName, "go-pkg", "", "The fully qualified Go package name of the root package.")
generateCmd.Flags().StringVar(&rootDir, "root-dir", ".", "The root level directory for all packages.")
generateCmd.Flags().StringVar(&importFile, "import-file", "builtin_gen.go", "Location relative to root-dir to place a file to import all generated packages.")
generateCmd.Flags().StringVar(&importFile, "import-file", "packages.go", "Location relative to root-dir to place a file to import all generated packages.")
}

const placeholder = "placeholder.go"

func generate(cmd *cobra.Command, args []string) error {
var goPackages []string
err := walkDirs(rootDir, func(dir string) error {
if ok, err := needsImport(dir); err != nil {
return err
} else if ok {
goPath := path.Join(pkgName, dir)
hasGo, hasFlux, err := dirProps(dir)
if err != nil {
return nil
}
goPath := path.Join(pkgName, dir)
if hasFlux && !isInternal(dir) {
// We need a package import
goPackages = append(goPackages, goPath)
if !hasGo {
// We need a placeholder file to ensure this is a Go package.
if err := savePlaceholder(filepath.Join(dir, placeholder), goPath); err != nil {
return err
}
} else {
// We no longer need the placeholder file
os.Remove(filepath.Join(dir, placeholder))
}
}
return nil
})
Expand Down Expand Up @@ -77,23 +93,26 @@ func walkDirs(path string, f func(dir string) error) error {
return nil
}

func needsImport(dir string) (bool, error) {
var hasGo, hasFlux bool
func dirProps(dir string) (hasGo, hasFlux bool, err error) {
files, err := ioutil.ReadDir(dir)
if err != nil {
return false, err
return
}
for _, f := range files {
if filepath.Ext(f.Name()) == ".go" &&
!strings.HasSuffix(path.Base(f.Name()), "_test.go") {
!strings.HasSuffix(path.Base(f.Name()), "_test.go") &&
// Do not count the placeholder file as Go because
// otherwise we will delete it.
f.Name() != placeholder {
hasGo = true
}
if filepath.Ext(f.Name()) == ".flux" {
hasFlux = true
}
}
return hasGo && hasFlux && !isInternal(dir), nil
return
}

func isInternal(p string) bool {
parts := strings.Split(p, "/")
// The toplevel `internal` package is allowed
Expand All @@ -105,3 +124,13 @@ func isInternal(p string) bool {
}
return false
}

func savePlaceholder(fpath, goPkgPath string) error {
// Write the import file
f := jen.NewFile(path.Base(goPkgPath))
f.HeaderComment(`// DO NOT EDIT: This file is autogenerated via the builtin command.
//
// This file ensures that this directory is a Go package
`)
return f.Save(fpath)
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
// DO NOT EDIT: This file is autogenerated via the builtin command.
//
// This file ensures that this directory is a Go package

package naiveBayesClassifier
5 changes: 5 additions & 0 deletions stdlib/contrib/anaisdg/anomalydetection/placeholder.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
// DO NOT EDIT: This file is autogenerated via the builtin command.
//
// This file ensures that this directory is a Go package

package anomalydetection
5 changes: 5 additions & 0 deletions stdlib/contrib/anaisdg/statsmodels/placeholder.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
// DO NOT EDIT: This file is autogenerated via the builtin command.
//
// This file ensures that this directory is a Go package

package statsmodels
5 changes: 5 additions & 0 deletions stdlib/contrib/bonitoo-io/alerta/placeholder.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
// DO NOT EDIT: This file is autogenerated via the builtin command.
//
// This file ensures that this directory is a Go package

package alerta
5 changes: 5 additions & 0 deletions stdlib/contrib/bonitoo-io/servicenow/placeholder.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
// DO NOT EDIT: This file is autogenerated via the builtin command.
//
// This file ensures that this directory is a Go package

package servicenow
5 changes: 5 additions & 0 deletions stdlib/contrib/bonitoo-io/tickscript/placeholder.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
// DO NOT EDIT: This file is autogenerated via the builtin command.
//
// This file ensures that this directory is a Go package

package tickscript
5 changes: 5 additions & 0 deletions stdlib/contrib/bonitoo-io/victorops/placeholder.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
// DO NOT EDIT: This file is autogenerated via the builtin command.
//
// This file ensures that this directory is a Go package

package victorops
5 changes: 5 additions & 0 deletions stdlib/contrib/bonitoo-io/zenoss/placeholder.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
// DO NOT EDIT: This file is autogenerated via the builtin command.
//
// This file ensures that this directory is a Go package

package zenoss
5 changes: 5 additions & 0 deletions stdlib/contrib/chobbs/discord/placeholder.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
// DO NOT EDIT: This file is autogenerated via the builtin command.
//
// This file ensures that this directory is a Go package

package discord
5 changes: 5 additions & 0 deletions stdlib/contrib/rhajek/bigpanda/placeholder.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
// DO NOT EDIT: This file is autogenerated via the builtin command.
//
// This file ensures that this directory is a Go package

package bigpanda
5 changes: 5 additions & 0 deletions stdlib/contrib/sranka/teams/placeholder.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
// DO NOT EDIT: This file is autogenerated via the builtin command.
//
// This file ensures that this directory is a Go package

package teams
5 changes: 5 additions & 0 deletions stdlib/contrib/sranka/telegram/placeholder.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
// DO NOT EDIT: This file is autogenerated via the builtin command.
//
// This file ensures that this directory is a Go package

package telegram
5 changes: 5 additions & 0 deletions stdlib/contrib/sranka/webexteams/placeholder.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
// DO NOT EDIT: This file is autogenerated via the builtin command.
//
// This file ensures that this directory is a Go package

package webexteams
5 changes: 5 additions & 0 deletions stdlib/date/boundaries/placeholder.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
// DO NOT EDIT: This file is autogenerated via the builtin command.
//
// This file ensures that this directory is a Go package

package boundaries
5 changes: 5 additions & 0 deletions stdlib/experimental/aggregate/placeholder.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
// DO NOT EDIT: This file is autogenerated via the builtin command.
//
// This file ensures that this directory is a Go package

package aggregate
5 changes: 5 additions & 0 deletions stdlib/experimental/array/placeholder.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
// DO NOT EDIT: This file is autogenerated via the builtin command.
//
// This file ensures that this directory is a Go package

package array
5 changes: 5 additions & 0 deletions stdlib/experimental/bitwise/placeholder.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
// DO NOT EDIT: This file is autogenerated via the builtin command.
//
// This file ensures that this directory is a Go package

package bitwise
5 changes: 5 additions & 0 deletions stdlib/experimental/csv/placeholder.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
// DO NOT EDIT: This file is autogenerated via the builtin command.
//
// This file ensures that this directory is a Go package

package csv
5 changes: 5 additions & 0 deletions stdlib/experimental/http/requests/placeholder.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
// DO NOT EDIT: This file is autogenerated via the builtin command.
//
// This file ensures that this directory is a Go package

package requests
5 changes: 5 additions & 0 deletions stdlib/experimental/oee/placeholder.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
// DO NOT EDIT: This file is autogenerated via the builtin command.
//
// This file ensures that this directory is a Go package

package oee
5 changes: 5 additions & 0 deletions stdlib/experimental/query/placeholder.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
// DO NOT EDIT: This file is autogenerated via the builtin command.
//
// This file ensures that this directory is a Go package

package query
5 changes: 5 additions & 0 deletions stdlib/experimental/usage/placeholder.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
// DO NOT EDIT: This file is autogenerated via the builtin command.
//
// This file ensures that this directory is a Go package

package usage
2 changes: 1 addition & 1 deletion stdlib/gen.go
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
package stdlib

//go:generate go generate ../libflux/go/libflux
//go:generate go run github.com/influxdata/flux/internal/cmd/builtin generate --go-pkg github.com/influxdata/flux/stdlib --import-file packages.go
//go:generate go run github.com/influxdata/flux/internal/cmd/builtin generate --go-pkg github.com/influxdata/flux/stdlib
Loading

0 comments on commit 64ae871

Please sign in to comment.