Skip to content

Commit

Permalink
Addressed review comments and Revised error messages
Browse files Browse the repository at this point in the history
  • Loading branch information
soumeh01 committed Jun 19, 2024
1 parent c25dcfd commit 01324ea
Show file tree
Hide file tree
Showing 14 changed files with 76 additions and 125 deletions.
19 changes: 8 additions & 11 deletions cmd/cbuild/commands/root.go
Original file line number Diff line number Diff line change
Expand Up @@ -239,17 +239,14 @@ func FlagErrorFunc(cmd *cobra.Command, err error) error {
}

func getBuilder(inputFile string, params builder.BuilderParams) (builder.IBuilderInterface, error) {
fileExtension := filepath.Ext(inputFile)
var b builder.IBuilderInterface

switch fileExtension {
case ".cprj":
b = cproject.CprjBuilder{BuilderParams: params}
case ".yml":
b = csolution.CSolutionBuilder{BuilderParams: params}
fileName := filepath.Base(inputFile)

switch {
case strings.HasSuffix(fileName, ".csolution.yml"):
return csolution.CSolutionBuilder{BuilderParams: params}, nil
case strings.HasSuffix(fileName, ".cprj"):
return cproject.CprjBuilder{BuilderParams: params}, nil
default:
return nil, errutils.New(errutils.ErrInvalidFileExtension, fileExtension, ".cprj & .yml")
return nil, errutils.New(errutils.ErrInvalidFileExtension, fileName, ".csolution.yml & .cprj")
}

return b, nil
}
2 changes: 1 addition & 1 deletion cmd/cbuild/commands/setup/setup.go
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ func SetUpProject(cmd *cobra.Command, args []string) error {
expectedExtension := ".csolution.yml"

if !strings.HasSuffix(fileName, expectedExtension) {
return errutils.New(errutils.ErrInvalidFile, fileName, "<project.>"+expectedExtension)
return errutils.New(errutils.ErrInvalidFileExtension, fileName, ".csolution.yml")
}

logFile, _ := cmd.Flags().GetString("log")
Expand Down
26 changes: 7 additions & 19 deletions pkg/builder/cbuildidx/builder.go
Original file line number Diff line number Diff line change
Expand Up @@ -23,20 +23,6 @@ type CbuildIdxBuilder struct {
builder.BuilderParams
}

func (b CbuildIdxBuilder) checkCbuildIdx() error {
fileName := filepath.Base(b.InputFile)
expectedExtension := ".cbuild-idx.yml"
if !strings.HasSuffix(fileName, expectedExtension) {
err := errutils.New(errutils.ErrInvalidFile, fileName, "<project>."+expectedExtension)
return err
} else {
if _, err := os.Stat(b.InputFile); os.IsNotExist(err) {
return err
}
}
return nil
}

func (b CbuildIdxBuilder) clean(dirs builder.BuildDirs, vars builder.InternalVars) (err error) {
removeDirectory := func(dir string) error {
if _, err := os.Stat(dir); os.IsNotExist(err) {
Expand Down Expand Up @@ -118,7 +104,8 @@ func (b CbuildIdxBuilder) getDirs(context string) (dirs builder.BuildDirs, err e
func (b CbuildIdxBuilder) build() error {
b.InputFile, _ = filepath.Abs(b.InputFile)
b.InputFile = utils.NormalizePath(b.InputFile)
err := b.checkCbuildIdx()

_, err := utils.FileExists(b.InputFile)
if err != nil {
return err
}
Expand Down Expand Up @@ -152,6 +139,11 @@ func (b CbuildIdxBuilder) build() error {
return nil
}

if vars.CmakeBin == "" {
err = errutils.New(errutils.ErrBinaryNotFound, "cmake", "")
return err
}

args := []string{b.InputFile}
if b.Options.UseContextSet {
args = append(args, "--context-set")
Expand All @@ -169,10 +161,6 @@ func (b CbuildIdxBuilder) build() error {
return err
}

if vars.CmakeBin == "" {
err = errutils.New(errutils.ErrBinaryNotFound, "cmake", "")
return err
}
if b.Options.Generator == "" {
b.Options.Generator = "Ninja"
if vars.NinjaBin == "" {
Expand Down
30 changes: 1 addition & 29 deletions pkg/builder/cbuildidx/builder_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -39,39 +39,11 @@ func (r RunnerMock) ExecuteCommand(program string, quiet bool, args ...string) (
} else if strings.Contains(program, "ninja") {
} else if strings.Contains(program, "xmllint") {
} else {
return "", errutils.New(errutils.ErrInvalidCommand)
return "", errutils.New(errutils.ErrInvalidCommand, program)
}
return "", nil
}

func TestCheckCbuildIdx(t *testing.T) {
assert := assert.New(t)

b := CbuildIdxBuilder{
builder.BuilderParams{
Runner: RunnerMock{},
},
}

t.Run("test valid cbuild-idx", func(t *testing.T) {
b.InputFile = filepath.Join(testRoot, testDir, "Hello.cbuild-idx.yml")
err := b.checkCbuildIdx()
assert.Nil(err)
})

t.Run("test existent file, invalid extension", func(t *testing.T) {
b.InputFile = filepath.Join(testRoot, testDir, "main.c")
err := b.checkCbuildIdx()
assert.Error(err)
})

t.Run("test invalid file", func(t *testing.T) {
b.InputFile = filepath.Join(testRoot, testDir, "invalid-file.cbuild-idx.yml")
err := b.checkCbuildIdx()
assert.Error(err)
})
}

func TestGetDirs(t *testing.T) {
assert := assert.New(t)

Expand Down
3 changes: 2 additions & 1 deletion pkg/builder/cproject/builder.go
Original file line number Diff line number Diff line change
Expand Up @@ -167,7 +167,8 @@ func (b CprjBuilder) build() error {
if _, err := os.Stat(packlistFile); !os.IsNotExist(err) {
if b.Options.Packs {
if vars.CpackgetBin == "" {
err = errutils.New(errutils.ErrBinaryNotFound, "cpackget", "missing packs cannot be downloaded")
err = errutils.New(errutils.ErrBinaryNotFound,
"cpackget", "in "+vars.BinPath+". Missing packs cannot be installed")
return err
}
args = []string{"add", "--agree-embedded-license", "--no-dependencies", "--packs-list-filename", packlistFile}
Expand Down
2 changes: 1 addition & 1 deletion pkg/builder/cproject/builder_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,7 @@ func (r RunnerMock) ExecuteCommand(program string, quiet bool, args ...string) (
} else if strings.Contains(program, "ninja") {
} else if strings.Contains(program, "xmllint") {
} else {
return "", errutils.New(errutils.ErrInvalidCommand)
return "", errutils.New(errutils.ErrInvalidCommand, program)
}
return "", nil
}
Expand Down
25 changes: 16 additions & 9 deletions pkg/builder/csolution/builder.go
Original file line number Diff line number Diff line change
Expand Up @@ -234,7 +234,7 @@ func (b CSolutionBuilder) getCprjFilePath(idxFile string, context string) (strin
}
}
if path == "" {
err = errutils.New(errutils.ErrNoRefToCPRJFile, context+".cprj", idxFile)
err = errutils.New(errutils.ErrCPRJNotFound, context+".cprj")
} else {
cprjPath = filepath.Join(filepath.Dir(idxFile), filepath.Dir(path), context+".cprj")
}
Expand Down Expand Up @@ -273,26 +273,33 @@ func (b CSolutionBuilder) getCSolutionPath() (path string, err error) {
}

func (b CSolutionBuilder) getIdxFilePath() (string, error) {
projName, err := utils.GetProjectName(b.InputFile)
if err != nil {
return "", err
}

projName := b.getProjectName(b.InputFile)
outputDir := b.Options.Output
if outputDir == "" {
outputDir = filepath.Dir(b.InputFile)
}
idxFilePath := utils.NormalizePath(filepath.Join(outputDir, projName+".cbuild-idx.yml"))
_, err := utils.FileExists(idxFilePath)
if err != nil {
return "", err
}
return idxFilePath, nil
}

func (b CSolutionBuilder) getProjectName(csolutionFile string) (projectName string) {
csolutionFile = utils.NormalizePath(csolutionFile)
nameTokens := strings.Split(filepath.Base(csolutionFile), ".")
return nameTokens[0]
}

func (b CSolutionBuilder) getCbuildSetFilePath() (string, error) {
projName, err := utils.GetProjectName(b.InputFile)
projName := b.getProjectName(b.InputFile)
setFilePath := utils.NormalizePath(filepath.Join(filepath.Dir(b.InputFile), projName+".cbuild-set.yml"))

_, err := utils.FileExists(setFilePath)
if err != nil {
return "", err
}
setFilePath := utils.NormalizePath(filepath.Join(filepath.Dir(b.InputFile), projName+".cbuild-set.yml"))

return setFilePath, nil
}

Expand Down
11 changes: 1 addition & 10 deletions pkg/builder/csolution/builder_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,7 @@ func (r RunnerMock) ExecuteCommand(program string, quiet bool, args ...string) (
} else if strings.Contains(program, "ninja") {
} else if strings.Contains(program, "xmllint") {
} else {
return "", errutils.New(errutils.ErrInvalidCommand)
return "", errutils.New(errutils.ErrInvalidCommand, program)
}
return "", nil
}
Expand Down Expand Up @@ -431,15 +431,6 @@ func TestGetIdxFilePath(t *testing.T) {
assert.Nil(err)
assert.Equal(path, utils.NormalizePath(filepath.Join(testRoot, testDir, "TestSolution/test.cbuild-idx.yml")))
})

t.Run("test get idx file path with output path", func(t *testing.T) {
b.InputFile = filepath.Join(testRoot, testDir, "TestSolution/test.csolution.yml")
b.Options.Output = filepath.Join(testRoot, testDir, "outdir")

path, err := b.getIdxFilePath()
assert.Nil(err)
assert.Equal(path, utils.NormalizePath(b.Options.Output+"/test.cbuild-idx.yml"))
})
}

func TestFormulateArg(t *testing.T) {
Expand Down
24 changes: 11 additions & 13 deletions pkg/errutils/errutils.go
Original file line number Diff line number Diff line change
Expand Up @@ -12,19 +12,17 @@ import (
)

const (
ErrInvalidFileExtension = "unsupported file extension: %s. Supported extension(s): %s"
ErrInvalidCmdLineArg = "expected only one argument specifying the input file"
ErrFileNotExist = "file %s does not exist"
ErrNoContextFound = "no context(s) found to process"
ErrBinaryNotFound = "%s binary not found %s"
ErrMissingPacks = "missing packs must be installed, rerun cbuild with the --packs option"
ErrPathNotFound = "path %s not found"
ErrInvalidContextFormat = "invalid context format. Expected [project][.buildType][+targetType]"
ErrInvalidCSolutionFileName = "invalid csolution file name format. Expected '<projectname>.csolution.yml'"
ErrNoFilteredContextFound = "no suitable context matched for filter '%s'"
ErrNoRefToCPRJFile = "reference to '%s' not found in '%s' file"
ErrInvalidCommand = "invalid command entered. Please check your input and try again"
ErrInvalidFile = "invalid file: %s. Expected '%s' file"
ErrInvalidFileExtension = "invalid file extension: '%s'. Expected: '%s'"
ErrInvalidCmdLineArg = "multiple input files"
ErrFileNotExist = "file %s does not exist"
ErrNoContextFound = "no context found to process"
ErrBinaryNotFound = "%s not found %s"
ErrMissingPacks = "missing packs. Use --packs option with cbuild command to install them"
ErrETCPathNotFound = "couldn't locate '%s' directory relative to '%s'"
ErrInvalidContextFormat = "invalid context format. Expected [<project-name>][.<build-type>][+<target-type>]"
ErrNoFilteredContextFound = "no valid context found for '%s'"
ErrInvalidCommand = "invalid command '%s'. Run 'cbuild --help' for supported commands"
ErrCPRJNotFound = "couldn't locate %s file"
)

func New(errorFormat string, args ...any) error {
Expand Down
2 changes: 1 addition & 1 deletion pkg/utils/configs.go
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ func GetInstallConfigs() (configs Configurations, err error) {
configs.BinPath = binPath
etcPath := filepath.Clean(binPath + "/../etc")
if _, err = os.Stat(etcPath); os.IsNotExist(err) {
err = errutils.New(errutils.ErrPathNotFound, etcPath)
err = errutils.New(errutils.ErrETCPathNotFound, "../etc", configs.BinPath)
return Configurations{}, err
}
if etcPath != "" {
Expand Down
9 changes: 0 additions & 9 deletions pkg/utils/utils.go
Original file line number Diff line number Diff line change
Expand Up @@ -266,15 +266,6 @@ func NormalizePath(path string) string {
return path
}

func GetProjectName(csolutionFile string) (projectName string, err error) {
csolutionFile = NormalizePath(csolutionFile)
nameTokens := strings.Split(filepath.Base(csolutionFile), ".")
if len(nameTokens) != 3 || nameTokens[1] != "csolution" || nameTokens[2] != "yml" {
return "", errutils.New(errutils.ErrInvalidCSolutionFileName)
}
return nameTokens[0], nil
}

func ResolveContexts(allContext []string, contextFilters []string) ([]string, error) {
var selectedContexts []string

Expand Down
21 changes: 0 additions & 21 deletions pkg/utils/utils_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -246,27 +246,6 @@ func TestNormalizePath(t *testing.T) {
})
}

func TestGetProjectName(t *testing.T) {
assert := assert.New(t)
t.Run("test get project name from backslash path", func(t *testing.T) {
projName, err := GetProjectName("test\\input\\test.csolution.yml")
assert.Nil(err)
assert.Equal(projName, "test")
})

t.Run("test get project name from path", func(t *testing.T) {
projName, err := GetProjectName("test/input/test.csolution.yml")
assert.Nil(err)
assert.Equal(projName, "test")
})

t.Run("test get project name with invalid file name", func(t *testing.T) {
projName, err := GetProjectName("test/input/csolution.yml")
assert.Error(err)
assert.Equal(projName, "")
})
}

func TestResolveContexts(t *testing.T) {
assert := assert.New(t)

Expand Down
20 changes: 20 additions & 0 deletions test/data/TestSolution/test.cbuild-idx.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
build-idx:
generated-by: csolution version 1.1.1
description: test description string
csolution: ../data/TestSolution/test.csolution.yml
cprojects:
- cproject: ../data/TestSolution/TestProject2/test2.cproject.yml
- cproject: ../data/TestSolution/TestProject1/test1.cproject.yml
cbuilds:
- cbuild: test2.Debug+CM0.cbuild.yml
project: test2
configuration: .Debug+CM0
- cbuild: test2.Debug+CM3.cbuild.yml
project: test2
configuration: .Debug+CM3
- cbuild: test1.Debug+CM0.cbuild.yml
project: test1
configuration: .Debug+CM0
- cbuild: test1.Release+CM0.cbuild.yml
project: test1
configuration: .Release+CM0
7 changes: 7 additions & 0 deletions test/data/TestSolution/test.cbuild-set.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
cbuild-set:
generated-by: csolution version 2.1.0
contexts:
- context: test2.Debug+CM0
- context: test2.Debug+CM3
- context: test1.Debug+CM0
- context: test1.Release+CM0

0 comments on commit 01324ea

Please sign in to comment.