From 64792ce69fe5d7d7549c3b12c4e4e42d3397e1f4 Mon Sep 17 00:00:00 2001 From: Cristian Maglie Date: Thu, 4 Mar 2021 00:27:29 +0100 Subject: [PATCH 1/9] Remove some constants indirection --- legacy/builder/constants/constants.go | 7 ------- .../builder/merge_sketch_with_bootloader.go | 2 +- legacy/builder/phases/sizer.go | 19 +++++++++---------- 3 files changed, 10 insertions(+), 18 deletions(-) diff --git a/legacy/builder/constants/constants.go b/legacy/builder/constants/constants.go index 4bd03d31aac..464bdc46937 100644 --- a/legacy/builder/constants/constants.go +++ b/legacy/builder/constants/constants.go @@ -64,19 +64,12 @@ const PLATFORM_REWRITE_NEW = "new" const PLATFORM_REWRITE_OLD = "old" const PLATFORM_URL = "url" const PLATFORM_VERSION = "version" -const PROPERTY_WARN_DATA_PERCENT = "build.warn_data_percentage" -const PROPERTY_UPLOAD_MAX_SIZE = "upload.maximum_size" -const PROPERTY_UPLOAD_MAX_DATA_SIZE = "upload.maximum_data_size" const RECIPE_AR_PATTERN = "recipe.ar.pattern" const RECIPE_C_COMBINE_PATTERN = "recipe.c.combine.pattern" const RECIPE_C_PATTERN = "recipe.c.o.pattern" const RECIPE_CPP_PATTERN = "recipe.cpp.o.pattern" -const RECIPE_SIZE_PATTERN = "recipe.size.pattern" const RECIPE_PREPROC_MACROS = "recipe.preproc.macros" const RECIPE_S_PATTERN = "recipe.S.o.pattern" -const RECIPE_SIZE_REGEXP = "recipe.size.regex" -const RECIPE_SIZE_REGEXP_DATA = "recipe.size.regex.data" -const RECIPE_SIZE_REGEXP_EEPROM = "recipe.size.regex.eeprom" const REWRITING_DISABLED = "disabled" const REWRITING = "rewriting" const SPACE = " " diff --git a/legacy/builder/merge_sketch_with_bootloader.go b/legacy/builder/merge_sketch_with_bootloader.go index 377b5a9f697..b16b293ea44 100644 --- a/legacy/builder/merge_sketch_with_bootloader.go +++ b/legacy/builder/merge_sketch_with_bootloader.go @@ -75,7 +75,7 @@ func (s *MergeSketchWithBootloader) Run(ctx *types.Context) error { // Ignore merger errors for the first iteration maximumBinSize := 16000000 - if uploadMaxSize, ok := ctx.BuildProperties.GetOk(constants.PROPERTY_UPLOAD_MAX_SIZE); ok { + if uploadMaxSize, ok := ctx.BuildProperties.GetOk("upload.maximum_size"); ok { maximumBinSize, _ = strconv.Atoi(uploadMaxSize) maximumBinSize *= 2 } diff --git a/legacy/builder/phases/sizer.go b/legacy/builder/phases/sizer.go index de73f7999df..7aa437daf83 100644 --- a/legacy/builder/phases/sizer.go +++ b/legacy/builder/phases/sizer.go @@ -21,7 +21,6 @@ import ( "strconv" "github.com/arduino/arduino-cli/legacy/builder/builder_utils" - "github.com/arduino/arduino-cli/legacy/builder/constants" "github.com/arduino/arduino-cli/legacy/builder/types" "github.com/arduino/arduino-cli/legacy/builder/utils" "github.com/arduino/go-properties-orderedmap" @@ -52,10 +51,10 @@ func (s *Sizer) Run(ctx *types.Context) error { func checkSize(ctx *types.Context, buildProperties *properties.Map) error { properties := buildProperties.Clone() - properties.Set(constants.BUILD_PROPERTIES_COMPILER_WARNING_FLAGS, properties.Get(constants.BUILD_PROPERTIES_COMPILER_WARNING_FLAGS+"."+ctx.WarningsLevel)) + properties.Set("compiler.warning_flags", properties.Get("compiler.warning_flags."+ctx.WarningsLevel)) - maxTextSizeString := properties.Get(constants.PROPERTY_UPLOAD_MAX_SIZE) - maxDataSizeString := properties.Get(constants.PROPERTY_UPLOAD_MAX_DATA_SIZE) + maxTextSizeString := properties.Get("upload.maximum_size") + maxDataSizeString := properties.Get("upload.maximum_data_size") if maxTextSizeString == "" { return nil @@ -121,8 +120,8 @@ func checkSize(ctx *types.Context, buildProperties *properties.Map) error { return errors.New(tr("data section exceeds available space in board")) } - if properties.Get(constants.PROPERTY_WARN_DATA_PERCENT) != "" { - warnDataPercentage, err := strconv.Atoi(properties.Get(constants.PROPERTY_WARN_DATA_PERCENT)) + if w := properties.Get("build.warn_data_percentage"); w != "" { + warnDataPercentage, err := strconv.Atoi(w) if err != nil { return err } @@ -135,7 +134,7 @@ func checkSize(ctx *types.Context, buildProperties *properties.Map) error { } func execSizeRecipe(ctx *types.Context, properties *properties.Map) (textSize int, dataSize int, eepromSize int, resErr error) { - command, err := builder_utils.PrepareCommandForRecipe(properties, constants.RECIPE_SIZE_PATTERN, false, ctx.PackageManager.GetEnvVarsForSpawnedProcess()) + command, err := builder_utils.PrepareCommandForRecipe(properties, "recipe.size.pattern", false, ctx.PackageManager.GetEnvVarsForSpawnedProcess()) if err != nil { resErr = fmt.Errorf(tr("Error while determining sketch size: %s"), err) return @@ -150,7 +149,7 @@ func execSizeRecipe(ctx *types.Context, properties *properties.Map) (textSize in // force multiline match prepending "(?m)" to the actual regexp // return an error if RECIPE_SIZE_REGEXP doesn't exist - textSize, err = computeSize(properties.Get(constants.RECIPE_SIZE_REGEXP), out) + textSize, err = computeSize(properties.Get("recipe.size.regex"), out) if err != nil { resErr = fmt.Errorf(tr("Invalid size regexp: %s"), err) return @@ -160,13 +159,13 @@ func execSizeRecipe(ctx *types.Context, properties *properties.Map) (textSize in return } - dataSize, err = computeSize(properties.Get(constants.RECIPE_SIZE_REGEXP_DATA), out) + dataSize, err = computeSize(properties.Get("recipe.size.regex.data"), out) if err != nil { resErr = fmt.Errorf(tr("Invalid data size regexp: %s"), err) return } - eepromSize, err = computeSize(properties.Get(constants.RECIPE_SIZE_REGEXP_EEPROM), out) + eepromSize, err = computeSize(properties.Get("recipe.size.regex.eeprom"), out) if err != nil { resErr = fmt.Errorf(tr("Invalid eeprom size regexp: %s"), err) return From d9f6b66861fe48ed50d59cb7941decc0c938374e Mon Sep 17 00:00:00 2001 From: Cristian Maglie Date: Thu, 4 Mar 2021 14:54:15 +0100 Subject: [PATCH 2/9] Added support for advanced-sizers --- legacy/builder/phases/sizer.go | 47 ++++++++++++++++++++++++++++++++-- 1 file changed, 45 insertions(+), 2 deletions(-) diff --git a/legacy/builder/phases/sizer.go b/legacy/builder/phases/sizer.go index 7aa437daf83..3e3285e062e 100644 --- a/legacy/builder/phases/sizer.go +++ b/legacy/builder/phases/sizer.go @@ -16,6 +16,7 @@ package phases import ( + "encoding/json" "fmt" "regexp" "strconv" @@ -41,11 +42,53 @@ func (s *Sizer) Run(ctx *types.Context) error { buildProperties := ctx.BuildProperties - err := checkSize(ctx, buildProperties) + if buildProperties.ContainsKey("recipe.advanced_size.pattern") { + return checkSizeAdvanced(ctx, buildProperties) + } + + return checkSize(ctx, buildProperties) +} + +func checkSizeAdvanced(ctx *types.Context, properties *properties.Map) error { + command, err := builder_utils.PrepareCommandForRecipe(properties, "recipe.advanced_size.pattern", false, ctx.PackageManager.GetEnvVarsForSpawnedProcess()) if err != nil { - return errors.WithStack(err) + return errors.New(tr("Error while determining sketch size: %s", err)) } + out, _, err := utils.ExecCommand(ctx, command, utils.Capture /* stdout */, utils.Show /* stderr */) + if err != nil { + return errors.New(tr("Error while determining sketch size: %s", err)) + } + + type AdvancedSizerResponse struct { + // Output are the messages displayed in console to the user + Output string `json:"output"` + // Severity may be one of "info", "warning" or "error". Warnings and errors will + // likely be printed in red. Errors will stop build/upload. + Severity string `json:"severity"` + // Sections are the sections sizes for machine readable use + Sections []types.ExecutableSectionSize `json:"sections"` + // ErrorMessage is a one line error message like: + // "text section exceeds available space in board" + // it must be set when Severity is "error" + ErrorMessage string `json:"error"` + } + + var resp AdvancedSizerResponse + if err := json.Unmarshal(out, &resp); err != nil { + return errors.New(tr("Error while determining sketch size: %s", err)) + } + + ctx.ExecutableSectionsSize = resp.Sections + switch resp.Severity { + case "error": + ctx.Warn(resp.Output) + return errors.New(resp.ErrorMessage) + case "warning": + ctx.Warn(resp.Output) + default: // or "info" + ctx.Info(resp.Output) + } return nil } From 0fd2b9f4b1bbc6981d27d74280562080094ea1b2 Mon Sep 17 00:00:00 2001 From: Cristian Maglie Date: Tue, 1 Feb 2022 09:35:19 +0100 Subject: [PATCH 3/9] Added documentation --- docs/platform-specification.md | 53 ++++++++++++++++++++++++++++++++++ 1 file changed, 53 insertions(+) diff --git a/docs/platform-specification.md b/docs/platform-specification.md index 7c6050826a7..24bdc5f96d9 100644 --- a/docs/platform-specification.md +++ b/docs/platform-specification.md @@ -309,6 +309,59 @@ Sketch uses 924 bytes (2%) of program storage space. Maximum is 32256 bytes. Global variables use 9 bytes (0%) of dynamic memory, leaving 2039 bytes for local variables. Maximum is 2048 bytes. ``` +#### Recipes to compute binary sketch size for more complex systems (since Arduino CLI >=0.21.0) + +A platform may provide a tool for the specific purpose to analize the binaries and compute the sketch size and memory +usage statistics. This is especially useful for boards with non-trivial memory layouts where the classic reg-exp based +approach is not sufficient. + +The command line to run is specified with the recipe **recipe.advanced_size.pattern**. + +The expected output from the tool is a JSON object with the following format: + +```json +{ + "output": "Your sketch use 2200 bytes of program memory out of 8192 (22%)\nThe static RAM used is 200 bytes (of 2048 max)\n", + "severity": "info", + "sections": [ + { "name": "text", "size": 2200, "maxsize": 8192 }, + { "name": "data", "size": 200, "maxsize": 2048 } + ] +} +``` + +The meaning of the fields is the following: + +- `output`: is a preformatted text that is displayed as-is in console. +- `severity`: indicates the warning level of the output messages, it can be `info`, `warning` or `error`. Warnings and + errors are displayed in red (or in a different color than normal output). Errors will make the build/upload fail. +- `sections`: is an array containing the memory sections and their usage level. Each item representis a memory section. + This array is used to report memory usage in a machine-readable format if requested by the user. + +When the `severity` is set to `error` the build/upload is interrupted and an exception is returned to the calling +process. In this case an extra exception message must be provided through the `error` field, for example: + +```json +{ + "output": "Your sketch use 12200 bytes of program memory out of 8192 (122%)", + "severity": "error", + "error": "Sketch is too big!", + "sections": [ + { "name": "text", "size": 12200, "maxsize": 8192 }, + { "name": "data", "size": 200, "maxsize": 2048 } + ] +} +``` + +This means that the `sections` part is **NOT** used to automatically check if the sketch size exceeds the available +memory: this check is now delegated to the tool that must report a `"severity":"error"` with a meaningful error message. + +If both **recipe.size.pattern** and **recipe.advanced_size.pattern** are present then **recipe.advanced_size.pattern** +will be used. Since the **recipe.advanced_size.pattern** feature is avaiable starting from Arduino CLI>=0.21.0, to +maximize backward compatibility, we recommend to provide both **recipe.size.pattern** and +**recipe.advanced_size.pattern** if possible, so the old versions of the IDE/CLI will continue to work (even with a less +detailed memory usage reports). + #### Recipes to export compiled binary When you do a **Sketch > Export compiled Binary** in the Arduino IDE, the compiled binary is copied from the build From 3712fcd694aaa9bcfca4a855c2e4fe3738bead1d Mon Sep 17 00:00:00 2001 From: Cristian Maglie Date: Tue, 1 Feb 2022 11:14:18 +0100 Subject: [PATCH 4/9] Apply suggestions from code review Co-authored-by: Silvano Cerza <3314350+silvanocerza@users.noreply.github.com> --- docs/platform-specification.md | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/docs/platform-specification.md b/docs/platform-specification.md index 24bdc5f96d9..208efab422e 100644 --- a/docs/platform-specification.md +++ b/docs/platform-specification.md @@ -333,9 +333,9 @@ The expected output from the tool is a JSON object with the following format: The meaning of the fields is the following: - `output`: is a preformatted text that is displayed as-is in console. -- `severity`: indicates the warning level of the output messages, it can be `info`, `warning` or `error`. Warnings and +- `severity`: indicates the warning level of the output messages, it must be `info`, `warning` or `error`. Warnings and errors are displayed in red (or in a different color than normal output). Errors will make the build/upload fail. -- `sections`: is an array containing the memory sections and their usage level. Each item representis a memory section. +- `sections`: is an array containing the memory sections and their usage level. Each item represents a memory section. This array is used to report memory usage in a machine-readable format if requested by the user. When the `severity` is set to `error` the build/upload is interrupted and an exception is returned to the calling From a10b7b6955f68d032e3da237eb96d92143eccf9e Mon Sep 17 00:00:00 2001 From: Cristian Maglie Date: Tue, 1 Feb 2022 11:14:54 +0100 Subject: [PATCH 5/9] Update docs/platform-specification.md Co-authored-by: Silvano Cerza <3314350+silvanocerza@users.noreply.github.com> --- docs/platform-specification.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/platform-specification.md b/docs/platform-specification.md index 208efab422e..d5b06caca91 100644 --- a/docs/platform-specification.md +++ b/docs/platform-specification.md @@ -357,7 +357,7 @@ This means that the `sections` part is **NOT** used to automatically check if th memory: this check is now delegated to the tool that must report a `"severity":"error"` with a meaningful error message. If both **recipe.size.pattern** and **recipe.advanced_size.pattern** are present then **recipe.advanced_size.pattern** -will be used. Since the **recipe.advanced_size.pattern** feature is avaiable starting from Arduino CLI>=0.21.0, to +will be used. Since the **recipe.advanced_size.pattern** feature is available starting from Arduino CLI>=0.21.0, to maximize backward compatibility, we recommend to provide both **recipe.size.pattern** and **recipe.advanced_size.pattern** if possible, so the old versions of the IDE/CLI will continue to work (even with a less detailed memory usage reports). From c01e2e529ff742f625d993b16d0ada0b212cb823 Mon Sep 17 00:00:00 2001 From: Cristian Maglie Date: Tue, 1 Feb 2022 11:57:53 +0100 Subject: [PATCH 6/9] Enforce severity correctness from sizer tool --- legacy/builder/phases/sizer.go | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/legacy/builder/phases/sizer.go b/legacy/builder/phases/sizer.go index 3e3285e062e..d466aca2b06 100644 --- a/legacy/builder/phases/sizer.go +++ b/legacy/builder/phases/sizer.go @@ -86,8 +86,10 @@ func checkSizeAdvanced(ctx *types.Context, properties *properties.Map) error { return errors.New(resp.ErrorMessage) case "warning": ctx.Warn(resp.Output) - default: // or "info" + case "info": ctx.Info(resp.Output) + default: + return fmt.Errorf("invalid '%s' severity from sketch sizer: it must be 'error', 'warning' or 'info'", resp.Severity) } return nil } From 8c39004ebf116bf4770ed36733d0b7efeace58f1 Mon Sep 17 00:00:00 2001 From: Cristian Maglie Date: Tue, 1 Feb 2022 12:32:46 +0100 Subject: [PATCH 7/9] Apply suggestions from code review Co-authored-by: per1234 --- docs/platform-specification.md | 20 ++++++++++++-------- 1 file changed, 12 insertions(+), 8 deletions(-) diff --git a/docs/platform-specification.md b/docs/platform-specification.md index d5b06caca91..efec5167a49 100644 --- a/docs/platform-specification.md +++ b/docs/platform-specification.md @@ -311,9 +311,9 @@ Global variables use 9 bytes (0%) of dynamic memory, leaving 2039 bytes for loca #### Recipes to compute binary sketch size for more complex systems (since Arduino CLI >=0.21.0) -A platform may provide a tool for the specific purpose to analize the binaries and compute the sketch size and memory -usage statistics. This is especially useful for boards with non-trivial memory layouts where the classic reg-exp based -approach is not sufficient. +A platform may provide a tool for the specific purpose to analyze the binaries and compute the sketch size and memory +usage statistics. This is especially useful for boards with non-trivial memory layouts where +[the classic reg-exp based approach](#recipes-to-compute-binary-sketch-size) is not sufficient. The command line to run is specified with the recipe **recipe.advanced_size.pattern**. @@ -321,7 +321,7 @@ The expected output from the tool is a JSON object with the following format: ```json { - "output": "Your sketch use 2200 bytes of program memory out of 8192 (22%)\nThe static RAM used is 200 bytes (of 2048 max)\n", + "output": "Your sketch uses 2200 bytes of program memory out of 8192 (27%)\nThe static RAM used is 200 bytes (of 2048 max)\n", "severity": "info", "sections": [ { "name": "text", "size": 2200, "maxsize": 8192 }, @@ -335,15 +335,19 @@ The meaning of the fields is the following: - `output`: is a preformatted text that is displayed as-is in console. - `severity`: indicates the warning level of the output messages, it must be `info`, `warning` or `error`. Warnings and errors are displayed in red (or in a different color than normal output). Errors will make the build/upload fail. -- `sections`: is an array containing the memory sections and their usage level. Each item represents a memory section. - This array is used to report memory usage in a machine-readable format if requested by the user. +- `sections`: is an array containing the memory sections and their usage level. This array is used to report memory + usage in a machine-readable format if requested by the user. Each item represents a memory section and may contain the + following fields + - `name`: an identifier for the section + - `size`: the sketch size for the section + - `maxsize`: the maximum size for the section When the `severity` is set to `error` the build/upload is interrupted and an exception is returned to the calling process. In this case an extra exception message must be provided through the `error` field, for example: ```json { - "output": "Your sketch use 12200 bytes of program memory out of 8192 (122%)", + "output": "Your sketch uses 12200 bytes of program memory out of 8192 (149%))\nThe static RAM used is 200 bytes (of 2048 max)\n", "severity": "error", "error": "Sketch is too big!", "sections": [ @@ -360,7 +364,7 @@ If both **recipe.size.pattern** and **recipe.advanced_size.pattern** are present will be used. Since the **recipe.advanced_size.pattern** feature is available starting from Arduino CLI>=0.21.0, to maximize backward compatibility, we recommend to provide both **recipe.size.pattern** and **recipe.advanced_size.pattern** if possible, so the old versions of the IDE/CLI will continue to work (even with a less -detailed memory usage reports). +detailed memory usage report). #### Recipes to export compiled binary From b407245db44114a605b3c6d0e16f84279987c79b Mon Sep 17 00:00:00 2001 From: Cristian Maglie Date: Tue, 1 Feb 2022 12:41:58 +0100 Subject: [PATCH 8/9] Use 'max_size' field name in json to match cli output --- docs/platform-specification.md | 10 +++++----- legacy/builder/types/context.go | 6 +++--- 2 files changed, 8 insertions(+), 8 deletions(-) diff --git a/docs/platform-specification.md b/docs/platform-specification.md index efec5167a49..4f6016ffa65 100644 --- a/docs/platform-specification.md +++ b/docs/platform-specification.md @@ -324,8 +324,8 @@ The expected output from the tool is a JSON object with the following format: "output": "Your sketch uses 2200 bytes of program memory out of 8192 (27%)\nThe static RAM used is 200 bytes (of 2048 max)\n", "severity": "info", "sections": [ - { "name": "text", "size": 2200, "maxsize": 8192 }, - { "name": "data", "size": 200, "maxsize": 2048 } + { "name": "text", "size": 2200, "max_size": 8192 }, + { "name": "data", "size": 200, "max_size": 2048 } ] } ``` @@ -340,7 +340,7 @@ The meaning of the fields is the following: following fields - `name`: an identifier for the section - `size`: the sketch size for the section - - `maxsize`: the maximum size for the section + - `max_size`: the maximum size for the section When the `severity` is set to `error` the build/upload is interrupted and an exception is returned to the calling process. In this case an extra exception message must be provided through the `error` field, for example: @@ -351,8 +351,8 @@ process. In this case an extra exception message must be provided through the `e "severity": "error", "error": "Sketch is too big!", "sections": [ - { "name": "text", "size": 12200, "maxsize": 8192 }, - { "name": "data", "size": 200, "maxsize": 2048 } + { "name": "text", "size": 12200, "max_size": 8192 }, + { "name": "data", "size": 200, "max_size": 2048 } ] } ``` diff --git a/legacy/builder/types/context.go b/legacy/builder/types/context.go index 77610afbb8e..84c59c7efa6 100644 --- a/legacy/builder/types/context.go +++ b/legacy/builder/types/context.go @@ -183,9 +183,9 @@ type Context struct { // ExecutableSectionSize represents a section of the executable output file type ExecutableSectionSize struct { - Name string - Size int - MaxSize int + Name string `json:"name"` + Size int `json:"size"` + MaxSize int `json:"max_size"` } // ExecutablesFileSections is an array of ExecutablesFileSection From bed80f952a2f5b712f17fbc86863fe6b09c2dc5a Mon Sep 17 00:00:00 2001 From: Cristian Maglie Date: Tue, 1 Feb 2022 12:53:10 +0100 Subject: [PATCH 9/9] Apply suggestions from code review Co-authored-by: per1234 --- docs/platform-specification.md | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/docs/platform-specification.md b/docs/platform-specification.md index 4f6016ffa65..cc71d6810c0 100644 --- a/docs/platform-specification.md +++ b/docs/platform-specification.md @@ -321,7 +321,7 @@ The expected output from the tool is a JSON object with the following format: ```json { - "output": "Your sketch uses 2200 bytes of program memory out of 8192 (27%)\nThe static RAM used is 200 bytes (of 2048 max)\n", + "output": "Your sketch uses 2200 bytes of program memory out of 8192 (27%)\nThe static RAM used is 200 bytes (of 2048 max)", "severity": "info", "sections": [ { "name": "text", "size": 2200, "max_size": 8192 }, @@ -347,7 +347,7 @@ process. In this case an extra exception message must be provided through the `e ```json { - "output": "Your sketch uses 12200 bytes of program memory out of 8192 (149%))\nThe static RAM used is 200 bytes (of 2048 max)\n", + "output": "Your sketch uses 12200 bytes of program memory out of 8192 (149%))\nThe static RAM used is 200 bytes (of 2048 max)", "severity": "error", "error": "Sketch is too big!", "sections": [