Code climate findings

Open-CMSIS-Pack · Feb 14, 2024 · 795fc3a · 795fc3a
1 parent ec73be6
commit 795fc3a
Show file tree

Hide file tree

Showing 5 changed files with 96 additions and 49 deletions.
diff --git a/pkg/builder/cbuildidx/builder.go b/pkg/builder/cbuildidx/builder.go
@@ -37,35 +37,41 @@ func (b CbuildIdxBuilder) checkCbuildIdx() error {
 }
 
 func (b CbuildIdxBuilder) clean(dirs builder.BuildDirs, vars builder.InternalVars) (err error) {
-	if _, err := os.Stat(dirs.IntDir); !os.IsNotExist(err) {
-		args := []string{"-E", "remove_directory", dirs.IntDir}
-		_, err = b.Runner.ExecuteCommand(vars.CmakeBin, false, args...)
-		if err != nil {
-			log.Error("error executing 'cmake' clean " + dirs.IntDir)
+	removeDirectory := func(dir string) error {
+		if _, err := os.Stat(dir); os.IsNotExist(err) {
 			return err
 		}
-	}
-	if _, err := os.Stat(dirs.OutDir); !os.IsNotExist(err) {
-		args := []string{"-E", "remove_directory", dirs.OutDir}
+		args := []string{"-E", "remove_directory", dir}
 		_, err = b.Runner.ExecuteCommand(vars.CmakeBin, false, args...)
 		if err != nil {
-			log.Error("error executing 'cmake' clean " + dirs.OutDir)
-			return err
+			log.Error("error executing 'cmake' clean for " + dir)
 		}
+		return err
+	}
+
+	if err := removeDirectory(dirs.IntDir); err != nil {
+		return err
+	}
+
+	if err := removeDirectory(dirs.OutDir); err != nil {
+		return err
 	}
+
 	log.Info("clean finished successfully!")
 	return nil
 }
 
 func (b CbuildIdxBuilder) getDirs() (dirs builder.BuildDirs, err error) {
 	if _, err := os.Stat(b.InputFile); os.IsNotExist(err) {
-		log.Error("project file " + b.InputFile + " does not exist")
+		log.Error("file " + b.InputFile + " does not exist")
 		return dirs, err
 	}
 
-	if b.Options.IntDir != "" {
-		dirs.IntDir = b.Options.IntDir
+	if len(b.Options.Contexts) != 1 {
+		err = errors.New("error invalid context(s) process request")
+		return dirs, err
 	}
+
 	if b.Options.OutDir != "" {
 		dirs.OutDir = b.Options.OutDir
 	}
@@ -75,19 +81,20 @@ func (b CbuildIdxBuilder) getDirs() (dirs builder.BuildDirs, err error) {
 		dirs.OutDir = ""
 	}
 
-	path := filepath.Dir(b.InputFile)
-	cbuildFile := filepath.Join(path, b.Options.Contexts[0]+".cbuild.yml")
-	_, outDir, err := GetBuildDirs(cbuildFile)
-	if err != nil {
-		log.Error("error parsing file: " + cbuildFile)
-		return dirs, err
-	}
-
 	// cbuild2cmake generates cmake files under fixed tmp directory
 	dirs.IntDir = "tmp"
 	dirs.IntDir = filepath.Join(filepath.Dir(b.InputFile), dirs.IntDir)
 
 	if dirs.OutDir == "" {
+		// get output directory from cbuild.yml file
+		path := filepath.Dir(b.InputFile)
+		cbuildFile := filepath.Join(path, b.Options.Contexts[0]+".cbuild.yml")
+		_, outDir, err := GetBuildDirs(cbuildFile)
+		if err != nil {
+			log.Error("error parsing file: " + cbuildFile)
+			return dirs, err
+		}
+
 		dirs.OutDir = outDir
 		if dirs.OutDir == "" {
 			dirs.OutDir = "OutDir"

diff --git a/pkg/builder/cbuildidx/reader.go b/pkg/builder/cbuildidx/reader.go
@@ -17,7 +17,6 @@ type Cbuild struct {
 		OutputDirs struct {
 			Intdir string `yaml:"intdir"`
 			Outdir string `yaml:"outdir"`
-			// Rtedir string `yaml:"rtedir"`
 		} `yaml:"output-dirs"`
 	} `yaml:"build"`
 }

diff --git a/pkg/builder/cbuildidx/reader_test.go b/pkg/builder/cbuildidx/reader_test.go
@@ -0,0 +1,31 @@
+/*
+ * Copyright (c) 2024 Arm Limited. All rights reserved.
+ *
+ * SPDX-License-Identifier: Apache-2.0
+ */
+
+package cbuildidx
+
+import (
+	"path/filepath"
+	"testing"
+
+	"github.com/stretchr/testify/assert"
+)
+
+func TestGetBuildDirs(t *testing.T) {
+	assert := assert.New(t)
+	t.Run("test get build directories when file is missing", func(t *testing.T) {
+		cbuildFile := filepath.Join(testRoot, testDir, "missing.cbuild.yml")
+		_, _, err := GetBuildDirs(cbuildFile)
+		assert.Error(err)
+	})
+
+	t.Run("test get build directories from .cbuild.yml", func(t *testing.T) {
+		cbuildFile := filepath.Join(testRoot, testDir, "Hello.Debug+AVH.cbuild.yml")
+		intDir, outDir, err := GetBuildDirs(cbuildFile)
+		assert.Nil(err)
+		assert.Equal("tmp/Hello/AVH/Debug", intDir)
+		assert.Equal("out/AVH", outDir)
+	})
+}
diff --git a/pkg/builder/csolution/builder.go b/pkg/builder/csolution/builder.go
@@ -250,27 +250,41 @@ func (b CSolutionBuilder) getProjsBuilders(selectedContexts []string) (projBuild
 	return projBuilders, err
 }
 
+func (b CSolutionBuilder) setBuilderOptions(builder *builder.IBuilderInterface, clean bool) {
+	if b.Options.UseCbuild2CMake {
+		idxBuilder := (*builder).(cbuildidx.CbuildIdxBuilder)
+		idxBuilder.Options.Rebuild = false
+		idxBuilder.Options.Clean = clean
+		(*builder) = idxBuilder
+	} else {
+		cprjBuilder := (*builder).(cproject.CprjBuilder)
+		cprjBuilder.Options.Rebuild = false
+		cprjBuilder.Options.Clean = clean
+		(*builder) = cprjBuilder
+	}
+}
+
+func (b CSolutionBuilder) getBuilderInputFile(builder builder.IBuilderInterface) string {
+	var inputFile string
+	if b.Options.UseCbuild2CMake {
+		idxBuilder := builder.(cbuildidx.CbuildIdxBuilder)
+		inputFile = idxBuilder.InputFile
+	} else {
+		cprjBuilder := builder.(cproject.CprjBuilder)
+		inputFile = cprjBuilder.InputFile
+	}
+	return inputFile
+}
+
 func (b CSolutionBuilder) cleanContexts(selectedContexts []string, projBuilders []builder.IBuilderInterface) (err error) {
 	for index, bldr := range projBuilders {
 		infoMsg := "Cleaning context: \"" + selectedContexts[index] + "\""
 		log.Info(infoMsg)
 
-		var inputFile string
-		if b.Options.UseCbuild2CMake {
-			idxBuilder := bldr.(cbuildidx.CbuildIdxBuilder)
-			idxBuilder.Options.Rebuild = false
-			idxBuilder.Options.Clean = true
-			inputFile = idxBuilder.InputFile
-		} else {
-			cprjBuilder := bldr.(cproject.CprjBuilder)
-			cprjBuilder.Options.Rebuild = false
-			cprjBuilder.Options.Clean = true
-			inputFile = cprjBuilder.InputFile
-		}
-
+		b.setBuilderOptions(&bldr, true)
 		err = bldr.Build()
 		if err != nil {
-			log.Error("error cleaning '" + inputFile + "'")
+			log.Error("error cleaning '" + b.getBuilderInputFile(bldr) + "'")
 		}
 	}
 	return
@@ -284,21 +298,10 @@ func (b CSolutionBuilder) buildContexts(selectedContexts []string, projBuilders
 		_, _ = log.StandardLogger().Out.Write([]byte(sep))
 		log.Info(infoMsg)
 
-		var inputFile string
-		if b.Options.UseCbuild2CMake {
-			idxBuilder := bldr.(cbuildidx.CbuildIdxBuilder)
-			idxBuilder.Options.Rebuild = false
-			idxBuilder.Options.Clean = false
-			inputFile = idxBuilder.InputFile
-		} else {
-			cprjBuilder := bldr.(cproject.CprjBuilder)
-			cprjBuilder.Options.Rebuild = false
-			cprjBuilder.Options.Clean = false
-			inputFile = cprjBuilder.InputFile
-		}
+		b.setBuilderOptions(&bldr, false)
 		err = bldr.Build()
 		if err != nil {
-			log.Error("error building '" + inputFile + "'")
+			log.Error("error building '" + b.getBuilderInputFile(bldr) + "'")
 		}
 	}
 	return

diff --git a/pkg/builder/csolution/builder_test.go b/pkg/builder/csolution/builder_test.go
@@ -260,6 +260,13 @@ func TestBuild(t *testing.T) {
 		err := b.Build()
 		assert.Error(err)
 	})
+
+	t.Run("test build csolution using cbuild2cmake", func(t *testing.T) {
+		b.Options.Contexts = []string{"test.Debug+CM0"}
+		b.Options.UseCbuild2CMake = true
+		err := b.Build()
+		assert.Error(err)
+	})
 }
 
 func TestRebuild(t *testing.T) {