From 4256524ebc54341573ba67e1ae1fcd71313b74bd Mon Sep 17 00:00:00 2001 From: Cristian Maglie Date: Mon, 31 Jan 2022 15:27:37 +0100 Subject: [PATCH] [breaking] Add env variable to let tools know the cli version and the gRPC client version (#1640) * Fix lint warning * Add support for global env variable set over arduino-cli * PackageManager now has a user-agent * Propagate 'user-agent' info to tools via environment vars * Allow nil PackageManager in GetEnvVarsForSpawnedProcess() * Added docs * Apply suggestions from code review Co-authored-by: per1234 * Added docs for breaking change in golang API * Fixed behaviour of Process.SetEnvironment * Simplified some appends Co-authored-by: per1234 --- .../cores/packagemanager/install_uninstall.go | 2 +- arduino/cores/packagemanager/loader_test.go | 2 +- .../cores/packagemanager/package_manager.go | 15 +++++++- .../packagemanager/package_manager_test.go | 19 +++++----- arduino/discovery/discovery.go | 2 +- arduino/discovery/discovery_test.go | 2 +- arduino/monitor/monitor.go | 2 +- arduino/monitor/monitor_test.go | 2 +- commands/board/list_test.go | 2 +- commands/daemon/daemon.go | 12 +++++-- commands/debug/debug.go | 2 +- commands/debug/debug_test.go | 2 +- commands/instances.go | 7 +++- commands/upload/upload.go | 13 +++---- commands/upload/upload_test.go | 2 +- docs/UPGRADING.md | 36 +++++++++++++++++++ docs/platform-specification.md | 9 +++++ executils/process.go | 22 +++++++----- executils/process_test.go | 4 +-- legacy/builder/builder_utils/utils.go | 7 ++-- legacy/builder/create_cmake_rule.go | 2 +- legacy/builder/ctags_runner.go | 3 ++ legacy/builder/gcc_preproc_runner.go | 2 +- legacy/builder/hardware_loader.go | 4 ++- legacy/builder/phases/libraries_builder.go | 2 +- legacy/builder/phases/linker.go | 4 +-- legacy/builder/phases/sizer.go | 2 +- legacy/builder/preprocess_sketch.go | 2 ++ legacy/builder/recipe_runner.go | 2 +- 29 files changed, 136 insertions(+), 51 deletions(-) diff --git a/arduino/cores/packagemanager/install_uninstall.go b/arduino/cores/packagemanager/install_uninstall.go index 4a0e11d752d..7f527bc1fb0 100644 --- a/arduino/cores/packagemanager/install_uninstall.go +++ b/arduino/cores/packagemanager/install_uninstall.go @@ -70,7 +70,7 @@ func (pm *PackageManager) RunPostInstallScript(platformRelease *cores.PlatformRe } postInstall := platformRelease.InstallDir.Join(postInstallFilename) if postInstall.Exist() && postInstall.IsNotDir() { - cmd, err := executils.NewProcessFromPath(postInstall) + cmd, err := executils.NewProcessFromPath(pm.GetEnvVarsForSpawnedProcess(), postInstall) if err != nil { return err } diff --git a/arduino/cores/packagemanager/loader_test.go b/arduino/cores/packagemanager/loader_test.go index c1dbd99b65e..ecb9b892140 100644 --- a/arduino/cores/packagemanager/loader_test.go +++ b/arduino/cores/packagemanager/loader_test.go @@ -111,7 +111,7 @@ func TestLoadDiscoveries(t *testing.T) { fakePath := paths.New("fake-path") createTestPackageManager := func() *PackageManager { - packageManager := NewPackageManager(fakePath, fakePath, fakePath, fakePath) + packageManager := NewPackageManager(fakePath, fakePath, fakePath, fakePath, "test") pack := packageManager.Packages.GetOrCreatePackage("arduino") // ble-discovery tool tool := pack.GetOrCreateTool("ble-discovery") diff --git a/arduino/cores/packagemanager/package_manager.go b/arduino/cores/packagemanager/package_manager.go index 5f8bd86edec..a7670828ec7 100644 --- a/arduino/cores/packagemanager/package_manager.go +++ b/arduino/cores/packagemanager/package_manager.go @@ -45,12 +45,13 @@ type PackageManager struct { TempDir *paths.Path CustomGlobalProperties *properties.Map discoveryManager *discoverymanager.DiscoveryManager + userAgent string } var tr = i18n.Tr // NewPackageManager returns a new instance of the PackageManager -func NewPackageManager(indexDir, packagesDir, downloadDir, tempDir *paths.Path) *PackageManager { +func NewPackageManager(indexDir, packagesDir, downloadDir, tempDir *paths.Path, userAgent string) *PackageManager { return &PackageManager{ Log: logrus.StandardLogger(), Packages: cores.NewPackages(), @@ -60,6 +61,18 @@ func NewPackageManager(indexDir, packagesDir, downloadDir, tempDir *paths.Path) TempDir: tempDir, CustomGlobalProperties: properties.NewMap(), discoveryManager: discoverymanager.New(), + userAgent: userAgent, + } +} + +// GetEnvVarsForSpawnedProcess produces a set of environment variables that +// must be sent to all processes spawned from the arduino-cli. +func (pm *PackageManager) GetEnvVarsForSpawnedProcess() []string { + if pm == nil { + return nil + } + return []string{ + "ARDUINO_USER_AGENT=" + pm.userAgent, } } diff --git a/arduino/cores/packagemanager/package_manager_test.go b/arduino/cores/packagemanager/package_manager_test.go index ea13f2ce4af..ec12a6070b9 100644 --- a/arduino/cores/packagemanager/package_manager_test.go +++ b/arduino/cores/packagemanager/package_manager_test.go @@ -37,7 +37,7 @@ var dataDir1 = paths.New("testdata", "data_dir_1") var extraHardware = paths.New("testdata", "extra_hardware") func TestFindBoardWithFQBN(t *testing.T) { - pm := packagemanager.NewPackageManager(customHardware, customHardware, customHardware, customHardware) + pm := packagemanager.NewPackageManager(customHardware, customHardware, customHardware, customHardware, "test") pm.LoadHardwareFromDirectory(customHardware) board, err := pm.FindBoardWithFQBN("arduino:avr:uno") @@ -53,7 +53,7 @@ func TestFindBoardWithFQBN(t *testing.T) { func TestResolveFQBN(t *testing.T) { // Pass nil, since these paths are only used for installing - pm := packagemanager.NewPackageManager(nil, nil, nil, nil) + pm := packagemanager.NewPackageManager(nil, nil, nil, nil, "test") // Hardware from main packages directory pm.LoadHardwareFromDirectory(dataDir1.Join("packages")) // This contains the arduino:avr core @@ -174,7 +174,7 @@ func TestResolveFQBN(t *testing.T) { } func TestBoardOptionsFunctions(t *testing.T) { - pm := packagemanager.NewPackageManager(customHardware, customHardware, customHardware, customHardware) + pm := packagemanager.NewPackageManager(customHardware, customHardware, customHardware, customHardware, "test") pm.LoadHardwareFromDirectory(customHardware) nano, err := pm.FindBoardWithFQBN("arduino:avr:nano") @@ -218,6 +218,7 @@ func TestFindToolsRequiredForBoard(t *testing.T) { configuration.PackagesDir(configuration.Settings), paths.New(configuration.Settings.GetString("directories.Downloads")), dataDir1, + "test", ) loadIndex := func(addr string) { @@ -301,7 +302,7 @@ func TestFindToolsRequiredForBoard(t *testing.T) { } func TestIdentifyBoard(t *testing.T) { - pm := packagemanager.NewPackageManager(customHardware, customHardware, customHardware, customHardware) + pm := packagemanager.NewPackageManager(customHardware, customHardware, customHardware, customHardware, "test") pm.LoadHardwareFromDirectory(customHardware) identify := func(vid, pid string) []*cores.Board { @@ -325,11 +326,11 @@ func TestIdentifyBoard(t *testing.T) { func TestPackageManagerClear(t *testing.T) { // Create a PackageManager and load the harware - packageManager := packagemanager.NewPackageManager(customHardware, customHardware, customHardware, customHardware) + packageManager := packagemanager.NewPackageManager(customHardware, customHardware, customHardware, customHardware, "test") packageManager.LoadHardwareFromDirectory(customHardware) // Creates another PackageManager but don't load the hardware - emptyPackageManager := packagemanager.NewPackageManager(customHardware, customHardware, customHardware, customHardware) + emptyPackageManager := packagemanager.NewPackageManager(customHardware, customHardware, customHardware, customHardware, "test") // Verifies they're not equal require.NotEqual(t, packageManager, emptyPackageManager) @@ -344,7 +345,7 @@ func TestFindToolsRequiredFromPlatformRelease(t *testing.T) { // Create all the necessary data to load discoveries fakePath := paths.New("fake-path") - pm := packagemanager.NewPackageManager(fakePath, fakePath, fakePath, fakePath) + pm := packagemanager.NewPackageManager(fakePath, fakePath, fakePath, fakePath, "test") pack := pm.Packages.GetOrCreatePackage("arduino") { @@ -444,7 +445,7 @@ func TestFindToolsRequiredFromPlatformRelease(t *testing.T) { } func TestFindPlatformReleaseDependencies(t *testing.T) { - pm := packagemanager.NewPackageManager(nil, nil, nil, nil) + pm := packagemanager.NewPackageManager(nil, nil, nil, nil, "test") pm.LoadPackageIndexFromFile(paths.New("testdata", "package_tooltest_index.json")) pl, tools, err := pm.FindPlatformReleaseDependencies(&packagemanager.PlatformReference{Package: "test", PlatformArchitecture: "avr"}) require.NoError(t, err) @@ -455,7 +456,7 @@ func TestFindPlatformReleaseDependencies(t *testing.T) { func TestLegacyPackageConversionToPluggableDiscovery(t *testing.T) { // Pass nil, since these paths are only used for installing - pm := packagemanager.NewPackageManager(nil, nil, nil, nil) + pm := packagemanager.NewPackageManager(nil, nil, nil, nil, "test") // Hardware from main packages directory pm.LoadHardwareFromDirectory(dataDir1.Join("packages")) { diff --git a/arduino/discovery/discovery.go b/arduino/discovery/discovery.go index 648c536733d..4af87a0bddf 100644 --- a/arduino/discovery/discovery.go +++ b/arduino/discovery/discovery.go @@ -235,7 +235,7 @@ func (disc *PluggableDiscovery) sendCommand(command string) error { func (disc *PluggableDiscovery) runProcess() error { logrus.Infof("starting discovery %s process", disc.id) - proc, err := executils.NewProcess(disc.processArgs...) + proc, err := executils.NewProcess(nil, disc.processArgs...) if err != nil { return err } diff --git a/arduino/discovery/discovery_test.go b/arduino/discovery/discovery_test.go index 9acbe62739a..f0e9c03bcc4 100644 --- a/arduino/discovery/discovery_test.go +++ b/arduino/discovery/discovery_test.go @@ -26,7 +26,7 @@ import ( func TestDiscoveryStdioHandling(t *testing.T) { // Build `cat` helper inside testdata/cat - builder, err := executils.NewProcess("go", "build") + builder, err := executils.NewProcess(nil, "go", "build") require.NoError(t, err) builder.SetDir("testdata/cat") require.NoError(t, builder.Run()) diff --git a/arduino/monitor/monitor.go b/arduino/monitor/monitor.go index 5184448eda9..76eeb232dd0 100644 --- a/arduino/monitor/monitor.go +++ b/arduino/monitor/monitor.go @@ -171,7 +171,7 @@ func (mon *PluggableMonitor) sendCommand(command string) error { func (mon *PluggableMonitor) runProcess() error { mon.log.Infof("Starting monitor process") - proc, err := executils.NewProcess(mon.processArgs...) + proc, err := executils.NewProcess(nil, mon.processArgs...) if err != nil { return err } diff --git a/arduino/monitor/monitor_test.go b/arduino/monitor/monitor_test.go index d4535c449c2..0ccae7f7fe0 100644 --- a/arduino/monitor/monitor_test.go +++ b/arduino/monitor/monitor_test.go @@ -33,7 +33,7 @@ func TestDummyMonitor(t *testing.T) { // Build `dummy-monitor` helper inside testdata/dummy-monitor testDataDir, err := paths.New("testdata").Abs() require.NoError(t, err) - builder, err := executils.NewProcess("go", "install", "github.com/arduino/pluggable-monitor-protocol-handler/dummy-monitor@main") + builder, err := executils.NewProcess(nil, "go", "install", "github.com/arduino/pluggable-monitor-protocol-handler/dummy-monitor@main") fmt.Println(testDataDir.String()) env := os.Environ() env = append(env, "GOBIN="+testDataDir.String()) diff --git a/commands/board/list_test.go b/commands/board/list_test.go index 0e99170ab15..c20b467ca0e 100644 --- a/commands/board/list_test.go +++ b/commands/board/list_test.go @@ -119,7 +119,7 @@ func TestBoardIdentifySorting(t *testing.T) { defer paths.TempDir().Join("test").RemoveAll() // We don't really care about the paths in this case - pm := packagemanager.NewPackageManager(dataDir, dataDir, dataDir, dataDir) + pm := packagemanager.NewPackageManager(dataDir, dataDir, dataDir, dataDir, "test") // Create some boards with identical VID:PID combination pack := pm.Packages.GetOrCreatePackage("packager") diff --git a/commands/daemon/daemon.go b/commands/daemon/daemon.go index a86a2cac5a1..6748e48c284 100644 --- a/commands/daemon/daemon.go +++ b/commands/daemon/daemon.go @@ -35,6 +35,7 @@ import ( rpc "github.com/arduino/arduino-cli/rpc/cc/arduino/cli/commands/v1" "github.com/sirupsen/logrus" "google.golang.org/grpc/codes" + "google.golang.org/grpc/metadata" "google.golang.org/grpc/status" ) @@ -226,8 +227,15 @@ func (s *ArduinoCoreServerImpl) Upgrade(req *rpc.UpgradeRequest, stream rpc.Ardu } // Create FIXMEDOC -func (s *ArduinoCoreServerImpl) Create(_ context.Context, req *rpc.CreateRequest) (*rpc.CreateResponse, error) { - res, err := commands.Create(req) +func (s *ArduinoCoreServerImpl) Create(ctx context.Context, req *rpc.CreateRequest) (*rpc.CreateResponse, error) { + var userAgent []string + if md, ok := metadata.FromIncomingContext(ctx); ok { + userAgent = md.Get("user-agent") + } + if len(userAgent) == 0 { + userAgent = []string{"gRPCClientUnknown/0.0.0"} + } + res, err := commands.Create(req, userAgent...) return res, convertErrorToRPCStatus(err) } diff --git a/commands/debug/debug.go b/commands/debug/debug.go index 931668078e7..28731a7869f 100644 --- a/commands/debug/debug.go +++ b/commands/debug/debug.go @@ -62,7 +62,7 @@ func Debug(ctx context.Context, req *dbg.DebugConfigRequest, inStream io.Reader, } entry.Debug("Executing debugger") - cmd, err := executils.NewProcess(commandLine...) + cmd, err := executils.NewProcess(pm.GetEnvVarsForSpawnedProcess(), commandLine...) if err != nil { return nil, &arduino.FailedDebugError{Message: tr("Cannot execute debug tool"), Cause: err} } diff --git a/commands/debug/debug_test.go b/commands/debug/debug_test.go index 61c732ca75d..b495e4338be 100644 --- a/commands/debug/debug_test.go +++ b/commands/debug/debug_test.go @@ -36,7 +36,7 @@ func TestGetCommandLine(t *testing.T) { sketchPath := paths.New("testdata", sketch) require.NoError(t, sketchPath.ToAbs()) - pm := packagemanager.NewPackageManager(nil, nil, nil, nil) + pm := packagemanager.NewPackageManager(nil, nil, nil, nil, "test") pm.LoadHardwareFromDirectory(customHardware) pm.LoadHardwareFromDirectory(dataDir) diff --git a/commands/instances.go b/commands/instances.go index 1e17bd485d5..f26cf07c4fa 100644 --- a/commands/instances.go +++ b/commands/instances.go @@ -106,7 +106,7 @@ func (instance *CoreInstance) installToolIfMissing(tool *cores.ToolRelease, down } // Create a new CoreInstance ready to be initialized, supporting directories are also created. -func Create(req *rpc.CreateRequest) (*rpc.CreateResponse, error) { +func Create(req *rpc.CreateRequest, extraUserAgent ...string) (*rpc.CreateResponse, error) { instance := &CoreInstance{} // Setup downloads directory @@ -129,11 +129,16 @@ func Create(req *rpc.CreateRequest) (*rpc.CreateResponse, error) { } // Create package manager + userAgent := "arduino-cli/" + globals.VersionInfo.VersionString + for _, ua := range extraUserAgent { + userAgent += " " + ua + } instance.PackageManager = packagemanager.NewPackageManager( dataDir, configuration.PackagesDir(configuration.Settings), downloadsDir, dataDir.Join("tmp"), + userAgent, ) // Create library manager and add libraries directories diff --git a/commands/upload/upload.go b/commands/upload/upload.go index fd4af21a358..f9a83d87afb 100644 --- a/commands/upload/upload.go +++ b/commands/upload/upload.go @@ -531,19 +531,20 @@ func runProgramAction(pm *packagemanager.PackageManager, } // Run recipes for upload + toolEnv := pm.GetEnvVarsForSpawnedProcess() if burnBootloader { - if err := runTool("erase.pattern", uploadProperties, outStream, errStream, verbose, dryRun); err != nil { + if err := runTool("erase.pattern", uploadProperties, outStream, errStream, verbose, dryRun, toolEnv); err != nil { return &arduino.FailedUploadError{Message: tr("Failed chip erase"), Cause: err} } - if err := runTool("bootloader.pattern", uploadProperties, outStream, errStream, verbose, dryRun); err != nil { + if err := runTool("bootloader.pattern", uploadProperties, outStream, errStream, verbose, dryRun, toolEnv); err != nil { return &arduino.FailedUploadError{Message: tr("Failed to burn bootloader"), Cause: err} } } else if programmer != nil { - if err := runTool("program.pattern", uploadProperties, outStream, errStream, verbose, dryRun); err != nil { + if err := runTool("program.pattern", uploadProperties, outStream, errStream, verbose, dryRun, toolEnv); err != nil { return &arduino.FailedUploadError{Message: tr("Failed programming"), Cause: err} } } else { - if err := runTool("upload.pattern", uploadProperties, outStream, errStream, verbose, dryRun); err != nil { + if err := runTool("upload.pattern", uploadProperties, outStream, errStream, verbose, dryRun, toolEnv); err != nil { return &arduino.FailedUploadError{Message: tr("Failed uploading"), Cause: err} } } @@ -552,7 +553,7 @@ func runProgramAction(pm *packagemanager.PackageManager, return nil } -func runTool(recipeID string, props *properties.Map, outStream, errStream io.Writer, verbose bool, dryRun bool) error { +func runTool(recipeID string, props *properties.Map, outStream, errStream io.Writer, verbose bool, dryRun bool, toolEnv []string) error { recipe, ok := props.GetOk(recipeID) if !ok { return fmt.Errorf(tr("recipe not found '%s'"), recipeID) @@ -577,7 +578,7 @@ func runTool(recipeID string, props *properties.Map, outStream, errStream io.Wri if dryRun { return nil } - cmd, err := executils.NewProcess(cmdArgs...) + cmd, err := executils.NewProcess(toolEnv, cmdArgs...) if err != nil { return fmt.Errorf(tr("cannot execute upload tool: %s"), err) } diff --git a/commands/upload/upload_test.go b/commands/upload/upload_test.go index a5bb7d16c1b..1debccb2b8d 100644 --- a/commands/upload/upload_test.go +++ b/commands/upload/upload_test.go @@ -127,7 +127,7 @@ func TestDetermineBuildPathAndSketchName(t *testing.T) { } func TestUploadPropertiesComposition(t *testing.T) { - pm := packagemanager.NewPackageManager(nil, nil, nil, nil) + pm := packagemanager.NewPackageManager(nil, nil, nil, nil, "test") errs := pm.LoadHardwareFromDirectory(paths.New("testdata", "hardware")) require.Len(t, errs, 0) buildPath1 := paths.New("testdata", "build_path_1") diff --git a/docs/UPGRADING.md b/docs/UPGRADING.md index c61370cc08b..730010d9a3d 100644 --- a/docs/UPGRADING.md +++ b/docs/UPGRADING.md @@ -4,6 +4,27 @@ Here you can find a list of migration guides to handle breaking changes between ## Unreleased +### `packagemanager.NewPackageManager` function change + +A new argument `userAgent` has been added to `packagemanager.NewPackageManager`, the new function signature is: + +```go +func NewPackageManager(indexDir, packagesDir, downloadDir, tempDir *paths.Path, userAgent string) *PackageManager { +``` + +The userAgent string must be in the format `"ProgramName/Version"`, for example `"arduino-cli/0.20.1"`. + +### `commands.Create` function change + +A new argument `extraUserAgent` has been added to `commands.Create`, the new function signature is: + +```go +func Create(req *rpc.CreateRequest, extraUserAgent ...string) (*rpc.CreateResponse, error) { +``` + +`extraUserAgent` is an array of strings, so multiple user agent may be provided. Each user agent must be in the format +`"ProgramName/Version"`, for example `"arduino-cli/0.20.1"`. + ### `commands.Compile` function change A new argument `progressCB` has been added to `commands.Compile`, the new function signature is: @@ -32,6 +53,21 @@ The `parseArch` parameter was removed since it was unused and was always true. T always parsed by the function. Furthermore the function now should also correctly interpret `packager:arch` spelled with the wrong casing. +### `github.com/arduino/arduino-cli/executils.NewProcess` and `executils.NewProcessFromPath` function change + +A new argument `extraEnv` has been added to `executils.NewProcess` and `executils.NewProcessFromPath`, the new function +signature is: + +```go +func NewProcess(extraEnv []string, args ...string) (*Process, error) { +``` + +```go +func NewProcessFromPath(extraEnv []string, executable *paths.Path, args ...string) (*Process, error) { +``` + +The `extraEnv` params allow to pass environment variables (in addition to the default ones) to the spawned process. + ### `github.com/arduino/arduino-cli/i18n.Init(...)` now requires an empty string to be passed for autodetection of locale For automated detection of locale, change the call from: diff --git a/docs/platform-specification.md b/docs/platform-specification.md index 56a3878e77e..7c6050826a7 100644 --- a/docs/platform-specification.md +++ b/docs/platform-specification.md @@ -702,6 +702,15 @@ The tool configuration properties are available globally without the prefix. For property can be used as **{cmd.path}** inside the recipe, and the same happens for all the other avrdude configuration variables. +### Environment variables + +All the tools launched to compile or upload a sketch will have the following environment variable set: + +`ARDUINO_USER_AGENT`: contains the name and version of the client used by the user in +[HTTP user-agent format](https://en.wikipedia.org/wiki/User_agent), for example `"arduino-cli/0.21.0"`. It may also +contain multiple space-delimited entries like `"arduino-cli/0.21.0 ArduinoIDE/2.0.0-rc1"` if this information is +available. + ### Pluggable discovery Discovery tools are a special kind of tool used to find supported boards. A platform must declare one or more Pluggable diff --git a/executils/process.go b/executils/process.go index 620f66a5d8d..a1a6fbabc89 100644 --- a/executils/process.go +++ b/executils/process.go @@ -30,16 +30,18 @@ type Process struct { cmd *exec.Cmd } -// NewProcess creates a command with the provided command line arguments. -// The first argument is the path to the executable, the remainder are the +// NewProcess creates a command with the provided command line arguments +// and environment variables (that will be added to the parent os.Environ). +// The argument args[0] is the path to the executable, the remainder are the // arguments to the command. -func NewProcess(args ...string) (*Process, error) { - if args == nil || len(args) == 0 { +func NewProcess(extraEnv []string, args ...string) (*Process, error) { + if len(args) == 0 { return nil, errors.New(tr("no executable specified")) } p := &Process{ cmd: exec.Command(args[0], args[1:]...), } + p.cmd.Env = append(os.Environ(), extraEnv...) TellCommandNotToSpawnShell(p.cmd) // This is required because some tools detects if the program is running @@ -49,12 +51,13 @@ func NewProcess(args ...string) (*Process, error) { return p, nil } -// NewProcessFromPath creates a command from the provided executable path and -// command line arguments. -func NewProcessFromPath(executable *paths.Path, args ...string) (*Process, error) { +// NewProcessFromPath creates a command from the provided executable path, +// additional environment vars (in addition to the system default ones) +// and command line arguments. +func NewProcessFromPath(extraEnv []string, executable *paths.Path, args ...string) (*Process, error) { processArgs := []string{executable.String()} processArgs = append(processArgs, args...) - return NewProcess(processArgs...) + return NewProcess(extraEnv, processArgs...) } // RedirectStdoutTo will redirect the process' stdout to the specified @@ -139,8 +142,9 @@ func (p *Process) Run() error { } // SetEnvironment set the enviroment for the running process. Each entry is of the form "key=value". +// System default environments will be wiped out. func (p *Process) SetEnvironment(values []string) { - p.cmd.Env = values + p.cmd.Env = append([]string{}, values...) } // RunWithinContext starts the specified command and waits for it to complete. If the given context diff --git a/executils/process_test.go b/executils/process_test.go index 966438f4de0..4e50f51993f 100644 --- a/executils/process_test.go +++ b/executils/process_test.go @@ -25,13 +25,13 @@ import ( func TestProcessWithinContext(t *testing.T) { // Build `delay` helper inside testdata/delay - builder, err := NewProcess("go", "build") + builder, err := NewProcess(nil, "go", "build") require.NoError(t, err) builder.SetDir("testdata/delay") require.NoError(t, builder.Run()) // Run delay and test if the process is terminated correctly due to context - process, err := NewProcess("testdata/delay/delay") + process, err := NewProcess(nil, "testdata/delay/delay") require.NoError(t, err) start := time.Now() ctx, cancel := context.WithTimeout(context.Background(), 250*time.Millisecond) diff --git a/legacy/builder/builder_utils/utils.go b/legacy/builder/builder_utils/utils.go index 3f98139956b..05634f796da 100644 --- a/legacy/builder/builder_utils/utils.go +++ b/legacy/builder/builder_utils/utils.go @@ -237,7 +237,7 @@ func compileFileWithRecipe(ctx *types.Context, sourcePath *paths.Path, source *p if err != nil { return nil, errors.WithStack(err) } - command, err := PrepareCommandForRecipe(properties, recipe, false) + command, err := PrepareCommandForRecipe(properties, recipe, false, ctx.PackageManager.GetEnvVarsForSpawnedProcess()) if err != nil { return nil, errors.WithStack(err) } @@ -469,7 +469,7 @@ func ArchiveCompiledFiles(ctx *types.Context, buildPath *paths.Path, archiveFile properties.SetPath(constants.BUILD_PROPERTIES_ARCHIVE_FILE_PATH, archiveFilePath) properties.SetPath(constants.BUILD_PROPERTIES_OBJECT_FILE, objectFile) - command, err := PrepareCommandForRecipe(properties, constants.RECIPE_AR_PATTERN, false) + command, err := PrepareCommandForRecipe(properties, constants.RECIPE_AR_PATTERN, false, ctx.PackageManager.GetEnvVarsForSpawnedProcess()) if err != nil { return nil, errors.WithStack(err) } @@ -485,7 +485,7 @@ func ArchiveCompiledFiles(ctx *types.Context, buildPath *paths.Path, archiveFile const COMMANDLINE_LIMIT = 30000 -func PrepareCommandForRecipe(buildProperties *properties.Map, recipe string, removeUnsetProperties bool) (*exec.Cmd, error) { +func PrepareCommandForRecipe(buildProperties *properties.Map, recipe string, removeUnsetProperties bool, toolEnv []string) (*exec.Cmd, error) { pattern := buildProperties.Get(recipe) if pattern == "" { return nil, errors.Errorf(tr("%[1]s pattern is missing"), recipe) @@ -501,6 +501,7 @@ func PrepareCommandForRecipe(buildProperties *properties.Map, recipe string, rem return nil, errors.WithStack(err) } command := exec.Command(parts[0], parts[1:]...) + command.Env = append(os.Environ(), toolEnv...) // if the overall commandline is too long for the platform // try reducing the length by making the filenames relative diff --git a/legacy/builder/create_cmake_rule.go b/legacy/builder/create_cmake_rule.go index 9e8f499605c..87b1be674d1 100644 --- a/legacy/builder/create_cmake_rule.go +++ b/legacy/builder/create_cmake_rule.go @@ -237,7 +237,7 @@ func canExportCmakeProject(ctx *types.Context) bool { } func extractCompileFlags(ctx *types.Context, recipe string, defines, dynamicLibs, linkerflags, linkDirectories *[]string) { - command, _ := builder_utils.PrepareCommandForRecipe(ctx.BuildProperties, recipe, true) + command, _ := builder_utils.PrepareCommandForRecipe(ctx.BuildProperties, recipe, true, ctx.PackageManager.GetEnvVarsForSpawnedProcess()) for _, arg := range command.Args { if strings.HasPrefix(arg, "-D") { diff --git a/legacy/builder/ctags_runner.go b/legacy/builder/ctags_runner.go index 299ffb45962..22364f876d3 100644 --- a/legacy/builder/ctags_runner.go +++ b/legacy/builder/ctags_runner.go @@ -16,6 +16,7 @@ package builder import ( + "os" "os/exec" "github.com/arduino/arduino-cli/legacy/builder/constants" @@ -47,6 +48,8 @@ func (s *CTagsRunner) Run(ctx *types.Context) error { return errors.WithStack(err) } command := exec.Command(parts[0], parts[1:]...) + command.Env = append(os.Environ(), ctx.PackageManager.GetEnvVarsForSpawnedProcess()...) + sourceBytes, _, err := utils.ExecCommand(ctx, command, utils.Capture /* stdout */, utils.Ignore /* stderr */) if err != nil { return errors.WithStack(err) diff --git a/legacy/builder/gcc_preproc_runner.go b/legacy/builder/gcc_preproc_runner.go index a1ddab96cd2..45507d34e17 100644 --- a/legacy/builder/gcc_preproc_runner.go +++ b/legacy/builder/gcc_preproc_runner.go @@ -69,7 +69,7 @@ func prepareGCCPreprocRecipeProperties(ctx *types.Context, sourceFilePath *paths properties.Set(constants.RECIPE_PREPROC_MACROS, GeneratePreprocPatternFromCompile(properties.Get(constants.RECIPE_CPP_PATTERN))) } - cmd, err := builder_utils.PrepareCommandForRecipe(properties, constants.RECIPE_PREPROC_MACROS, true) + cmd, err := builder_utils.PrepareCommandForRecipe(properties, constants.RECIPE_PREPROC_MACROS, true, ctx.PackageManager.GetEnvVarsForSpawnedProcess()) if err != nil { return nil, errors.WithStack(err) } diff --git a/legacy/builder/hardware_loader.go b/legacy/builder/hardware_loader.go index 70ce36a9524..61a0aca530f 100644 --- a/legacy/builder/hardware_loader.go +++ b/legacy/builder/hardware_loader.go @@ -24,7 +24,9 @@ type HardwareLoader struct{} func (s *HardwareLoader) Run(ctx *types.Context) error { if ctx.PackageManager == nil { - pm := packagemanager.NewPackageManager(nil, nil, nil, nil) + // This should happen only on legacy arduino-builder. + // Hopefully this piece will be removed once the legacy package will be cleanedup. + pm := packagemanager.NewPackageManager(nil, nil, nil, nil, "arduino-builder") if errs := pm.LoadHardwareFromDirectories(ctx.HardwareDirs); len(errs) > 0 { // With the refactoring of the initialization step of the CLI we changed how // errors are returned when loading platforms and libraries, that meant returning a list of diff --git a/legacy/builder/phases/libraries_builder.go b/legacy/builder/phases/libraries_builder.go index b19363db01f..84ce972c4de 100644 --- a/legacy/builder/phases/libraries_builder.go +++ b/legacy/builder/phases/libraries_builder.go @@ -65,7 +65,7 @@ func findExpectedPrecompiledLibFolder(ctx *types.Context, library *libraries.Lib // Add fpu specifications if they exist // To do so, resolve recipe.cpp.o.pattern, // search for -mfpu=xxx -mfloat-abi=yyy and add to a subfolder - command, _ := builder_utils.PrepareCommandForRecipe(ctx.BuildProperties, constants.RECIPE_CPP_PATTERN, true) + command, _ := builder_utils.PrepareCommandForRecipe(ctx.BuildProperties, constants.RECIPE_CPP_PATTERN, true, ctx.PackageManager.GetEnvVarsForSpawnedProcess()) fpuSpecs := "" for _, el := range strings.Split(command.String(), " ") { if strings.Contains(el, FPU_CFLAG) { diff --git a/legacy/builder/phases/linker.go b/legacy/builder/phases/linker.go index a78762ebac0..0f2f76d336e 100644 --- a/legacy/builder/phases/linker.go +++ b/legacy/builder/phases/linker.go @@ -92,7 +92,7 @@ func link(ctx *types.Context, objectFiles paths.PathList, coreDotARelPath *paths properties.SetPath("archive_file_path", archive) properties.SetPath("object_file", object) - command, err := builder_utils.PrepareCommandForRecipe(properties, constants.RECIPE_AR_PATTERN, false) + command, err := builder_utils.PrepareCommandForRecipe(properties, constants.RECIPE_AR_PATTERN, false, ctx.PackageManager.GetEnvVarsForSpawnedProcess()) if err != nil { return errors.WithStack(err) } @@ -113,7 +113,7 @@ func link(ctx *types.Context, objectFiles paths.PathList, coreDotARelPath *paths properties.Set(constants.BUILD_PROPERTIES_ARCHIVE_FILE_PATH, coreArchiveFilePath.String()) properties.Set("object_files", objectFileList) - command, err := builder_utils.PrepareCommandForRecipe(properties, constants.RECIPE_C_COMBINE_PATTERN, false) + command, err := builder_utils.PrepareCommandForRecipe(properties, constants.RECIPE_C_COMBINE_PATTERN, false, ctx.PackageManager.GetEnvVarsForSpawnedProcess()) if err != nil { return err } diff --git a/legacy/builder/phases/sizer.go b/legacy/builder/phases/sizer.go index a23a8312ca2..de73f7999df 100644 --- a/legacy/builder/phases/sizer.go +++ b/legacy/builder/phases/sizer.go @@ -135,7 +135,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) + command, err := builder_utils.PrepareCommandForRecipe(properties, constants.RECIPE_SIZE_PATTERN, false, ctx.PackageManager.GetEnvVarsForSpawnedProcess()) if err != nil { resErr = fmt.Errorf(tr("Error while determining sketch size: %s"), err) return diff --git a/legacy/builder/preprocess_sketch.go b/legacy/builder/preprocess_sketch.go index a252ae4900b..a0403f08273 100644 --- a/legacy/builder/preprocess_sketch.go +++ b/legacy/builder/preprocess_sketch.go @@ -17,6 +17,7 @@ package builder import ( "fmt" + "os" "os/exec" "path/filepath" "runtime" @@ -109,6 +110,7 @@ func (s *ArduinoPreprocessorRunner) Run(ctx *types.Context) error { return errors.WithStack(err) } command := exec.Command(parts[0], parts[1:]...) + command.Env = append(os.Environ(), ctx.PackageManager.GetEnvVarsForSpawnedProcess()...) if runtime.GOOS == "windows" { // chdir in the uppermost directory to avoid UTF-8 bug in clang (https://github.com/arduino/arduino-preprocessor/issues/2) diff --git a/legacy/builder/recipe_runner.go b/legacy/builder/recipe_runner.go index 6dfb914d7b9..a6b552b2c6d 100644 --- a/legacy/builder/recipe_runner.go +++ b/legacy/builder/recipe_runner.go @@ -44,7 +44,7 @@ func (s *RecipeByPrefixSuffixRunner) Run(ctx *types.Context) error { for _, recipe := range recipes { logrus.Debugf(fmt.Sprintf("Running recipe: %s", recipe)) - command, err := builder_utils.PrepareCommandForRecipe(properties, recipe, false) + command, err := builder_utils.PrepareCommandForRecipe(properties, recipe, false, ctx.PackageManager.GetEnvVarsForSpawnedProcess()) if err != nil { return errors.WithStack(err) }