Skip to content

Commit

Permalink
fix: Community Integration Bugs (#595)
Browse files Browse the repository at this point in the history
* fix: Resolve an issue with manifest file names.

This commit resolves an issue where the manifest file was not being
written to the correct location and was overwriting the starlark source,
but only in the community repo.

* fix: Make the manifest filename a constant.

This commit removes the ability to have a .yml ending switches all uses
to be a constant. This will significantly reduce headaches down the
line given there is only one file name.

* community: Add load app command.

This commit adds a load app command so that we can ensure that an app
can load, even if it cannot render.

* fix: Update checks to match community.

This commit makes some of the checks a bit less aggressive with a TODO
on how to make it more agressive in the future. The reason being, I'd
like to get this enabled today as a drop in replacement rather then be
more strict.
  • Loading branch information
betterengineering authored Jan 24, 2023
1 parent 40c7664 commit dac3229
Show file tree
Hide file tree
Showing 7 changed files with 74 additions and 36 deletions.
55 changes: 24 additions & 31 deletions cmd/check.go
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ import (
"github.com/fatih/color"
"github.com/spf13/cobra"
"tidbyt.dev/pixlet/cmd/community"
"tidbyt.dev/pixlet/manifest"
)

func init() {
Expand Down Expand Up @@ -45,6 +46,8 @@ func checkCmd(cmd *cobra.Command, args []string) error {
}
}

// TODO: this needs to be parallelized.

// Check every app.
foundIssue := false
for _, app := range apps {
Expand All @@ -64,13 +67,14 @@ func checkCmd(cmd *cobra.Command, args []string) error {
}
defer os.Remove(f.Name())

// Check if app will render.
silenceOutput = true
output = f.Name()
err = render(cmd, []string{app})
// TODO: Check if app will render once we are able to enable target
// determination.

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

Expand All @@ -93,39 +97,31 @@ func checkCmd(cmd *cobra.Command, args []string) error {

// Check app manifest exists
dir := filepath.Dir(app)
options := []string{
filepath.Join(dir, "manifest.yaml"),
filepath.Join(dir, "manifest.yml"),
}
manifestFile, err := findManifestFile(options)
if err != nil {
if !doesManifestExist(dir) {
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.yaml")))
failure(app, fmt.Errorf("couldn't find app manifest: %w", err), fmt.Sprintf("try `pixlet community create-manifest %s`", filepath.Join(dir, manifest.ManifestFileName)))
continue
}

// Validate manifest.
manifestFile := filepath.Join(dir, manifest.ManifestFileName)
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 in both the app and the manifest.
// Check spelling.
community.SilentSpelling = true
err = community.SpellCheck(cmd, []string{app})
if err != nil {
foundIssue = true
failure(app, fmt.Errorf("app contains spelling errors: %w", err), fmt.Sprintf("try `pixlet community spell-check --fix %s`", app))
continue
}
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))
continue
}
// TODO: enable spell check for apps once we can run it successfully
// against the community repo.

// If we're here, the app and manifest are good to go!
success(app)
Expand All @@ -138,21 +134,18 @@ func checkCmd(cmd *cobra.Command, args []string) error {
return nil
}

func findManifestFile(options []string) (string, error) {
for _, file := range options {
_, err := os.Stat(file)
if os.IsNotExist(err) {
continue
}

if err != nil {
return "", fmt.Errorf("couldn't check manifest existence: %w", err)
}
func doesManifestExist(dir string) bool {
file := filepath.Join(dir, manifest.ManifestFileName)
_, err := os.Stat(file)
if os.IsNotExist(err) {
return false
}

return file, nil
if err != nil {
return false
}

return "", fmt.Errorf("manifest doesn't exist")
return true
}

func success(app string) {
Expand Down
1 change: 1 addition & 0 deletions cmd/community/community.go
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ import (
func init() {
CommunityCmd.AddCommand(CreateManifestCmd)
CommunityCmd.AddCommand(ListIconsCmd)
CommunityCmd.AddCommand(LoadAppCmd)
CommunityCmd.AddCommand(SpellCheckCmd)
CommunityCmd.AddCommand(TargetDeterminatorCmd)
CommunityCmd.AddCommand(ValidateIconsCmd)
Expand Down
5 changes: 3 additions & 2 deletions cmd/community/createmanifest.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ import (
"path/filepath"

"github.com/spf13/cobra"
"tidbyt.dev/pixlet/manifest"
)

var CreateManifestCmd = &cobra.Command{
Expand All @@ -19,8 +20,8 @@ var CreateManifestCmd = &cobra.Command{

func CreateManifest(cmd *cobra.Command, args []string) error {
fileName := filepath.Base(args[0])
if fileName != "manifest.yaml" && fileName != "manifest.yml" {
return fmt.Errorf("supplied manifest must be named manifest.yaml or manifest.yml")
if fileName != manifest.ManifestFileName {
return fmt.Errorf("supplied manifest must be named %s", manifest.ManifestFileName)
}

f, err := os.Create(args[0])
Expand Down
41 changes: 41 additions & 0 deletions cmd/community/loadapp.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,41 @@
package community

import (
"fmt"
"io/ioutil"
"strings"

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

var LoadAppCmd = &cobra.Command{
Use: "load-app <filespec>",
Short: "Validates an app can be successfully loaded in our runtime.",
Example: ` pixlet community load-app app.star`,
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]

if !strings.HasSuffix(script, ".star") {
return fmt.Errorf("script file must have suffix .star: %s", script)
}

src, err := ioutil.ReadFile(script)
if err != nil {
return fmt.Errorf("failed to read file %s: %w", script, err)
}
runtime.InitCache(runtime.NewInMemoryCache())

applet := runtime.Applet{}
err = applet.Load(script, src, nil)
if err != nil {
return fmt.Errorf("failed to load applet: %w", err)
}

return nil
}
4 changes: 2 additions & 2 deletions cmd/community/validatemanifest.go
Original file line number Diff line number Diff line change
Expand Up @@ -21,8 +21,8 @@ validating the contents of each field.`,

func ValidateManifest(cmd *cobra.Command, args []string) error {
fileName := filepath.Base(args[0])
if fileName != "manifest.yaml" && fileName != "manifest.yml" {
return fmt.Errorf("supplied manifest must be named manifest.yaml or manifest.yml")
if fileName != manifest.ManifestFileName {
return fmt.Errorf("supplied manifest must be named %s", manifest.ManifestFileName)
}

f, err := os.Open(args[0])
Expand Down
2 changes: 2 additions & 0 deletions manifest/manifest.go
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,8 @@ import (
"gopkg.in/yaml.v3"
)

const ManifestFileName = "manifest.yaml"

// Manifest is a structure to define a starlark applet for Tidbyt in Go.
type Manifest struct {
// ID is the unique identifier of this app. It has to be globally unique,
Expand Down
2 changes: 1 addition & 1 deletion tools/generator/generator.go
Original file line number Diff line number Diff line change
Expand Up @@ -98,7 +98,7 @@ func (g *Generator) writeManifest(app *manifest.Manifest) error {
var p string
switch g.appType {
case Community:
p = path.Join(g.root, appsDir, app.PackageName, app.FileName)
p = path.Join(g.root, appsDir, app.PackageName, manifestName)
default:
p = path.Join(g.root, manifestName)
}
Expand Down

0 comments on commit dac3229

Please sign in to comment.