Skip to content

Commit df9f204

Browse files
cmagliematthijskooijman
cmaglie
and
matthijskooijman
authored
[skip-changelog] Some small refactoring on legacy package (#1064)
* legacy: output --preprocess result on ExecStdout * legacy: removed redundant argument filters * legacy: moving path-relativization code out of PrepareCommand * legacy: removed i18n on unneded message * legacy: replacing ParseCommandLine with the equivalent library call * legacy: removed parameter that happens to be always false * legacy: removed constants.MSG_PATTERN_MISSING from i18n * Let ExecRecipe return the command executed This prepares for building a compilation database later. The returned command is not currently used anywhere yet, so this commit should not change behaviour. * Fixed typo Co-authored-by: Matthijs Kooijman <[email protected]>
1 parent bb42ebe commit df9f204

13 files changed

+44
-159
lines changed

‎legacy/builder/builder.go

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,6 @@
1616
package builder
1717

1818
import (
19-
"fmt"
2019
"os"
2120
"reflect"
2221
"strconv"
@@ -159,7 +158,7 @@ func (s *Preprocess) Run(ctx *types.Context) error {
159158
}
160159

161160
// Output arduino-preprocessed source
162-
fmt.Println(ctx.Source)
161+
ctx.ExecStdout.Write([]byte(ctx.Source))
163162
return nil
164163
}
165164

‎legacy/builder/builder_utils/utils.go

Lines changed: 24 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -25,7 +25,6 @@ import (
2525
"sync"
2626

2727
"github.com/arduino/arduino-cli/legacy/builder/constants"
28-
"github.com/arduino/arduino-cli/legacy/builder/i18n"
2928
"github.com/arduino/arduino-cli/legacy/builder/types"
3029
"github.com/arduino/arduino-cli/legacy/builder/utils"
3130
"github.com/arduino/go-paths-helper"
@@ -249,7 +248,7 @@ func compileFileWithRecipe(ctx *types.Context, sourcePath *paths.Path, source *p
249248
return nil, errors.WithStack(err)
250249
}
251250
if !objIsUpToDate {
252-
_, _, err = ExecRecipe(ctx, properties, recipe, false /* stdout */, utils.ShowIfVerbose /* stderr */, utils.Show)
251+
_, _, _, err := ExecRecipe(ctx, properties, recipe, utils.ShowIfVerbose /* stdout */, utils.Show /* stderr */)
253252
if err != nil {
254253
return nil, errors.WithStack(err)
255254
}
@@ -480,46 +479,57 @@ func ArchiveCompiledFiles(ctx *types.Context, buildPath *paths.Path, archiveFile
480479
properties.SetPath(constants.BUILD_PROPERTIES_ARCHIVE_FILE_PATH, archiveFilePath)
481480
properties.SetPath(constants.BUILD_PROPERTIES_OBJECT_FILE, objectFile)
482481

483-
if _, _, err := ExecRecipe(ctx, properties, constants.RECIPE_AR_PATTERN, false /* stdout */, utils.ShowIfVerbose /* stderr */, utils.Show); err != nil {
482+
if _, _, _, err := ExecRecipe(ctx, properties, constants.RECIPE_AR_PATTERN, utils.ShowIfVerbose /* stdout */, utils.Show /* stderr */); err != nil {
484483
return nil, errors.WithStack(err)
485484
}
486485
}
487486

488487
return archiveFilePath, nil
489488
}
490489

491-
func ExecRecipe(ctx *types.Context, buildProperties *properties.Map, recipe string, removeUnsetProperties bool, stdout int, stderr int) ([]byte, []byte, error) {
490+
func ExecRecipe(ctx *types.Context, buildProperties *properties.Map, recipe string, stdout int, stderr int) (*exec.Cmd, []byte, []byte, error) {
492491
// See util.ExecCommand for stdout/stderr arguments
493-
command, err := PrepareCommandForRecipe(ctx, buildProperties, recipe, removeUnsetProperties)
492+
command, err := PrepareCommandForRecipe(buildProperties, recipe, false)
494493
if err != nil {
495-
return nil, nil, errors.WithStack(err)
494+
return nil, nil, nil, errors.WithStack(err)
496495
}
497496

498-
return utils.ExecCommand(ctx, command, stdout, stderr)
497+
outbytes, errbytes, err := utils.ExecCommand(ctx, command, stdout, stderr)
498+
return command, outbytes, errbytes, err
499499
}
500500

501501
const COMMANDLINE_LIMIT = 30000
502502

503-
func PrepareCommandForRecipe(ctx *types.Context, buildProperties *properties.Map, recipe string, removeUnsetProperties bool) (*exec.Cmd, error) {
504-
logger := ctx.GetLogger()
503+
func PrepareCommandForRecipe(buildProperties *properties.Map, recipe string, removeUnsetProperties bool) (*exec.Cmd, error) {
505504
pattern := buildProperties.Get(recipe)
506505
if pattern == "" {
507-
return nil, i18n.ErrorfWithLogger(logger, constants.MSG_PATTERN_MISSING, recipe)
506+
return nil, errors.Errorf("%s pattern is missing", recipe)
508507
}
509508

510-
var err error
511509
commandLine := buildProperties.ExpandPropsInString(pattern)
512510
if removeUnsetProperties {
513511
commandLine = properties.DeleteUnexpandedPropsFromString(commandLine)
514512
}
515513

516-
relativePath := ""
514+
command, err := utils.PrepareCommand(commandLine)
517515

516+
// if the overall commandline is too long for the platform
517+
// try reducing the length by making the filenames relative
518+
// and changing working directory to build.path
518519
if len(commandLine) > COMMANDLINE_LIMIT {
519-
relativePath = buildProperties.Get("build.path")
520+
relativePath := buildProperties.Get("build.path")
521+
for i, arg := range command.Args {
522+
if _, err := os.Stat(arg); os.IsNotExist(err) {
523+
continue
524+
}
525+
rel, err := filepath.Rel(relativePath, arg)
526+
if err == nil && !strings.Contains(rel, "..") && len(rel) < len(arg) {
527+
command.Args[i] = rel
528+
}
529+
}
530+
command.Dir = relativePath
520531
}
521532

522-
command, err := utils.PrepareCommand(commandLine, logger, relativePath)
523533
if err != nil {
524534
return nil, errors.WithStack(err)
525535
}

‎legacy/builder/constants/constants.go

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -90,7 +90,6 @@ const MSG_BUILD_OPTIONS_CHANGED = "Build options changed, rebuilding all"
9090
const MSG_CANT_FIND_SKETCH_IN_PATH = "Unable to find {0} in {1}"
9191
const MSG_FQBN_INVALID = "{0} is not a valid fully qualified board name. Required format is targetPackageName:targetPlatformName:targetBoardName."
9292
const MSG_FIND_INCLUDES_FAILED = "Error while detecting libraries included by {0}"
93-
const MSG_INVALID_QUOTING = "Invalid quoting: no closing [{0}] char found."
9493
const MSG_LIB_LEGACY = "(legacy)"
9594
const MSG_LIBRARIES_MULTIPLE_LIBS_FOUND_FOR = "Multiple libraries were found for \"{0}\""
9695
const MSG_LIBRARIES_NOT_USED = " Not used: {0}"
@@ -101,7 +100,6 @@ const MSG_LOOKING_FOR_RECIPES = "Looking for recipes like {0}*{1}"
101100
const MSG_MISSING_BUILD_BOARD = "Warning: Board {0}:{1}:{2} doesn''t define a ''build.board'' preference. Auto-set to: {3}"
102101
const MSG_MISSING_CORE_FOR_BOARD = "Selected board depends on '{0}' core (not installed)."
103102
const MSG_PACKAGE_UNKNOWN = "{0}: Unknown package"
104-
const MSG_PATTERN_MISSING = "{0} pattern is missing"
105103
const MSG_PLATFORM_UNKNOWN = "Platform {0} (package {1}) is unknown"
106104
const MSG_PROGRESS = "Progress {0}"
107105
const MSG_PROP_IN_LIBRARY = "Missing '{0}' from library in {1}"

‎legacy/builder/create_cmake_rule.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -240,8 +240,8 @@ func canExportCmakeProject(ctx *types.Context) bool {
240240
return ctx.BuildProperties.Get("compiler.export_cmake") != ""
241241
}
242242

243-
func extractCompileFlags(ctx *types.Context, receipe string, defines, dynamicLibs, linkerflags, linkDirectories *[]string, logger i18n.Logger) {
244-
command, _ := builder_utils.PrepareCommandForRecipe(ctx, ctx.BuildProperties, receipe, true)
243+
func extractCompileFlags(ctx *types.Context, recipe string, defines, dynamicLibs, linkerflags, linkDirectories *[]string, logger i18n.Logger) {
244+
command, _ := builder_utils.PrepareCommandForRecipe(ctx.BuildProperties, recipe, true)
245245

246246
for _, arg := range command.Args {
247247
if strings.HasPrefix(arg, "-D") {

‎legacy/builder/ctags_runner.go

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,6 @@ package builder
1818
import (
1919
"github.com/arduino/arduino-cli/legacy/builder/constants"
2020
"github.com/arduino/arduino-cli/legacy/builder/ctags"
21-
"github.com/arduino/arduino-cli/legacy/builder/i18n"
2221
"github.com/arduino/arduino-cli/legacy/builder/types"
2322
"github.com/arduino/arduino-cli/legacy/builder/utils"
2423
"github.com/pkg/errors"
@@ -29,19 +28,18 @@ type CTagsRunner struct{}
2928
func (s *CTagsRunner) Run(ctx *types.Context) error {
3029
buildProperties := ctx.BuildProperties
3130
ctagsTargetFilePath := ctx.CTagsTargetFile
32-
logger := ctx.GetLogger()
3331

3432
properties := buildProperties.Clone()
3533
properties.Merge(buildProperties.SubTree(constants.BUILD_PROPERTIES_TOOLS_KEY).SubTree(constants.CTAGS))
3634
properties.SetPath(constants.BUILD_PROPERTIES_SOURCE_FILE, ctagsTargetFilePath)
3735

3836
pattern := properties.Get(constants.BUILD_PROPERTIES_PATTERN)
3937
if pattern == constants.EMPTY_STRING {
40-
return i18n.ErrorfWithLogger(logger, constants.MSG_PATTERN_MISSING, constants.CTAGS)
38+
return errors.Errorf("%s pattern is missing", constants.CTAGS)
4139
}
4240

4341
commandLine := properties.ExpandPropsInString(pattern)
44-
command, err := utils.PrepareCommand(commandLine, logger, "")
42+
command, err := utils.PrepareCommand(commandLine)
4543
if err != nil {
4644
return errors.WithStack(err)
4745
}

‎legacy/builder/gcc_preproc_runner.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -69,7 +69,7 @@ func prepareGCCPreprocRecipeProperties(ctx *types.Context, sourceFilePath *paths
6969
properties.Set(constants.RECIPE_PREPROC_MACROS, GeneratePreprocPatternFromCompile(properties.Get(constants.RECIPE_CPP_PATTERN)))
7070
}
7171

72-
cmd, err := builder_utils.PrepareCommandForRecipe(ctx, properties, constants.RECIPE_PREPROC_MACROS, true)
72+
cmd, err := builder_utils.PrepareCommandForRecipe(properties, constants.RECIPE_PREPROC_MACROS, true)
7373
if err != nil {
7474
return nil, errors.WithStack(err)
7575
}

‎legacy/builder/phases/libraries_builder.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -58,7 +58,7 @@ func findExpectedPrecompiledLibFolder(ctx *types.Context, library *libraries.Lib
5858
// Add fpu specifications if they exist
5959
// To do so, resolve recipe.cpp.o.pattern,
6060
// search for -mfpu=xxx -mfloat-abi=yyy and add to a subfolder
61-
command, _ := builder_utils.PrepareCommandForRecipe(ctx, ctx.BuildProperties, constants.RECIPE_CPP_PATTERN, true)
61+
command, _ := builder_utils.PrepareCommandForRecipe(ctx.BuildProperties, constants.RECIPE_CPP_PATTERN, true)
6262
fpuSpecs := ""
6363
for _, el := range strings.Split(command.String(), " ") {
6464
if strings.Contains(el, FPU_CFLAG) {

‎legacy/builder/phases/linker.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -80,7 +80,7 @@ func link(ctx *types.Context, objectFiles paths.PathList, coreDotARelPath *paths
8080
properties.Set("archive_file", archive.Base())
8181
properties.SetPath("archive_file_path", archive)
8282
properties.SetPath("object_file", object)
83-
_, _, err := builder_utils.ExecRecipe(ctx, properties, constants.RECIPE_AR_PATTERN, false, utils.ShowIfVerbose /* stdout */, utils.Show /* stderr */)
83+
_, _, _, err := builder_utils.ExecRecipe(ctx, properties, constants.RECIPE_AR_PATTERN, utils.ShowIfVerbose /* stdout */, utils.Show /* stderr */)
8484
if err != nil {
8585
return err
8686
}
@@ -97,7 +97,7 @@ func link(ctx *types.Context, objectFiles paths.PathList, coreDotARelPath *paths
9797
properties.Set(constants.BUILD_PROPERTIES_ARCHIVE_FILE_PATH, coreArchiveFilePath.String())
9898
properties.Set("object_files", objectFileList)
9999

100-
_, _, err := builder_utils.ExecRecipe(ctx, properties, constants.RECIPE_C_COMBINE_PATTERN, false, utils.ShowIfVerbose /* stdout */, utils.Show /* stderr */)
100+
_, _, _, err := builder_utils.ExecRecipe(ctx, properties, constants.RECIPE_C_COMBINE_PATTERN, utils.ShowIfVerbose /* stdout */, utils.Show /* stderr */)
101101
return err
102102
}
103103

‎legacy/builder/phases/sizer.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -112,7 +112,7 @@ func checkSize(ctx *types.Context, buildProperties *properties.Map) error {
112112
}
113113

114114
func execSizeRecipe(ctx *types.Context, properties *properties.Map) (textSize int, dataSize int, eepromSize int, resErr error) {
115-
out, _, err := builder_utils.ExecRecipe(ctx, properties, constants.RECIPE_SIZE_PATTERN, false /* stdout */, utils.Capture /* stderr */, utils.Show)
115+
_, out, _, err := builder_utils.ExecRecipe(ctx, properties, constants.RECIPE_SIZE_PATTERN, utils.Capture /* stdout */, utils.Show /* stderr */)
116116
if err != nil {
117117
resErr = errors.New("Error while determining sketch size: " + err.Error())
118118
return

‎legacy/builder/preprocess_sketch.go

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -24,7 +24,6 @@ import (
2424

2525
bldr "github.com/arduino/arduino-cli/arduino/builder"
2626
"github.com/arduino/arduino-cli/legacy/builder/constants"
27-
"github.com/arduino/arduino-cli/legacy/builder/i18n"
2827
"github.com/arduino/arduino-cli/legacy/builder/types"
2928
"github.com/arduino/arduino-cli/legacy/builder/utils"
3029
properties "github.com/arduino/go-properties-orderedmap"
@@ -78,7 +77,6 @@ type ArduinoPreprocessorRunner struct{}
7877
func (s *ArduinoPreprocessorRunner) Run(ctx *types.Context) error {
7978
buildProperties := ctx.BuildProperties
8079
targetFilePath := ctx.PreprocPath.Join(constants.FILE_CTAGS_TARGET_FOR_GCC_MINUS_E)
81-
logger := ctx.GetLogger()
8280

8381
properties := buildProperties.Clone()
8482
toolProps := buildProperties.SubTree("tools").SubTree("arduino-preprocessor")
@@ -102,11 +100,11 @@ func (s *ArduinoPreprocessorRunner) Run(ctx *types.Context) error {
102100

103101
pattern := properties.Get(constants.BUILD_PROPERTIES_PATTERN)
104102
if pattern == constants.EMPTY_STRING {
105-
return i18n.ErrorfWithLogger(logger, constants.MSG_PATTERN_MISSING, "arduino-preprocessor")
103+
return errors.New("arduino-preprocessor pattern is missing")
106104
}
107105

108106
commandLine := properties.ExpandPropsInString(pattern)
109-
command, err := utils.PrepareCommand(commandLine, logger, "")
107+
command, err := utils.PrepareCommand(commandLine)
110108
if err != nil {
111109
return errors.WithStack(err)
112110
}

‎legacy/builder/recipe_runner.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -47,7 +47,7 @@ func (s *RecipeByPrefixSuffixRunner) Run(ctx *types.Context) error {
4747
if ctx.DebugLevel >= 10 {
4848
logger.Fprintln(os.Stdout, constants.LOG_LEVEL_DEBUG, constants.MSG_RUNNING_RECIPE, recipe)
4949
}
50-
_, _, err := builder_utils.ExecRecipe(ctx, properties, recipe, false /* stdout */, utils.ShowIfVerbose /* stderr */, utils.Show)
50+
_, _, _, err := builder_utils.ExecRecipe(ctx, properties, recipe, utils.ShowIfVerbose /* stdout */, utils.Show /* stderr */)
5151
if err != nil {
5252
return errors.WithStack(err)
5353
}

‎legacy/builder/test/utils_test.go

Lines changed: 3 additions & 43 deletions
Original file line numberDiff line numberDiff line change
@@ -16,45 +16,12 @@
1616
package test
1717

1818
import (
19-
"github.com/arduino/arduino-cli/legacy/builder/i18n"
20-
"github.com/arduino/arduino-cli/legacy/builder/utils"
21-
"github.com/stretchr/testify/require"
2219
"strings"
2320
"testing"
24-
)
25-
26-
func TestCommandLineParser(t *testing.T) {
27-
command := "\"/home/federico/materiale/works_Arduino/Arduino/build/hardware/tools/coan\" source -m -E -P -kb -c -g -Os -w -ffunction-sections -fdata-sections -MMD -mmcu=atmega32u4 -DF_CPU=16000000L -DARDUINO=010600 -DARDUINO_AVR_LEONARDO -DARDUINO_ARCH_AVR -DUSB_VID=0x2341 -DUSB_PID=0x8036 '-DUSB_MANUFACTURER=' '-DUSB_PRODUCT=\"Arduino Leonardo\"' \"/tmp/sketch321469072.cpp\""
2821

29-
parts, err := utils.ParseCommandLine(command, i18n.HumanLogger{})
30-
NoError(t, err)
31-
32-
require.Equal(t, 23, len(parts))
33-
34-
require.Equal(t, "/home/federico/materiale/works_Arduino/Arduino/build/hardware/tools/coan", parts[0])
35-
require.Equal(t, "source", parts[1])
36-
require.Equal(t, "-m", parts[2])
37-
require.Equal(t, "-E", parts[3])
38-
require.Equal(t, "-P", parts[4])
39-
require.Equal(t, "-kb", parts[5])
40-
require.Equal(t, "-c", parts[6])
41-
require.Equal(t, "-g", parts[7])
42-
require.Equal(t, "-Os", parts[8])
43-
require.Equal(t, "-w", parts[9])
44-
require.Equal(t, "-ffunction-sections", parts[10])
45-
require.Equal(t, "-fdata-sections", parts[11])
46-
require.Equal(t, "-MMD", parts[12])
47-
require.Equal(t, "-mmcu=atmega32u4", parts[13])
48-
require.Equal(t, "-DF_CPU=16000000L", parts[14])
49-
require.Equal(t, "-DARDUINO=010600", parts[15])
50-
require.Equal(t, "-DARDUINO_AVR_LEONARDO", parts[16])
51-
require.Equal(t, "-DARDUINO_ARCH_AVR", parts[17])
52-
require.Equal(t, "-DUSB_VID=0x2341", parts[18])
53-
require.Equal(t, "-DUSB_PID=0x8036", parts[19])
54-
require.Equal(t, "-DUSB_MANUFACTURER=", parts[20])
55-
require.Equal(t, "-DUSB_PRODUCT=\"Arduino Leonardo\"", parts[21])
56-
require.Equal(t, "/tmp/sketch321469072.cpp", parts[22])
57-
}
22+
"github.com/arduino/arduino-cli/legacy/builder/utils"
23+
"github.com/stretchr/testify/require"
24+
)
5825

5926
func TestPrintableCommand(t *testing.T) {
6027
parts := []string{
@@ -75,13 +42,6 @@ func TestPrintableCommand(t *testing.T) {
7542
require.Equal(t, correct, result)
7643
}
7744

78-
func TestCommandLineParserError(t *testing.T) {
79-
command := "\"command missing quote"
80-
81-
_, err := utils.ParseCommandLine(command, i18n.HumanLogger{})
82-
require.Error(t, err)
83-
}
84-
8545
func TestMapTrimSpace(t *testing.T) {
8646
value := "hello, world , how are,you? "
8747
parts := utils.Map(strings.Split(value, ","), utils.TrimSpace)

0 commit comments

Comments
 (0)