From 509e86590c99aa321022ac80d1ba92dd21073935 Mon Sep 17 00:00:00 2001 From: Sourabh Mehta Date: Thu, 15 Feb 2024 14:36:55 +0100 Subject: [PATCH] Addressed review commments --- pkg/builder/cbuildidx/builder.go | 50 +++++++++-------- pkg/builder/cbuildidx/builder_test.go | 21 +++---- pkg/builder/cproject/builder.go | 4 ++ pkg/builder/csolution/builder.go | 81 +++++++++++++++------------ 4 files changed, 83 insertions(+), 73 deletions(-) diff --git a/pkg/builder/cbuildidx/builder.go b/pkg/builder/cbuildidx/builder.go index 361ab88..d9a34dc 100644 --- a/pkg/builder/cbuildidx/builder.go +++ b/pkg/builder/cbuildidx/builder.go @@ -61,17 +61,12 @@ func (b CbuildIdxBuilder) clean(dirs builder.BuildDirs, vars builder.InternalVar return nil } -func (b CbuildIdxBuilder) getDirs() (dirs builder.BuildDirs, err error) { +func (b CbuildIdxBuilder) getDirs(context string) (dirs builder.BuildDirs, err error) { if _, err := os.Stat(b.InputFile); os.IsNotExist(err) { log.Error("file " + b.InputFile + " does not exist") return dirs, err } - 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 } @@ -88,7 +83,7 @@ func (b CbuildIdxBuilder) getDirs() (dirs builder.BuildDirs, err error) { 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") + cbuildFile := filepath.Join(path, context+".cbuild.yml") _, outDir, err := GetBuildDirs(cbuildFile) if err != nil { log.Error("error parsing file: " + cbuildFile) @@ -127,18 +122,28 @@ func (b CbuildIdxBuilder) Build() error { _ = utils.UpdateEnvVars(vars.BinPath, vars.EtcPath) - dirs, err := b.getDirs() - if err != nil { + if len(b.Options.Contexts) == 0 { + err = errors.New("error no context(s) to process") return err } - if b.Options.Rebuild { - err = b.clean(dirs, vars) - if err != nil { - return err + dirs := builder.BuildDirs{ + IntDir: filepath.Join(filepath.Dir(b.InputFile), "tmp"), + } + + if b.Options.Clean { + for _, context := range b.Options.Contexts { + dirs, err := b.getDirs(context) + if err != nil { + return err + } + + log.Info("Cleaning context: \"" + context + "\"") + if err := b.clean(dirs, vars); err != nil { + return err + } } - } else if b.Options.Clean { - return b.clean(dirs, vars) + return nil } args := []string{b.InputFile} @@ -151,7 +156,6 @@ func (b CbuildIdxBuilder) Build() error { log.Error("error executing 'cbuild2cmake " + b.InputFile + "'") return err } - if _, err := os.Stat(dirs.IntDir + "/CMakeLists.txt"); errors.Is(err, os.ErrNotExist) { return err } @@ -160,7 +164,6 @@ func (b CbuildIdxBuilder) Build() error { log.Error("cmake was not found") return err } - if b.Options.Generator == "" { b.Options.Generator = "Ninja" if vars.NinjaBin == "" { @@ -169,6 +172,7 @@ func (b CbuildIdxBuilder) Build() error { } } + // CMake configuration command args = []string{"-G", b.Options.Generator, "-S", dirs.IntDir, "-B", dirs.IntDir} if b.Options.Debug { args = append(args, "-Wdev") @@ -186,15 +190,13 @@ func (b CbuildIdxBuilder) Build() error { return err } + // CMake build target(s) command args = []string{"--build", dirs.IntDir, "-j", fmt.Sprintf("%d", b.GetJobs())} - - if len(b.Options.Contexts) == 1 { - args = append(args, "--target", b.Options.Contexts[0]) - args = append(args, "--target", b.Options.Contexts[0]+"-database") - } else { - err = errors.New("error invalid context(s) process request") - return err + for _, context := range b.Options.Contexts { + args = append(args, "--target", context) + args = append(args, "--target", context+"-database") } + if b.Options.Debug || b.Options.Verbose { args = append(args, "--verbose") } diff --git a/pkg/builder/cbuildidx/builder_test.go b/pkg/builder/cbuildidx/builder_test.go index a03cf06..81fe5aa 100644 --- a/pkg/builder/cbuildidx/builder_test.go +++ b/pkg/builder/cbuildidx/builder_test.go @@ -79,14 +79,11 @@ func TestGetDirs(t *testing.T) { builder.BuilderParams{ Runner: RunnerMock{}, InputFile: filepath.Join(testRoot, testDir, "Hello.cbuild-idx.yml"), - Options: builder.Options{ - Contexts: []string{"Hello.Debug+AVH"}, - }, }, } - t.Run("test valid directories in cprj", func(t *testing.T) { - dirs, err := b.getDirs() + t.Run("test valid directories", func(t *testing.T) { + dirs, err := b.getDirs("Hello.Debug+AVH") assert.Nil(err) intDir, _ := filepath.Abs(filepath.Join(testRoot, testDir, "tmp")) outDir, _ := filepath.Abs(filepath.Join(testRoot, testDir, "out/AVH")) @@ -97,7 +94,7 @@ func TestGetDirs(t *testing.T) { t.Run("test valid directories as arguments", func(t *testing.T) { b.Options.IntDir = "cmdOptionsIntDir" b.Options.OutDir = "cmdOptionsOutDir" - dirs, err := b.getDirs() + dirs, err := b.getDirs("Hello.Debug+AVH") assert.Nil(err) intDir, _ := filepath.Abs(filepath.Join(testRoot, testDir, "tmp")) outDir, _ := filepath.Abs(b.Options.OutDir) @@ -105,9 +102,9 @@ func TestGetDirs(t *testing.T) { assert.Equal(outDir, dirs.OutDir) }) - t.Run("test invalid cprj", func(t *testing.T) { + t.Run("test invalid cbuild-idx", func(t *testing.T) { b.InputFile = filepath.Join(testRoot, testDir, "invalid-file.cbuild-idx.yml") - _, err := b.getDirs() + _, err := b.getDirs("Hello.Debug+AVH") assert.Error(err) }) } @@ -173,19 +170,19 @@ func TestBuild(t *testing.T) { assert.Nil(err) }) - t.Run("test build cprj quiet", func(t *testing.T) { + t.Run("test build cbuild-idx quiet", func(t *testing.T) { b.Options.Quiet = true err := b.Build() assert.Nil(err) }) - t.Run("test build cprj debug", func(t *testing.T) { + t.Run("test build cbuild-idx debug", func(t *testing.T) { b.Options.Debug = true err := b.Build() assert.Nil(err) }) - t.Run("test rebuild cprj", func(t *testing.T) { + t.Run("test rebuild cbuild-idx", func(t *testing.T) { b.Options.Rebuild = true err := b.Build() assert.Nil(err) @@ -218,7 +215,7 @@ func TestBuild(t *testing.T) { assert.Nil(err) }) - t.Run("test clean cprj", func(t *testing.T) { + t.Run("test clean cbuild-idx", func(t *testing.T) { b.Options.Clean = true err := b.Build() assert.Nil(err) diff --git a/pkg/builder/cproject/builder.go b/pkg/builder/cproject/builder.go index 6481913..dc3e066 100644 --- a/pkg/builder/cproject/builder.go +++ b/pkg/builder/cproject/builder.go @@ -36,6 +36,10 @@ func (b CprjBuilder) checkCprj() error { } func (b CprjBuilder) clean(dirs builder.BuildDirs, vars builder.InternalVars) (err error) { + fileName := filepath.Base(b.InputFile) + fileName = fileName[:len(fileName)-len(filepath.Ext(fileName))] + log.Info("Cleaning context: \"" + fileName + "\"") + if _, err := os.Stat(dirs.IntDir); !os.IsNotExist(err) { _, err = b.Runner.ExecuteCommand(vars.CbuildgenBin, false, "rmdir", dirs.IntDir) if err != nil { diff --git a/pkg/builder/csolution/builder.go b/pkg/builder/csolution/builder.go index 694b9ef..1886fe4 100644 --- a/pkg/builder/csolution/builder.go +++ b/pkg/builder/csolution/builder.go @@ -195,40 +195,42 @@ func (b CSolutionBuilder) getSetFilePath() (string, error) { } func (b CSolutionBuilder) getProjsBuilders(selectedContexts []string) (projBuilders []builder.IBuilderInterface, err error) { - for _, context := range selectedContexts { - infoMsg := "Retrieve build information for context: \"" + context + "\"" - log.Info(infoMsg) + buildOptions := b.Options - // if --output is used, ignore provided --outdir and --intdir - if b.Options.Output != "" && (b.Options.OutDir != "" || b.Options.IntDir != "") { - log.Warn("output files are generated under: \"" + - b.Options.Output + "\". Options --outdir and --intdir shall be ignored.") - } + // Set XML schema check to false, when input is yml + if b.Options.Schema { + buildOptions.Schema = false + } - idxFile, err := b.getIdxFilePath() - if err != nil { - return projBuilders, err - } + idxFile, err := b.getIdxFilePath() + if err != nil { + return projBuilders, err + } - buildOptions := b.Options - // Set XML schema check to false, when input is yml - if b.Options.Schema { - buildOptions.Schema = false + var projBuilder builder.IBuilderInterface + if b.Options.UseCbuild2CMake { + buildOptions.Contexts = selectedContexts + // get idx builder + projBuilder = cbuildidx.CbuildIdxBuilder{ + BuilderParams: builder.BuilderParams{ + Runner: b.Runner, + Options: buildOptions, + InputFile: idxFile, + InstallConfigs: b.InstallConfigs, + }, } - - var projBuilder builder.IBuilderInterface - if b.Options.UseCbuild2CMake { - buildOptions.Contexts = []string{context} - // get cprj builder - projBuilder = cbuildidx.CbuildIdxBuilder{ - BuilderParams: builder.BuilderParams{ - Runner: b.Runner, - Options: buildOptions, - InputFile: idxFile, - InstallConfigs: b.InstallConfigs, - }, + projBuilders = append(projBuilders, projBuilder) + } else { + for _, context := range selectedContexts { + infoMsg := "Retrieve build information for context: \"" + context + "\"" + log.Info(infoMsg) + + // if --output is used, ignore provided --outdir and --intdir + if b.Options.Output != "" && (b.Options.OutDir != "" || b.Options.IntDir != "") { + log.Warn("output files are generated under: \"" + + b.Options.Output + "\". Options --outdir and --intdir shall be ignored.") } - } else { + cprjFile, err := b.getCprjFilePath(idxFile, context) if err != nil { log.Error("error getting cprj file: " + err.Error()) @@ -244,8 +246,8 @@ func (b CSolutionBuilder) getProjsBuilders(selectedContexts []string) (projBuild InstallConfigs: b.InstallConfigs, }, } + projBuilders = append(projBuilders, projBuilder) } - projBuilders = append(projBuilders, projBuilder) } return projBuilders, err } @@ -276,10 +278,11 @@ func (b CSolutionBuilder) getBuilderInputFile(builder builder.IBuilderInterface) return inputFile } -func (b CSolutionBuilder) cleanContexts(selectedContexts []string, projBuilders []builder.IBuilderInterface) (err error) { +// func (b CSolutionBuilder) cleanContexts(selectedContexts []string, projBuilders []builder.IBuilderInterface) (err error) { +func (b CSolutionBuilder) cleanContexts(projBuilders []builder.IBuilderInterface) (err error) { for index := range projBuilders { - infoMsg := "Cleaning context: \"" + selectedContexts[index] + "\"" - log.Info(infoMsg) + // infoMsg := "Cleaning context: \"" + selectedContexts[index] + "\"" + // log.Info(infoMsg) b.setBuilderOptions(&projBuilders[index], true) err = projBuilders[index].Build() @@ -292,12 +295,16 @@ func (b CSolutionBuilder) cleanContexts(selectedContexts []string, projBuilders func (b CSolutionBuilder) buildContexts(selectedContexts []string, projBuilders []builder.IBuilderInterface) (err error) { for index := range projBuilders { - progress := fmt.Sprintf("(%s/%d)", strconv.Itoa(index+1), len(selectedContexts)) - infoMsg := progress + " Building context: \"" + selectedContexts[index] + "\"" + var infoMsg string + if b.Options.UseContextSet { + infoMsg = " Building " + b.InputFile + } else { + progress := fmt.Sprintf("(%s/%d)", strconv.Itoa(index+1), len(selectedContexts)) + infoMsg = progress + " Building context: \"" + selectedContexts[index] + "\"" + } sep := strings.Repeat("=", len(infoMsg)+13) + "\n" _, _ = log.StandardLogger().Out.Write([]byte(sep)) log.Info(infoMsg) - b.setBuilderOptions(&projBuilders[index], false) err = projBuilders[index].Build() if err != nil { @@ -480,7 +487,7 @@ func (b CSolutionBuilder) Build() (err error) { // clean all selected contexts when rebuild or clean are requested if b.Options.Rebuild || b.Options.Clean { - err = b.cleanContexts(selectedContexts, projBuilders) + err = b.cleanContexts(projBuilders) if b.Options.Clean || err != nil { return err }