Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[breaking] Add env variable to let tools know the cli version and the gRPC client version #1640

Merged
merged 10 commits into from
Jan 31, 2022
2 changes: 1 addition & 1 deletion arduino/cores/packagemanager/install_uninstall.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
Expand Down
2 changes: 1 addition & 1 deletion arduino/cores/packagemanager/loader_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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")
Expand Down
15 changes: 14 additions & 1 deletion arduino/cores/packagemanager/package_manager.go
Original file line number Diff line number Diff line change
Expand Up @@ -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(),
Expand All @@ -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,
}
}

Expand Down
19 changes: 10 additions & 9 deletions arduino/cores/packagemanager/package_manager_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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")
Expand All @@ -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
Expand Down Expand Up @@ -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")
Expand Down Expand Up @@ -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) {
Expand Down Expand Up @@ -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 {
Expand All @@ -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)
Expand All @@ -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")

{
Expand Down Expand Up @@ -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)
Expand All @@ -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"))
{
Expand Down
2 changes: 1 addition & 1 deletion arduino/discovery/discovery.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
Expand Down
2 changes: 1 addition & 1 deletion arduino/discovery/discovery_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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())
Expand Down
2 changes: 1 addition & 1 deletion arduino/monitor/monitor.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
Expand Down
2 changes: 1 addition & 1 deletion arduino/monitor/monitor_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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())
Expand Down
2 changes: 1 addition & 1 deletion commands/board/list_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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")
Expand Down
12 changes: 10 additions & 2 deletions commands/daemon/daemon.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
)

Expand Down Expand Up @@ -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)
}

Expand Down
2 changes: 1 addition & 1 deletion commands/debug/debug.go
Original file line number Diff line number Diff line change
Expand Up @@ -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}
}
Expand Down
2 changes: 1 addition & 1 deletion commands/debug/debug_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)

Expand Down
7 changes: 6 additions & 1 deletion commands/instances.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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
Expand Down
13 changes: 7 additions & 6 deletions commands/upload/upload.go
Original file line number Diff line number Diff line change
Expand Up @@ -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}
}
}
Expand All @@ -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)
Expand All @@ -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)
}
Expand Down
2 changes: 1 addition & 1 deletion commands/upload/upload_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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")
Expand Down
36 changes: 36 additions & 0 deletions docs/UPGRADING.md
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand Down Expand Up @@ -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:
Expand Down
9 changes: 9 additions & 0 deletions docs/platform-specification.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
Loading