From 5edfa984c5c21209937740ab5dd577fd8abf1d34 Mon Sep 17 00:00:00 2001 From: Cristian Maglie Date: Tue, 16 Jan 2024 14:18:24 +0100 Subject: [PATCH 1/3] Fixed missing `ARDUINO_USER_AGENT` env var setting. (#2501) * Added integration test * Fixed missing ARDUINO_USER_AGENT env var --- arduino/builder/builder.go | 6 +- commands/compile/compile.go | 1 + .../compile_3/compile_env_var_test.go | 60 +++++++++++++++++++ .../compile_3/testdata/printenv/.gitignore | 1 + .../compile_3/testdata/printenv/main.go | 12 ++++ 5 files changed, 79 insertions(+), 1 deletion(-) create mode 100644 internal/integrationtest/compile_3/compile_env_var_test.go create mode 100644 internal/integrationtest/compile_3/testdata/printenv/.gitignore create mode 100644 internal/integrationtest/compile_3/testdata/printenv/main.go diff --git a/arduino/builder/builder.go b/arduino/builder/builder.go index 51646bdcf48..1be8a93c90c 100644 --- a/arduino/builder/builder.go +++ b/arduino/builder/builder.go @@ -90,6 +90,8 @@ type Builder struct { buildOptions *buildOptions libsDetector *detector.SketchLibrariesDetector + + toolEnv []string } // buildArtifacts contains the result of various build @@ -126,6 +128,7 @@ func NewBuilder( libraryDirs paths.PathList, stdout, stderr io.Writer, verbose bool, warningsLevel string, progresCB rpc.TaskProgressCB, + toolEnv []string, ) (*Builder, error) { buildProperties := properties.NewMap() if boardBuildProperties != nil { @@ -209,6 +212,7 @@ func NewBuilder( buildArtifacts: &buildArtifacts{}, targetPlatform: targetPlatform, actualPlatform: actualPlatform, + toolEnv: toolEnv, libsDetector: detector.NewSketchLibrariesDetector( libsManager, libsResolver, useCachedLibrariesResolution, @@ -493,7 +497,7 @@ func (b *Builder) prepareCommandForRecipe(buildProperties *properties.Map, recip } } - command, err := executils.NewProcess(nil, parts...) + command, err := executils.NewProcess(b.toolEnv, parts...) if err != nil { return nil, err } diff --git a/commands/compile/compile.go b/commands/compile/compile.go index 604a7acb215..c1588d5571d 100644 --- a/commands/compile/compile.go +++ b/commands/compile/compile.go @@ -199,6 +199,7 @@ func Compile(ctx context.Context, req *rpc.CompileRequest, outStream, errStream paths.NewPathList(req.Library...), outStream, errStream, req.GetVerbose(), req.GetWarnings(), progressCB, + pme.GetEnvVarsForSpawnedProcess(), ) if err != nil { if strings.Contains(err.Error(), "invalid build properties") { diff --git a/internal/integrationtest/compile_3/compile_env_var_test.go b/internal/integrationtest/compile_3/compile_env_var_test.go new file mode 100644 index 00000000000..ee786c2d881 --- /dev/null +++ b/internal/integrationtest/compile_3/compile_env_var_test.go @@ -0,0 +1,60 @@ +// This file is part of arduino-cli. +// +// Copyright 2024 ARDUINO SA (http://www.arduino.cc/) +// +// This software is released under the GNU General Public License version 3, +// which covers the main part of arduino-cli. +// The terms of this license can be found at: +// https://www.gnu.org/licenses/gpl-3.0.en.html +// +// You can be released from the requirements of the above licenses by purchasing +// a commercial license. Buying such a license is mandatory if you want to +// modify or otherwise use the software for commercial activities involving the +// Arduino software without disclosing the source code of your own applications. +// To purchase a commercial license, send an email to license@arduino.cc. + +package compile_test + +import ( + "testing" + + "github.com/arduino/arduino-cli/internal/integrationtest" + "github.com/arduino/go-paths-helper" + "github.com/stretchr/testify/require" +) + +func TestCompileEnvVarOnNewProcess(t *testing.T) { + // See: https://github.com/arduino/arduino-cli/issues/2499 + + env, cli := integrationtest.CreateArduinoCLIWithEnvironment(t) + defer env.CleanUp() + + // Run update-index with our test index + _, _, err := cli.Run("core", "install", "arduino:avr@1.8.6") + require.NoError(t, err) + + // Prepare sketchbook and sketch + sketch, err := paths.New("testdata", "bare_minimum").Abs() + require.NoError(t, err) + + // Build "printenv" helper insider testdata/printenv + printenvDir, err := paths.New("testdata", "printenv").Abs() + require.NoError(t, err) + builder, err := paths.NewProcess(nil, "go", "build") + require.NoError(t, err) + builder.SetDir(printenvDir.String()) + require.NoError(t, builder.Run()) + printenv := printenvDir.Join("printenv") + + // Patch avr core to run printenv instead of size + plTxt, err := cli.DataDir().Join("packages", "arduino", "hardware", "avr", "1.8.6", "platform.txt").Append() + require.NoError(t, err) + _, err = plTxt.WriteString("recipe.size.pattern=" + printenv.String() + "\n") + require.NoError(t, err) + require.NoError(t, plTxt.Close()) + + // Run compile and get ENV + _, stderr, err := cli.Run("compile", "-v", "-b", "arduino:avr:uno", sketch.String()) + require.NoError(t, err) + require.Contains(t, string(stderr), "ENV> ARDUINO_USER_AGENT=") +} diff --git a/internal/integrationtest/compile_3/testdata/printenv/.gitignore b/internal/integrationtest/compile_3/testdata/printenv/.gitignore new file mode 100644 index 00000000000..8094186ae54 --- /dev/null +++ b/internal/integrationtest/compile_3/testdata/printenv/.gitignore @@ -0,0 +1 @@ +printenv diff --git a/internal/integrationtest/compile_3/testdata/printenv/main.go b/internal/integrationtest/compile_3/testdata/printenv/main.go new file mode 100644 index 00000000000..0701e4f1594 --- /dev/null +++ b/internal/integrationtest/compile_3/testdata/printenv/main.go @@ -0,0 +1,12 @@ +package main + +import ( + "fmt" + "os" +) + +func main() { + for _, env := range os.Environ() { + fmt.Fprintln(os.Stderr, "ENV>", env) + } +} From 01de174c7e3c08b9c4db121e77a5f7947f968097 Mon Sep 17 00:00:00 2001 From: Cristian Maglie Date: Wed, 24 Jan 2024 12:11:55 +0100 Subject: [PATCH 2/3] Fixed an extremely rare race-condition during compile (#2512) --- .../builder/internal/compilation/database.go | 22 ++++++++++++------- .../internal/compilation/database_test.go | 10 ++++----- 2 files changed, 19 insertions(+), 13 deletions(-) diff --git a/arduino/builder/internal/compilation/database.go b/arduino/builder/internal/compilation/database.go index f6a6b1ab5f0..6a9d8d8c9d0 100644 --- a/arduino/builder/internal/compilation/database.go +++ b/arduino/builder/internal/compilation/database.go @@ -19,6 +19,7 @@ import ( "encoding/json" "fmt" "os" + "sync" "github.com/arduino/arduino-cli/executils" "github.com/arduino/arduino-cli/i18n" @@ -29,8 +30,9 @@ var tr = i18n.Tr // Database keeps track of all the compile commands run by the builder type Database struct { - Contents []Command - File *paths.Path + lock sync.Mutex + contents []Command + file *paths.Path } // Command keeps track of a single run of a compile command @@ -44,8 +46,8 @@ type Command struct { // NewDatabase creates an empty CompilationDatabase func NewDatabase(filename *paths.Path) *Database { return &Database{ - File: filename, - Contents: []Command{}, + file: filename, + contents: []Command{}, } } @@ -56,16 +58,18 @@ func LoadDatabase(file *paths.Path) (*Database, error) { return nil, err } res := NewDatabase(file) - return res, json.Unmarshal(f, &res.Contents) + return res, json.Unmarshal(f, &res.contents) } // SaveToFile save the CompilationDatabase to file as a clangd-compatible compile_commands.json, // see https://clang.llvm.org/docs/JSONCompilationDatabase.html func (db *Database) SaveToFile() { - if jsonContents, err := json.MarshalIndent(db.Contents, "", " "); err != nil { + db.lock.Lock() + defer db.lock.Unlock() + if jsonContents, err := json.MarshalIndent(db.contents, "", " "); err != nil { fmt.Println(tr("Error serializing compilation database: %s", err)) return - } else if err := db.File.WriteFile(jsonContents); err != nil { + } else if err := db.file.WriteFile(jsonContents); err != nil { fmt.Println(tr("Error writing compilation database: %s", err)) } } @@ -89,5 +93,7 @@ func (db *Database) Add(target *paths.Path, command *executils.Process) { File: target.String(), } - db.Contents = append(db.Contents, entry) + db.lock.Lock() + db.contents = append(db.contents, entry) + db.lock.Unlock() } diff --git a/arduino/builder/internal/compilation/database_test.go b/arduino/builder/internal/compilation/database_test.go index 26badfc6cb2..cbe05b6c78e 100644 --- a/arduino/builder/internal/compilation/database_test.go +++ b/arduino/builder/internal/compilation/database_test.go @@ -37,11 +37,11 @@ func TestCompilationDatabase(t *testing.T) { db2, err := LoadDatabase(tmpfile) require.NoError(t, err) require.Equal(t, db, db2) - require.Len(t, db2.Contents, 1) - require.Equal(t, db2.Contents[0].File, "test") - require.Equal(t, db2.Contents[0].Command, "") - require.Equal(t, db2.Contents[0].Arguments, []string{"gcc", "arg1", "arg2"}) + require.Len(t, db2.contents, 1) + require.Equal(t, db2.contents[0].File, "test") + require.Equal(t, db2.contents[0].Command, "") + require.Equal(t, db2.contents[0].Arguments, []string{"gcc", "arg1", "arg2"}) cwd, err := paths.Getwd() require.NoError(t, err) - require.Equal(t, db2.Contents[0].Directory, cwd.String()) + require.Equal(t, db2.contents[0].Directory, cwd.String()) } From 95cfd654fe0dd6b07e903d3132c0faceabfbe9e3 Mon Sep 17 00:00:00 2001 From: Cristian Maglie Date: Mon, 19 Feb 2024 14:15:51 +0100 Subject: [PATCH 3/3] Allow optional programmer in debug (#2540) --- commands/debug/debug_info.go | 17 ++++++++--------- commands/debug/debug_test.go | 6 ------ internal/integrationtest/debug/debug_test.go | 3 --- 3 files changed, 8 insertions(+), 18 deletions(-) diff --git a/commands/debug/debug_info.go b/commands/debug/debug_info.go index 2ed1387280c..1855b8bb716 100644 --- a/commands/debug/debug_info.go +++ b/commands/debug/debug_info.go @@ -169,15 +169,14 @@ func getDebugProperties(req *rpc.GetDebugConfigRequest, pme *packagemanager.Expl } } - if req.GetProgrammer() == "" { - return nil, &arduino.MissingProgrammerError{} - } - if p, ok := platformRelease.Programmers[req.GetProgrammer()]; ok { - toolProperties.Merge(p.Properties) - } else if refP, ok := referencedPlatformRelease.Programmers[req.GetProgrammer()]; ok { - toolProperties.Merge(refP.Properties) - } else { - return nil, &arduino.ProgrammerNotFoundError{Programmer: req.GetProgrammer()} + if req.GetProgrammer() != "" { + if p, ok := platformRelease.Programmers[req.GetProgrammer()]; ok { + toolProperties.Merge(p.Properties) + } else if refP, ok := referencedPlatformRelease.Programmers[req.GetProgrammer()]; ok { + toolProperties.Merge(refP.Properties) + } else { + return nil, &arduino.ProgrammerNotFoundError{Programmer: req.GetProgrammer()} + } } var importPath *paths.Path diff --git a/commands/debug/debug_test.go b/commands/debug/debug_test.go index fbb61626144..6d7a0505e20 100644 --- a/commands/debug/debug_test.go +++ b/commands/debug/debug_test.go @@ -65,12 +65,6 @@ func TestGetCommandLine(t *testing.T) { pme, release := pm.NewExplorer() defer release() - { - // Check programmer required - _, err := getCommandLine(req, pme) - require.Error(t, err) - } - { // Check programmer not found req.Programmer = "not-existent" diff --git a/internal/integrationtest/debug/debug_test.go b/internal/integrationtest/debug/debug_test.go index 12d4c514e78..f8483d0f814 100644 --- a/internal/integrationtest/debug/debug_test.go +++ b/internal/integrationtest/debug/debug_test.go @@ -336,9 +336,6 @@ func testAllDebugInformation(t *testing.T, env *integrationtest.Environment, cli } func testDebugCheck(t *testing.T, env *integrationtest.Environment, cli *integrationtest.ArduinoCLI) { - _, _, err := cli.Run("debug", "check", "-b", "arduino:samd:mkr1000") - require.Error(t, err) - out, _, err := cli.Run("debug", "check", "-b", "arduino:samd:mkr1000", "-P", "atmel_ice") require.NoError(t, err) require.Contains(t, string(out), "The given board/programmer configuration supports debugging.")