Skip to content

Commit

Permalink
Support app directories in all commands (#1061)
Browse files Browse the repository at this point in the history
* Support app directories in all commands

Support multi-file app directories in remaining commands like `check`,
`profile`, `lint`, etc.

Note that `pixlet check` no longer supports the `-r` flag for
recursively searching through a directory. This is because a single app
can now contain subfolders with Starlark files, which should be checked
and validated as part of the enclosing app.

This change also removes spell-checking entirely.

* Speed up app loading and checks

* We were loading an applet by walking its entire source FS, but
  skipping any files outside of the root directory. Instead, just list
  and process the files in the root directory explicitly.

* Move buildifier-based checks to the end of `pixlet check`, since they
  walk the entire directory tree. This means they won't run if there are
  any other errors, which are quicker to find.

* Try to fix macOS build

It can't find `webp/demux.h` any more for some reason.
  • Loading branch information
rohansingh authored Apr 26, 2024
1 parent 147f068 commit 29d4064
Show file tree
Hide file tree
Showing 14 changed files with 188 additions and 253 deletions.
22 changes: 19 additions & 3 deletions .github/workflows/main.yml
Original file line number Diff line number Diff line change
Expand Up @@ -80,20 +80,34 @@ jobs:
- name: Build frontend
run: npm run build

- name: Build
- name: Build Linux
run: make build
if: matrix.os == 'ubuntu-22.04' || matrix.os == 'macos-latest'

- name: Build macOS
run: make build
if: matrix.os == 'macos-latest'
env:
LIBRARY_PATH: "/opt/homebrew/lib"
CGO_CPPFLAGS: "-I/opt/homebrew/include"

- name: Build Windows
shell: msys2 {0}
run: |
set MSYSTEM=MINGW64
make build
if: matrix.os == 'windows-latest'

- name: Test
- name: Test Linux
run: make test
if: matrix.os == 'ubuntu-22.04' || matrix.os == 'macos-latest'
if: matrix.os == 'ubuntu-22.04'

- name: Test macOS
run: make test
if: matrix.os == 'macos-latest'
env:
LIBRARY_PATH: "/opt/homebrew/lib"
CGO_CPPFLAGS: "-I/opt/homebrew/include"

- name: Test Windows
shell: msys2 {0}
Expand Down Expand Up @@ -126,6 +140,8 @@ jobs:
run: make release-macos
env:
PIXLET_VERSION: ${{ steps.vars.outputs.tag }}
LIBRARY_PATH: "/opt/homebrew/lib"
CGO_CPPFLAGS: "-I/opt/homebrew/include"

- name: Build Release Windows
if: matrix.os == 'windows-latest'
Expand Down
22 changes: 18 additions & 4 deletions .github/workflows/pull-request.yml
Original file line number Diff line number Diff line change
Expand Up @@ -78,9 +78,16 @@ jobs:
- name: Build frontend
run: npm run build

- name: Build
- name: Build Linux
run: make build
if: matrix.os == 'ubuntu-22.04' || matrix.os == 'macos-latest'
if: matrix.os == 'ubuntu-22.04'

- name: Build macOS
run: make build
if: matrix.os == 'macos-latest'
env:
LIBRARY_PATH: "/opt/homebrew/lib"
CGO_CPPFLAGS: "-I/opt/homebrew/include"

- name: Build Windows
shell: msys2 {0}
Expand All @@ -89,9 +96,16 @@ jobs:
make build
if: matrix.os == 'windows-latest'

- name: Test
- name: Test Linux
run: make test
if: matrix.os == 'ubuntu-22.04'

- name: Test macOS
run: make test
if: matrix.os == 'ubuntu-22.04' || matrix.os == 'macos-latest'
if: matrix.os == 'macos-latest'
env:
LIBRARY_PATH: "/opt/homebrew/lib"
CGO_CPPFLAGS: "-I/opt/homebrew/include"

- name: Test Windows
shell: msys2 {0}
Expand Down
141 changes: 75 additions & 66 deletions cmd/check.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,29 +2,32 @@ package cmd

import (
"fmt"
"io/fs"
"os"
"path/filepath"
"strings"
"time"

"github.com/bazelbuild/buildtools/buildifier/utils"
"github.com/fatih/color"
"github.com/spf13/cobra"
"tidbyt.dev/pixlet/cmd/community"
"tidbyt.dev/pixlet/manifest"
"tidbyt.dev/pixlet/tools"
)

const MaxRenderTime = 1000000000 // 1000ms

func init() {
CheckCmd.Flags().BoolVarP(&rflag, "recursive", "r", false, "find apps recursively")
}

var CheckCmd = &cobra.Command{
Use: "check <pathspec>...",
Example: ` pixlet check app.star`,
Short: "Checks if an app is ready to publish",
Long: `The check command runs a series of checks to ensure your app is ready
Use: "check <path>...",
Example: `pixlet check examples/clock`,
Short: "Check if an app is ready to publish",
Long: `Check if an app is ready to publish.
The path argument should be the path to the Pixlet app to check. The
app can be a single file with the .star extension, or a directory
containing multiple Starlark files and resources.
The check command runs a series of checks to ensure your app is ready
to publish in the community repo. Every failed check will have a solution
provided. If your app fails a check, try the provided solution and reach out on
Discord if you get stuck.`,
Expand All @@ -33,85 +36,59 @@ Discord if you get stuck.`,
}

func checkCmd(cmd *cobra.Command, args []string) error {
// Use the same logic as buildifier to find relevant Tidbyt apps.
apps := args
if rflag {
discovered, err := utils.ExpandDirectories(&args)
// check every path.
foundIssue := false
for _, path := range args {
// check if path exists, and whether it is a directory or a file
info, err := os.Stat(path)
if err != nil {
return fmt.Errorf("could not discover apps using recursive flag: %w", err)
return fmt.Errorf("failed to stat %s: %w", path, err)
}
apps = discovered
} else {
for _, app := range apps {
if filepath.Ext(app) != ".star" {
return fmt.Errorf("only starlark source files or directories with the recursive flag are supported")
}
}
}

// TODO: this needs to be parallelized.
var fsys fs.FS
var baseDir string
if info.IsDir() {
fsys = os.DirFS(path)
baseDir = path
} else {
if !strings.HasSuffix(path, ".star") {
return fmt.Errorf("script file must have suffix .star: %s", path)
}

// Check every app.
foundIssue := false
for _, app := range apps {
// Check app formatting.
dryRunFlag = true
err := formatCmd(cmd, []string{app})
if err != nil {
foundIssue = true
failure(app, fmt.Errorf("app is not formatted correctly: %w", err), fmt.Sprintf("try `pixlet format %s`", app))
continue
fsys = tools.NewSingleFileFS(path)
baseDir = filepath.Dir(path)
}

// Check if an app can load.
err = community.LoadApp(cmd, []string{app})
if err != nil {
foundIssue = true
failure(app, fmt.Errorf("app failed to load: %w", err), "try `pixlet community load-app` and resolve any runtime issues")
continue
}

// Check if app is linted.
outputFormat = "off"
err = lintCmd(cmd, []string{app})
err = community.LoadApp(cmd, []string{path})
if err != nil {
foundIssue = true
failure(app, fmt.Errorf("app has lint warnings: %w", err), fmt.Sprintf("try `pixlet lint --fix %s`", app))
failure(path, fmt.Errorf("app failed to load: %w", err), "try `pixlet community load-app` and resolve any runtime issues")
continue
}

// Ensure icons are valid.
err = community.ValidateIcons(cmd, []string{app})
err = community.ValidateIcons(cmd, []string{path})
if err != nil {
foundIssue = true
failure(app, fmt.Errorf("app has invalid icons: %w", err), "try `pixlet community list-icons` for the full list of valid icons")
failure(path, fmt.Errorf("app has invalid icons: %w", err), "try `pixlet community list-icons` for the full list of valid icons")
continue
}

// Check app manifest exists
dir := filepath.Dir(app)
if !doesManifestExist(dir) {
if !doesManifestExist(baseDir) {
foundIssue = true
failure(app, fmt.Errorf("couldn't find app manifest: %w", err), fmt.Sprintf("try `pixlet community create-manifest %s`", filepath.Join(dir, manifest.ManifestFileName)))
failure(path, fmt.Errorf("couldn't find app manifest"), fmt.Sprintf("try `pixlet community create-manifest %s`", filepath.Join(baseDir, manifest.ManifestFileName)))
continue
}

// Validate manifest.
manifestFile := filepath.Join(dir, manifest.ManifestFileName)
community.ValidateManifestAppFileName = filepath.Base(app)
manifestFile := filepath.Join(baseDir, manifest.ManifestFileName)
community.ValidateManifestAppFileName = filepath.Base(path)
err = community.ValidateManifest(cmd, []string{manifestFile})
if err != nil {
foundIssue = true
failure(app, fmt.Errorf("manifest didn't validate: %w", err), "try correcting the validation issue by updating your manifest")
continue
}

// Check spelling.
community.SilentSpelling = true
err = community.SpellCheck(cmd, []string{manifestFile})
if err != nil {
foundIssue = true
failure(app, fmt.Errorf("manifest contains spelling errors: %w", err), fmt.Sprintf("try `pixlet community spell-check --fix %s`", manifestFile))
failure(path, fmt.Errorf("manifest didn't validate: %w", err), "try correcting the validation issue by updating your manifest")
continue
}

Expand All @@ -125,26 +102,58 @@ func checkCmd(cmd *cobra.Command, args []string) error {
// Check if app renders.
silenceOutput = true
output = f.Name()
err = render(cmd, []string{app})
err = render(cmd, []string{path})
if err != nil {
foundIssue = true
failure(app, fmt.Errorf("app failed to render: %w", err), "try `pixlet render` and resolve any runtime issues")
failure(path, fmt.Errorf("app failed to render: %w", err), "try `pixlet render` and resolve any runtime issues")
continue
}

// Check performance.
p, err := ProfileApp(app, map[string]string{})
p, err := ProfileApp(path, map[string]string{})
if err != nil {
return fmt.Errorf("could not profile app: %w", err)
}
if p.DurationNanos > MaxRenderTime {
foundIssue = true
failure(app, fmt.Errorf("app takes too long to render %s", time.Duration(p.DurationNanos)), fmt.Sprintf("try optimizing your app using `pixlet profile %s` to get it under %s", app, time.Duration(MaxRenderTime)))
failure(
path,
fmt.Errorf("app takes too long to render %s", time.Duration(p.DurationNanos)),
fmt.Sprintf("try optimizing your app using `pixlet profile %s` to get it under %s", path, time.Duration(MaxRenderTime)),
)
continue
}

// run format and lint on *.star files in the fs
fs.WalkDir(fsys, ".", func(p string, d fs.DirEntry, err error) error {
if err != nil {
return err
}

if d.IsDir() || !strings.HasSuffix(p, ".star") {
return nil
}

realPath := filepath.Join(baseDir, p)

dryRunFlag = true
if err := formatCmd(cmd, []string{realPath}); err != nil {
foundIssue = true
failure(p, fmt.Errorf("app is not formatted correctly: %w", err), fmt.Sprintf("try `pixlet format %s`", realPath))
}

outputFormat = "off"
err = lintCmd(cmd, []string{realPath})
if err != nil {
foundIssue = true
failure(p, fmt.Errorf("app has lint warnings: %w", err), fmt.Sprintf("try `pixlet lint --fix %s`", realPath))
}

return nil
})

// If we're here, the app and manifest are good to go!
success(app)
success(path)
}

if foundIssue {
Expand Down
1 change: 0 additions & 1 deletion cmd/community/community.go
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,6 @@ import (
func init() {
CommunityCmd.AddCommand(ListIconsCmd)
CommunityCmd.AddCommand(LoadAppCmd)
CommunityCmd.AddCommand(SpellCheckCmd)
CommunityCmd.AddCommand(TargetDeterminatorCmd)
CommunityCmd.AddCommand(ValidateIconsCmd)
CommunityCmd.AddCommand(ValidateManifestCmd)
Expand Down
29 changes: 20 additions & 9 deletions cmd/community/loadapp.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,39 +2,50 @@ package community

import (
"fmt"
"io/fs"
"os"
"path/filepath"
"strings"

"github.com/spf13/cobra"
"tidbyt.dev/pixlet/runtime"
"tidbyt.dev/pixlet/tools"
)

var LoadAppCmd = &cobra.Command{
Use: "load-app <filespec>",
Use: "load-app <path>",
Short: "Validates an app can be successfully loaded in our runtime.",
Example: ` pixlet community load-app app.star`,
Example: `pixlet community load-app examples/clock`,
Long: `This command ensures an app can be loaded into our runtime successfully.`,
Args: cobra.ExactArgs(1),
RunE: LoadApp,
}

func LoadApp(cmd *cobra.Command, args []string) error {
script := args[0]
path := args[0]

if !strings.HasSuffix(script, ".star") {
return fmt.Errorf("script file must have suffix .star: %s", script)
// check if path exists, and whether it is a directory or a file
info, err := os.Stat(path)
if err != nil {
return fmt.Errorf("failed to stat %s: %w", path, err)
}

src, err := os.ReadFile(script)
if err != nil {
return fmt.Errorf("failed to read file %s: %w", script, err)
var fs fs.FS
if info.IsDir() {
fs = os.DirFS(path)
} else {
if !strings.HasSuffix(path, ".star") {
return fmt.Errorf("script file must have suffix .star: %s", path)
}

fs = tools.NewSingleFileFS(path)
}

cache := runtime.NewInMemoryCache()
runtime.InitHTTP(cache)
runtime.InitCache(cache)

if _, err := runtime.NewApplet(script, src, runtime.WithPrintDisabled()); err != nil {
if _, err := runtime.NewAppletFromFS(filepath.Base(path), fs, runtime.WithPrintDisabled()); err != nil {
return fmt.Errorf("failed to load applet: %w", err)
}

Expand Down
Loading

0 comments on commit 29d4064

Please sign in to comment.