Skip to content

Commit

Permalink
[breaking] gRPC: improved CompileRequest.export_binaries field defini…
Browse files Browse the repository at this point in the history
…tion (#2570)

* [breaking] gRPC: improved CompileRequest.export_binaries field definition

* Disable protogetter check
  • Loading branch information
cmaglie authored Mar 25, 2024
1 parent 32d8833 commit 71c55d7
Show file tree
Hide file tree
Showing 6 changed files with 152 additions and 160 deletions.
3 changes: 2 additions & 1 deletion .golangci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,8 @@ linters:
- tenv
- typecheck
- unconvert
- protogetter
# We must disable this one because there is no support 'optional' protobuf fields yet: https://github.com/arduino/arduino-cli/pull/2570
#- protogetter

linters-settings:
forbidigo:
Expand Down
13 changes: 2 additions & 11 deletions commands/compile/compile.go
Original file line number Diff line number Diff line change
Expand Up @@ -43,18 +43,9 @@ var tr = i18n.Tr

// Compile FIXMEDOC
func Compile(ctx context.Context, req *rpc.CompileRequest, outStream, errStream io.Writer, progressCB rpc.TaskProgressCB) (r *rpc.BuilderResult, e error) {

// There is a binding between the export binaries setting and the CLI flag to explicitly set it,
// since we want this binding to work also for the gRPC interface we must read it here in this
// package instead of the cli/compile one, otherwise we'd lose the binding.
exportBinaries := configuration.Settings.GetBool("sketch.always_export_binaries")
// If we'd just read the binding in any case, even if the request sets the export binaries setting,
// the settings value would always overwrite the request one and it wouldn't have any effect
// setting it for individual requests. To solve this we use a wrapper.BoolValue to handle
// the optionality of this property, otherwise we would have no way of knowing if the property
// was set in the request or it's just the default boolean value.
if reqExportBinaries := req.GetExportBinaries(); reqExportBinaries != nil {
exportBinaries = reqExportBinaries.GetValue()
if e := req.ExportBinaries; e != nil {
exportBinaries = *e
}

pme, release, err := instances.GetPackageManagerExplorer(req.GetInstance())
Expand Down
7 changes: 7 additions & 0 deletions docs/UPGRADING.md
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,13 @@ Here you can find a list of migration guides to handle breaking changes between

## 0.36.0

### The gRPC `cc.arduino.cli.commands.v1.CompileRequest.export_binaries` changed type.

Previously the field `export_binaries` was a `google.protobuf.BoolValue`. We used this type because it expresses this
field's optional nature (that is, it could be `true`, `false`, and `null` if not set).

Now the field is an `optional bool`, since the latest protobuf protocol changes now allows optional fields.

### The gRPC `cc.arduino.cli.commands.v1.UpdateIndexResponse` and `UpdateLibrariesIndexResponse` have changed.

The responses coming from the update index commands:
Expand Down
6 changes: 2 additions & 4 deletions internal/cli/compile/compile.go
Original file line number Diff line number Diff line change
Expand Up @@ -61,6 +61,7 @@ var (
uploadAfterCompile bool // Upload the binary after the compilation.
portArgs arguments.Port // Upload port, e.g.: COM10 or /dev/ttyACM0.
verify bool // Upload, verify uploaded binary after the upload.
exportBinaries bool //
exportDir string // The compiled binary is written to this file
optimizeForDebug bool // Optimize compile output for debug, not for release
programmer arguments.Programmer // Use the specified programmer to upload
Expand Down Expand Up @@ -127,10 +128,7 @@ func NewCommand() *cobra.Command {
programmer.AddToCommand(compileCommand)
compileCommand.Flags().BoolVar(&compilationDatabaseOnly, "only-compilation-database", false, tr("Just produce the compilation database, without actually compiling. All build commands are skipped except pre* hooks."))
compileCommand.Flags().BoolVar(&clean, "clean", false, tr("Optional, cleanup the build folder and do not use any cached build."))
// We must use the following syntax for this flag since it's also bound to settings.
// This must be done because the value is set when the binding is accessed from viper. Accessing from cobra would only
// read the value if the flag is set explicitly by the user.
compileCommand.Flags().BoolP("export-binaries", "e", false, tr("If set built binaries will be exported to the sketch folder."))
compileCommand.Flags().BoolVarP(&exportBinaries, "export-binaries", "e", false, tr("If set built binaries will be exported to the sketch folder."))
compileCommand.Flags().StringVar(&sourceOverrides, "source-override", "", tr("Optional. Path to a .json file that contains a set of replacements of the sketch source code."))
compileCommand.Flag("source-override").Hidden = true
compileCommand.Flags().BoolVar(&skipLibrariesDiscovery, "skip-libraries-discovery", false, "Skip libraries discovery. This flag is provided only for use in language server and other, very specific, use cases. Do not use for normal compiles")
Expand Down
Loading

0 comments on commit 71c55d7

Please sign in to comment.