Skip to content
This repository was archived by the owner on Mar 25, 2025. It is now read-only.

Improves project file detection #220

Merged
merged 6 commits into from
Feb 20, 2023
Merged
Show file tree
Hide file tree
Changes from 4 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
167 changes: 129 additions & 38 deletions pkg/input/readdir.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,9 +3,11 @@ package input
import (
"encoding/json"
"fmt"
"github.com/klothoplatform/klotho/pkg/filter/predicate"
"io"
"io/fs"
"os"
"path"
"path/filepath"
"strings"

Expand Down Expand Up @@ -33,12 +35,30 @@ func Upcast[F core.File](o fileOpener[F]) fileOpener[core.File] {
type languageFiles struct {
name languageName
foundSources bool
foundPackageFile bool
packageFileName string
// packageFileOpener reads a package file (as opposed to a source file).
foundProjectFile bool
// projectFileOpener reads a package file (as opposed to a source file).
// It is specialized to `core.File` so that the `languageFiles` struct doesn't need to be generic.
// If you have a func that returns a more specfic type, use Upcast to convert it to `fileOpener[core.File]`.
packageFileOpener fileOpener[core.File]
// If you have a func that returns a more specific type, use Upcast to convert it to `fileOpener[core.File]`.
projectFileOpener fileOpener[core.File]
projectFilePredicate predicate.Predicate[string]
projectFileDescription string
}

func (l languageFiles) isProjectFile(filepath string) bool {
return l.projectFilePredicate != nil && l.projectFilePredicate(filepath)
}

func hasName(expected string) predicate.Predicate[string] {
return func(s string) bool {
return expected == s
}
}

func hasExtension(expected string) predicate.Predicate[string] {
extension := "." + expected
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

IMO require the caller to include the . which matches prior art with filepath.Ext

return func(name string) bool {
return extension != name && strings.HasSuffix(name, extension)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
return extension != name && strings.HasSuffix(name, extension)
return filepath.Ext(name) == extension

}
}

type languageName string
Expand Down Expand Up @@ -110,19 +130,33 @@ func ReadDir(fsys fs.FS, cfg config.Application, cfgFilePath string) (*core.Inpu
zap.S().Debugf("Read TS config (%s): %+v", tsConfigPath, tsConfig)
}

jsLang := &languageFiles{name: JavaScript, packageFileName: "package.json", packageFileOpener: Upcast(javascript.NewPackageFile)}
pyLang := &languageFiles{name: Python, packageFileName: "requirements.txt", packageFileOpener: Upcast(python.NewRequirementsTxt)}
goLang := &languageFiles{name: Go, packageFileName: "go.mod", packageFileOpener: Upcast(golang.NewGoMod)}
jsLang := &languageFiles{
name: JavaScript,
projectFilePredicate: hasName("package.json"),
projectFileDescription: "package.json",
projectFileOpener: Upcast(javascript.NewPackageFile)}
pyLang := &languageFiles{
name: Python,
projectFilePredicate: hasName("requirements.txt"),
projectFileDescription: "requirements.txt",
projectFileOpener: Upcast(python.NewRequirementsTxt)}
goLang := &languageFiles{
name: Go,
projectFilePredicate: hasName("go.mod"),
projectFileDescription: "go.mod",
projectFileOpener: Upcast(golang.NewGoMod)}
csLang := &languageFiles{
name: CSharp,
// TODO: Since C# projects don't have a specific named project file, we'll need to update how looking for the project file works.
packageFileName: fmt.Sprintf("%s.csproj", cfg.AppName),
// TODO: package files in C# are currently unused, so no need to open & parse them.
packageFileOpener: func(path string, content io.Reader) (f core.File, err error) { return &core.FileRef{FPath: path}, nil },
name: CSharp,
projectFilePredicate: hasExtension("csproj"),
projectFileDescription: "MSBuild Project File (.csproj)",
// TODO: project files in C# are currently unused, so no need to open & parse them.
projectFileOpener: func(path string, content io.Reader) (f core.File, err error) { return &core.FileRef{FPath: path}, nil },
}
yamlLang := &languageFiles{name: Yaml}
dockerfileLang := &languageFiles{name: DockerFile}
allLangs := []*languageFiles{jsLang, pyLang, goLang, yamlLang}
allLangs := []*languageFiles{jsLang, pyLang, goLang, yamlLang, csLang}
projectDirs := map[languageName][]string{}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What's projectDirs used for? Seems like it's only used to skip folders. Seems like a lot of complication compared to just processing them normally. At least for now seems like premature optimization unless there's something you've already run into (in which case, can you document those cases in the comments?).


err := fs.WalkDir(fsys, cfg.Path, func(path string, info fs.DirEntry, err error) error {
if err != nil {
return err
Expand All @@ -143,13 +177,30 @@ func ReadDir(fsys fs.FS, cfg config.Application, cfgFilePath string) (*core.Inpu
}

if info.IsDir() {
err = detectProjectsInDir(allLangs, fsys, path, projectDirs)
if err != nil {
return err
}

switch info.Name() {
case "node_modules", "vendor":
// Skip modules/vendor folder for performance.
// If we ever support Klotho annotations from dependencies,
// we'll need to remove this skip and check those.
return fs.SkipDir

case "bin", "obj":
dir := filepath.Dir(info.Name())
if dir == "." {
dir = cfg.Path
}
for _, projectDir := range projectDirs[CSharp] {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is susceptible to iteration order problems. What if the csproj was found after the bin/obj directories?

Copy link
Contributor Author

@DavidSeptimus DavidSeptimus Feb 13, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You're right. I got the wrong impression regarding iteration order since these directories are lowercase and the .csproj file I was testing had an uppercase name.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

An alternative might be to instead just look at the peers of bin/obj and see if there's a *.csproj file. So you don't need to read it beforehand.

if dir == projectDir {
zap.L().With(logging.FileField(f)).Debug("detected C# project output, skipping directory")
return fs.SkipDir
}
}
case ".idea", ".vscode":
fallthrough
case ".git", ".svn":
return fs.SkipDir

Expand All @@ -166,16 +217,15 @@ func ReadDir(fsys fs.FS, cfg config.Application, cfgFilePath string) (*core.Inpu
}
return nil
}
isPackageFile := false
isProjectFile := false
for _, lang := range allLangs {
if info.Name() == lang.packageFileName {
f, err = addFile(fsys, path, relPath, lang.packageFileOpener)
lang.foundPackageFile = true
isPackageFile = true
if lang.isProjectFile(info.Name()) {
f, err = addFile(fsys, path, relPath, lang.projectFileOpener)
isProjectFile = true
break
}
}
if !isPackageFile {
if !isProjectFile {
ext := filepath.Ext(info.Name())
switch ext {
case ".js":
Expand Down Expand Up @@ -227,39 +277,80 @@ func ReadDir(fsys fs.FS, cfg config.Application, cfgFilePath string) (*core.Inpu
if err != nil {
return nil, err
}
for _, lang := range allLangs {
if lang.foundSources && !lang.foundPackageFile && lang.packageFileName != "" {

pkg, err := openFindUpward(lang.packageFileName, cfg.Path, fsys, lang.packageFileOpener)
for _, lang := range allLangs {
if lang.foundSources && !lang.foundProjectFile && lang.projectFilePredicate != nil {
projectFile, err := openFindUpward(lang, cfg.Path, fsys)
if err != nil {
return nil, err
}
input.Add(pkg)
zap.L().With(logging.FileField(pkg)).Sugar().Debugf("Read package file for %s", lang.name)
input.Add(projectFile)
zap.L().With(logging.FileField(projectFile)).Sugar().Debugf("Read project file for %s", lang.name)
}
}

return input, nil
}

func detectProjectsInDir(langs []*languageFiles, fsys fs.FS, dir string, projectDirs map[languageName][]string) error {
entries, _ := fs.ReadDir(fsys, dir)
for _, lang := range langs {
for _, e := range entries {
if lang.isProjectFile(e.Name()) {
for _, projectDir := range projectDirs[lang.name] {
if dir == projectDir {
return fmt.Errorf("multiple '%s' files found in directory: %s", lang.projectFileDescription, dir)
}
}
projectDirs[lang.name] = append(projectDirs[lang.name], dir)
lang.foundProjectFile = true
}
}
}
return nil
}

// openFindUpward tries to open the `basename` file in `rootPath`, or any of its parent dirs up to `fsys`'s root.
func openFindUpward[F core.File](basename string, rootPath string, fsys fs.FS, opener fileOpener[F]) (core.File, error) {
for pkgDir := rootPath; ; pkgDir = filepath.Dir(pkgDir) {
pkgPath := filepath.Join(pkgDir, basename)
f, err := fsys.Open(pkgPath)
if errors.Is(err, fs.ErrNotExist) {
if pkgDir == "/" || pkgDir == "." {
break
func openFindUpward(lang *languageFiles, rootPath string, fsys fs.FS) (core.File, error) {
for prjDir := rootPath; ; prjDir = filepath.Dir(prjDir) {
entries, err := fs.ReadDir(fsys, prjDir)
var projectFile core.File
for _, entry := range entries {
if entry.IsDir() {
continue
}

if lang.isProjectFile(entry.Name()) {
if projectFile != nil {
return nil, fmt.Errorf("multiple '%s' files found in directory: %s", lang.projectFileDescription, prjDir)
}

prjFilePath := path.Join(prjDir, entry.Name())
var f fs.File
f, err = fsys.Open(prjFilePath)
if err != nil {
break
}
projectFile, err = func() (core.File, error) {
defer f.Close()
return lang.projectFileOpener(prjFilePath, f)
}()
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This isn't really idiomatic for Go. Something more like

projectFile, err := lang.projectFileOpener(prjFilePath, f)
f.Close()
if err != nil {
// ...
}

if err != nil {
break
}
}
continue
}
if err != nil {
return nil, errors.Wrapf(err, "error looking upward for package file named '%s'", basename)
return nil, errors.Wrapf(err, "error looking upward for project file")
}
if projectFile != nil {
return projectFile, nil
}
if prjDir == "/" || prjDir == "." {
break
}
defer f.Close()
return opener(basename, f)
}
return nil, errors.Errorf("No %s found", basename)
return nil, errors.Errorf("No %s file found", lang.projectFileDescription)
}

func addFile[F core.File](fsys fs.FS, path string, relPath string, opener fileOpener[F]) (core.File, error) {
Expand Down
56 changes: 56 additions & 0 deletions pkg/input/readdir_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -109,6 +109,62 @@ func TestReadDir(t *testing.T) {
"requirements.txt",
},
},
{
name: "csharp: simple",
files: map[string]string{
"fizz/one.cs": "",
"fizz/myproject.csproj": "<Project></Project>",
},
rootPath: "fizz",
want: []string{
"one.cs",
"myproject.csproj",
},
},
{
name: "csharp: obj and bin directories excluded",
files: map[string]string{
"fizz/one.cs": "",
"fizz/aproject.csproj": "<Project></Project>",
"fizz/obj/two.cs": "",
"fizz/bin/aproject.dll": "",
},
rootPath: "fizz",
want: []string{
"one.cs",
"aproject.csproj",
},
},
{
name: "csharp: simple csproj in parent",
files: map[string]string{
"parent/src/one.cs": "", // will be within ./new_cwd/src
"parent/myproject.csproj": "<Project></Project>", // will be wi
},
rootPath: "parent",
want: []string{
"src/one.cs",
"myproject.csproj",
},
},
{
name: "csharp: multiple csproj in parent returns error",
files: map[string]string{
"parent/src/one.cs": "",
"parent/myproject.csproj": "<Project></Project>",
"parent/myproject2.csproj": "<Project></Project>",
},
rootPath: "parent/src",
},
{
name: "csharp: multiple csproj returns error",
files: map[string]string{
"fizz/one.cs": "",
"fizz/myproject.csproj": "<Project></Project>",
"fizz/myproject2.csproj": "<Project></Project>",
},
rootPath: "fizz",
},
{
name: "multi-language",
files: map[string]string{
Expand Down