Skip to content

Commit 4256524

Browse files
cmaglieper1234
cmaglie
and
per1234
authored
[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 <[email protected]> * Added docs for breaking change in golang API * Fixed behaviour of Process.SetEnvironment * Simplified some appends Co-authored-by: per1234 <[email protected]>
1 parent 085a31b commit 4256524

29 files changed

+136
-51
lines changed

Diff for: ‎arduino/cores/packagemanager/install_uninstall.go

+1-1
Original file line numberDiff line numberDiff line change
@@ -70,7 +70,7 @@ func (pm *PackageManager) RunPostInstallScript(platformRelease *cores.PlatformRe
7070
}
7171
postInstall := platformRelease.InstallDir.Join(postInstallFilename)
7272
if postInstall.Exist() && postInstall.IsNotDir() {
73-
cmd, err := executils.NewProcessFromPath(postInstall)
73+
cmd, err := executils.NewProcessFromPath(pm.GetEnvVarsForSpawnedProcess(), postInstall)
7474
if err != nil {
7575
return err
7676
}

Diff for: ‎arduino/cores/packagemanager/loader_test.go

+1-1
Original file line numberDiff line numberDiff line change
@@ -111,7 +111,7 @@ func TestLoadDiscoveries(t *testing.T) {
111111
fakePath := paths.New("fake-path")
112112

113113
createTestPackageManager := func() *PackageManager {
114-
packageManager := NewPackageManager(fakePath, fakePath, fakePath, fakePath)
114+
packageManager := NewPackageManager(fakePath, fakePath, fakePath, fakePath, "test")
115115
pack := packageManager.Packages.GetOrCreatePackage("arduino")
116116
// ble-discovery tool
117117
tool := pack.GetOrCreateTool("ble-discovery")

Diff for: ‎arduino/cores/packagemanager/package_manager.go

+14-1
Original file line numberDiff line numberDiff line change
@@ -45,12 +45,13 @@ type PackageManager struct {
4545
TempDir *paths.Path
4646
CustomGlobalProperties *properties.Map
4747
discoveryManager *discoverymanager.DiscoveryManager
48+
userAgent string
4849
}
4950

5051
var tr = i18n.Tr
5152

5253
// NewPackageManager returns a new instance of the PackageManager
53-
func NewPackageManager(indexDir, packagesDir, downloadDir, tempDir *paths.Path) *PackageManager {
54+
func NewPackageManager(indexDir, packagesDir, downloadDir, tempDir *paths.Path, userAgent string) *PackageManager {
5455
return &PackageManager{
5556
Log: logrus.StandardLogger(),
5657
Packages: cores.NewPackages(),
@@ -60,6 +61,18 @@ func NewPackageManager(indexDir, packagesDir, downloadDir, tempDir *paths.Path)
6061
TempDir: tempDir,
6162
CustomGlobalProperties: properties.NewMap(),
6263
discoveryManager: discoverymanager.New(),
64+
userAgent: userAgent,
65+
}
66+
}
67+
68+
// GetEnvVarsForSpawnedProcess produces a set of environment variables that
69+
// must be sent to all processes spawned from the arduino-cli.
70+
func (pm *PackageManager) GetEnvVarsForSpawnedProcess() []string {
71+
if pm == nil {
72+
return nil
73+
}
74+
return []string{
75+
"ARDUINO_USER_AGENT=" + pm.userAgent,
6376
}
6477
}
6578

Diff for: ‎arduino/cores/packagemanager/package_manager_test.go

+10-9
Original file line numberDiff line numberDiff line change
@@ -37,7 +37,7 @@ var dataDir1 = paths.New("testdata", "data_dir_1")
3737
var extraHardware = paths.New("testdata", "extra_hardware")
3838

3939
func TestFindBoardWithFQBN(t *testing.T) {
40-
pm := packagemanager.NewPackageManager(customHardware, customHardware, customHardware, customHardware)
40+
pm := packagemanager.NewPackageManager(customHardware, customHardware, customHardware, customHardware, "test")
4141
pm.LoadHardwareFromDirectory(customHardware)
4242

4343
board, err := pm.FindBoardWithFQBN("arduino:avr:uno")
@@ -53,7 +53,7 @@ func TestFindBoardWithFQBN(t *testing.T) {
5353

5454
func TestResolveFQBN(t *testing.T) {
5555
// Pass nil, since these paths are only used for installing
56-
pm := packagemanager.NewPackageManager(nil, nil, nil, nil)
56+
pm := packagemanager.NewPackageManager(nil, nil, nil, nil, "test")
5757
// Hardware from main packages directory
5858
pm.LoadHardwareFromDirectory(dataDir1.Join("packages"))
5959
// This contains the arduino:avr core
@@ -174,7 +174,7 @@ func TestResolveFQBN(t *testing.T) {
174174
}
175175

176176
func TestBoardOptionsFunctions(t *testing.T) {
177-
pm := packagemanager.NewPackageManager(customHardware, customHardware, customHardware, customHardware)
177+
pm := packagemanager.NewPackageManager(customHardware, customHardware, customHardware, customHardware, "test")
178178
pm.LoadHardwareFromDirectory(customHardware)
179179

180180
nano, err := pm.FindBoardWithFQBN("arduino:avr:nano")
@@ -218,6 +218,7 @@ func TestFindToolsRequiredForBoard(t *testing.T) {
218218
configuration.PackagesDir(configuration.Settings),
219219
paths.New(configuration.Settings.GetString("directories.Downloads")),
220220
dataDir1,
221+
"test",
221222
)
222223

223224
loadIndex := func(addr string) {
@@ -301,7 +302,7 @@ func TestFindToolsRequiredForBoard(t *testing.T) {
301302
}
302303

303304
func TestIdentifyBoard(t *testing.T) {
304-
pm := packagemanager.NewPackageManager(customHardware, customHardware, customHardware, customHardware)
305+
pm := packagemanager.NewPackageManager(customHardware, customHardware, customHardware, customHardware, "test")
305306
pm.LoadHardwareFromDirectory(customHardware)
306307

307308
identify := func(vid, pid string) []*cores.Board {
@@ -325,11 +326,11 @@ func TestIdentifyBoard(t *testing.T) {
325326

326327
func TestPackageManagerClear(t *testing.T) {
327328
// Create a PackageManager and load the harware
328-
packageManager := packagemanager.NewPackageManager(customHardware, customHardware, customHardware, customHardware)
329+
packageManager := packagemanager.NewPackageManager(customHardware, customHardware, customHardware, customHardware, "test")
329330
packageManager.LoadHardwareFromDirectory(customHardware)
330331

331332
// Creates another PackageManager but don't load the hardware
332-
emptyPackageManager := packagemanager.NewPackageManager(customHardware, customHardware, customHardware, customHardware)
333+
emptyPackageManager := packagemanager.NewPackageManager(customHardware, customHardware, customHardware, customHardware, "test")
333334

334335
// Verifies they're not equal
335336
require.NotEqual(t, packageManager, emptyPackageManager)
@@ -344,7 +345,7 @@ func TestFindToolsRequiredFromPlatformRelease(t *testing.T) {
344345
// Create all the necessary data to load discoveries
345346
fakePath := paths.New("fake-path")
346347

347-
pm := packagemanager.NewPackageManager(fakePath, fakePath, fakePath, fakePath)
348+
pm := packagemanager.NewPackageManager(fakePath, fakePath, fakePath, fakePath, "test")
348349
pack := pm.Packages.GetOrCreatePackage("arduino")
349350

350351
{
@@ -444,7 +445,7 @@ func TestFindToolsRequiredFromPlatformRelease(t *testing.T) {
444445
}
445446

446447
func TestFindPlatformReleaseDependencies(t *testing.T) {
447-
pm := packagemanager.NewPackageManager(nil, nil, nil, nil)
448+
pm := packagemanager.NewPackageManager(nil, nil, nil, nil, "test")
448449
pm.LoadPackageIndexFromFile(paths.New("testdata", "package_tooltest_index.json"))
449450
pl, tools, err := pm.FindPlatformReleaseDependencies(&packagemanager.PlatformReference{Package: "test", PlatformArchitecture: "avr"})
450451
require.NoError(t, err)
@@ -455,7 +456,7 @@ func TestFindPlatformReleaseDependencies(t *testing.T) {
455456

456457
func TestLegacyPackageConversionToPluggableDiscovery(t *testing.T) {
457458
// Pass nil, since these paths are only used for installing
458-
pm := packagemanager.NewPackageManager(nil, nil, nil, nil)
459+
pm := packagemanager.NewPackageManager(nil, nil, nil, nil, "test")
459460
// Hardware from main packages directory
460461
pm.LoadHardwareFromDirectory(dataDir1.Join("packages"))
461462
{

Diff for: ‎arduino/discovery/discovery.go

+1-1
Original file line numberDiff line numberDiff line change
@@ -235,7 +235,7 @@ func (disc *PluggableDiscovery) sendCommand(command string) error {
235235

236236
func (disc *PluggableDiscovery) runProcess() error {
237237
logrus.Infof("starting discovery %s process", disc.id)
238-
proc, err := executils.NewProcess(disc.processArgs...)
238+
proc, err := executils.NewProcess(nil, disc.processArgs...)
239239
if err != nil {
240240
return err
241241
}

Diff for: ‎arduino/discovery/discovery_test.go

+1-1
Original file line numberDiff line numberDiff line change
@@ -26,7 +26,7 @@ import (
2626

2727
func TestDiscoveryStdioHandling(t *testing.T) {
2828
// Build `cat` helper inside testdata/cat
29-
builder, err := executils.NewProcess("go", "build")
29+
builder, err := executils.NewProcess(nil, "go", "build")
3030
require.NoError(t, err)
3131
builder.SetDir("testdata/cat")
3232
require.NoError(t, builder.Run())

Diff for: ‎arduino/monitor/monitor.go

+1-1
Original file line numberDiff line numberDiff line change
@@ -171,7 +171,7 @@ func (mon *PluggableMonitor) sendCommand(command string) error {
171171

172172
func (mon *PluggableMonitor) runProcess() error {
173173
mon.log.Infof("Starting monitor process")
174-
proc, err := executils.NewProcess(mon.processArgs...)
174+
proc, err := executils.NewProcess(nil, mon.processArgs...)
175175
if err != nil {
176176
return err
177177
}

Diff for: ‎arduino/monitor/monitor_test.go

+1-1
Original file line numberDiff line numberDiff line change
@@ -33,7 +33,7 @@ func TestDummyMonitor(t *testing.T) {
3333
// Build `dummy-monitor` helper inside testdata/dummy-monitor
3434
testDataDir, err := paths.New("testdata").Abs()
3535
require.NoError(t, err)
36-
builder, err := executils.NewProcess("go", "install", "github.com/arduino/pluggable-monitor-protocol-handler/dummy-monitor@main")
36+
builder, err := executils.NewProcess(nil, "go", "install", "github.com/arduino/pluggable-monitor-protocol-handler/dummy-monitor@main")
3737
fmt.Println(testDataDir.String())
3838
env := os.Environ()
3939
env = append(env, "GOBIN="+testDataDir.String())

Diff for: ‎commands/board/list_test.go

+1-1
Original file line numberDiff line numberDiff line change
@@ -119,7 +119,7 @@ func TestBoardIdentifySorting(t *testing.T) {
119119
defer paths.TempDir().Join("test").RemoveAll()
120120

121121
// We don't really care about the paths in this case
122-
pm := packagemanager.NewPackageManager(dataDir, dataDir, dataDir, dataDir)
122+
pm := packagemanager.NewPackageManager(dataDir, dataDir, dataDir, dataDir, "test")
123123

124124
// Create some boards with identical VID:PID combination
125125
pack := pm.Packages.GetOrCreatePackage("packager")

Diff for: ‎commands/daemon/daemon.go

+10-2
Original file line numberDiff line numberDiff line change
@@ -35,6 +35,7 @@ import (
3535
rpc "github.com/arduino/arduino-cli/rpc/cc/arduino/cli/commands/v1"
3636
"github.com/sirupsen/logrus"
3737
"google.golang.org/grpc/codes"
38+
"google.golang.org/grpc/metadata"
3839
"google.golang.org/grpc/status"
3940
)
4041

@@ -226,8 +227,15 @@ func (s *ArduinoCoreServerImpl) Upgrade(req *rpc.UpgradeRequest, stream rpc.Ardu
226227
}
227228

228229
// Create FIXMEDOC
229-
func (s *ArduinoCoreServerImpl) Create(_ context.Context, req *rpc.CreateRequest) (*rpc.CreateResponse, error) {
230-
res, err := commands.Create(req)
230+
func (s *ArduinoCoreServerImpl) Create(ctx context.Context, req *rpc.CreateRequest) (*rpc.CreateResponse, error) {
231+
var userAgent []string
232+
if md, ok := metadata.FromIncomingContext(ctx); ok {
233+
userAgent = md.Get("user-agent")
234+
}
235+
if len(userAgent) == 0 {
236+
userAgent = []string{"gRPCClientUnknown/0.0.0"}
237+
}
238+
res, err := commands.Create(req, userAgent...)
231239
return res, convertErrorToRPCStatus(err)
232240
}
233241

Diff for: ‎commands/debug/debug.go

+1-1
Original file line numberDiff line numberDiff line change
@@ -62,7 +62,7 @@ func Debug(ctx context.Context, req *dbg.DebugConfigRequest, inStream io.Reader,
6262
}
6363
entry.Debug("Executing debugger")
6464

65-
cmd, err := executils.NewProcess(commandLine...)
65+
cmd, err := executils.NewProcess(pm.GetEnvVarsForSpawnedProcess(), commandLine...)
6666
if err != nil {
6767
return nil, &arduino.FailedDebugError{Message: tr("Cannot execute debug tool"), Cause: err}
6868
}

Diff for: ‎commands/debug/debug_test.go

+1-1
Original file line numberDiff line numberDiff line change
@@ -36,7 +36,7 @@ func TestGetCommandLine(t *testing.T) {
3636
sketchPath := paths.New("testdata", sketch)
3737
require.NoError(t, sketchPath.ToAbs())
3838

39-
pm := packagemanager.NewPackageManager(nil, nil, nil, nil)
39+
pm := packagemanager.NewPackageManager(nil, nil, nil, nil, "test")
4040
pm.LoadHardwareFromDirectory(customHardware)
4141
pm.LoadHardwareFromDirectory(dataDir)
4242

Diff for: ‎commands/instances.go

+6-1
Original file line numberDiff line numberDiff line change
@@ -106,7 +106,7 @@ func (instance *CoreInstance) installToolIfMissing(tool *cores.ToolRelease, down
106106
}
107107

108108
// Create a new CoreInstance ready to be initialized, supporting directories are also created.
109-
func Create(req *rpc.CreateRequest) (*rpc.CreateResponse, error) {
109+
func Create(req *rpc.CreateRequest, extraUserAgent ...string) (*rpc.CreateResponse, error) {
110110
instance := &CoreInstance{}
111111

112112
// Setup downloads directory
@@ -129,11 +129,16 @@ func Create(req *rpc.CreateRequest) (*rpc.CreateResponse, error) {
129129
}
130130

131131
// Create package manager
132+
userAgent := "arduino-cli/" + globals.VersionInfo.VersionString
133+
for _, ua := range extraUserAgent {
134+
userAgent += " " + ua
135+
}
132136
instance.PackageManager = packagemanager.NewPackageManager(
133137
dataDir,
134138
configuration.PackagesDir(configuration.Settings),
135139
downloadsDir,
136140
dataDir.Join("tmp"),
141+
userAgent,
137142
)
138143

139144
// Create library manager and add libraries directories

Diff for: ‎commands/upload/upload.go

+7-6
Original file line numberDiff line numberDiff line change
@@ -531,19 +531,20 @@ func runProgramAction(pm *packagemanager.PackageManager,
531531
}
532532

533533
// Run recipes for upload
534+
toolEnv := pm.GetEnvVarsForSpawnedProcess()
534535
if burnBootloader {
535-
if err := runTool("erase.pattern", uploadProperties, outStream, errStream, verbose, dryRun); err != nil {
536+
if err := runTool("erase.pattern", uploadProperties, outStream, errStream, verbose, dryRun, toolEnv); err != nil {
536537
return &arduino.FailedUploadError{Message: tr("Failed chip erase"), Cause: err}
537538
}
538-
if err := runTool("bootloader.pattern", uploadProperties, outStream, errStream, verbose, dryRun); err != nil {
539+
if err := runTool("bootloader.pattern", uploadProperties, outStream, errStream, verbose, dryRun, toolEnv); err != nil {
539540
return &arduino.FailedUploadError{Message: tr("Failed to burn bootloader"), Cause: err}
540541
}
541542
} else if programmer != nil {
542-
if err := runTool("program.pattern", uploadProperties, outStream, errStream, verbose, dryRun); err != nil {
543+
if err := runTool("program.pattern", uploadProperties, outStream, errStream, verbose, dryRun, toolEnv); err != nil {
543544
return &arduino.FailedUploadError{Message: tr("Failed programming"), Cause: err}
544545
}
545546
} else {
546-
if err := runTool("upload.pattern", uploadProperties, outStream, errStream, verbose, dryRun); err != nil {
547+
if err := runTool("upload.pattern", uploadProperties, outStream, errStream, verbose, dryRun, toolEnv); err != nil {
547548
return &arduino.FailedUploadError{Message: tr("Failed uploading"), Cause: err}
548549
}
549550
}
@@ -552,7 +553,7 @@ func runProgramAction(pm *packagemanager.PackageManager,
552553
return nil
553554
}
554555

555-
func runTool(recipeID string, props *properties.Map, outStream, errStream io.Writer, verbose bool, dryRun bool) error {
556+
func runTool(recipeID string, props *properties.Map, outStream, errStream io.Writer, verbose bool, dryRun bool, toolEnv []string) error {
556557
recipe, ok := props.GetOk(recipeID)
557558
if !ok {
558559
return fmt.Errorf(tr("recipe not found '%s'"), recipeID)
@@ -577,7 +578,7 @@ func runTool(recipeID string, props *properties.Map, outStream, errStream io.Wri
577578
if dryRun {
578579
return nil
579580
}
580-
cmd, err := executils.NewProcess(cmdArgs...)
581+
cmd, err := executils.NewProcess(toolEnv, cmdArgs...)
581582
if err != nil {
582583
return fmt.Errorf(tr("cannot execute upload tool: %s"), err)
583584
}

Diff for: ‎commands/upload/upload_test.go

+1-1
Original file line numberDiff line numberDiff line change
@@ -127,7 +127,7 @@ func TestDetermineBuildPathAndSketchName(t *testing.T) {
127127
}
128128

129129
func TestUploadPropertiesComposition(t *testing.T) {
130-
pm := packagemanager.NewPackageManager(nil, nil, nil, nil)
130+
pm := packagemanager.NewPackageManager(nil, nil, nil, nil, "test")
131131
errs := pm.LoadHardwareFromDirectory(paths.New("testdata", "hardware"))
132132
require.Len(t, errs, 0)
133133
buildPath1 := paths.New("testdata", "build_path_1")

Diff for: ‎docs/UPGRADING.md

+36
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,27 @@ Here you can find a list of migration guides to handle breaking changes between
44

55
## Unreleased
66

7+
### `packagemanager.NewPackageManager` function change
8+
9+
A new argument `userAgent` has been added to `packagemanager.NewPackageManager`, the new function signature is:
10+
11+
```go
12+
func NewPackageManager(indexDir, packagesDir, downloadDir, tempDir *paths.Path, userAgent string) *PackageManager {
13+
```
14+
15+
The userAgent string must be in the format `"ProgramName/Version"`, for example `"arduino-cli/0.20.1"`.
16+
17+
### `commands.Create` function change
18+
19+
A new argument `extraUserAgent` has been added to `commands.Create`, the new function signature is:
20+
21+
```go
22+
func Create(req *rpc.CreateRequest, extraUserAgent ...string) (*rpc.CreateResponse, error) {
23+
```
24+
25+
`extraUserAgent` is an array of strings, so multiple user agent may be provided. Each user agent must be in the format
26+
`"ProgramName/Version"`, for example `"arduino-cli/0.20.1"`.
27+
728
### `commands.Compile` function change
829
930
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
3253
always parsed by the function. Furthermore the function now should also correctly interpret `packager:arch` spelled with
3354
the wrong casing.
3455
56+
### `github.com/arduino/arduino-cli/executils.NewProcess` and `executils.NewProcessFromPath` function change
57+
58+
A new argument `extraEnv` has been added to `executils.NewProcess` and `executils.NewProcessFromPath`, the new function
59+
signature is:
60+
61+
```go
62+
func NewProcess(extraEnv []string, args ...string) (*Process, error) {
63+
```
64+
65+
```go
66+
func NewProcessFromPath(extraEnv []string, executable *paths.Path, args ...string) (*Process, error) {
67+
```
68+
69+
The `extraEnv` params allow to pass environment variables (in addition to the default ones) to the spawned process.
70+
3571
### `github.com/arduino/arduino-cli/i18n.Init(...)` now requires an empty string to be passed for autodetection of locale
3672
3773
For automated detection of locale, change the call from:

Diff for: ‎docs/platform-specification.md

+9
Original file line numberDiff line numberDiff line change
@@ -702,6 +702,15 @@ The tool configuration properties are available globally without the prefix. For
702702
property can be used as **{cmd.path}** inside the recipe, and the same happens for all the other avrdude configuration
703703
variables.
704704

705+
### Environment variables
706+
707+
All the tools launched to compile or upload a sketch will have the following environment variable set:
708+
709+
`ARDUINO_USER_AGENT`: contains the name and version of the client used by the user in
710+
[HTTP user-agent format](https://en.wikipedia.org/wiki/User_agent), for example `"arduino-cli/0.21.0"`. It may also
711+
contain multiple space-delimited entries like `"arduino-cli/0.21.0 ArduinoIDE/2.0.0-rc1"` if this information is
712+
available.
713+
705714
### Pluggable discovery
706715

707716
Discovery tools are a special kind of tool used to find supported boards. A platform must declare one or more Pluggable

0 commit comments

Comments
 (0)