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 set to all processes spawned from the arduino-cli.
cmaglie marked this conversation as resolved.
Show resolved Hide resolved
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
8 changes: 8 additions & 0 deletions docs/platform-specification.md
Original file line number Diff line number Diff line change
Expand Up @@ -702,6 +702,14 @@ 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`: will 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
contains multiple entries like `"arduino-cli/0.21.0 ArduinoIDE/2.0.0-rc1"` if this information is available.
cmaglie marked this conversation as resolved.
Show resolved Hide resolved

### 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
25 changes: 15 additions & 10 deletions executils/process.go
Original file line number Diff line number Diff line change
Expand Up @@ -30,16 +30,19 @@ 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
// arguments to the command.
func NewProcess(args ...string) (*Process, error) {
if args == nil || len(args) == 0 {
// NewProcess creates a command with the provided command line arguments
// and environment variables (that will be added to the parent os.Environ).
// The first argument args[0] is the path to the executable, the remainder
// are the arguments to the command.
cmaglie marked this conversation as resolved.
Show resolved Hide resolved
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(p.cmd.Env, os.Environ()...)
p.cmd.Env = append(p.cmd.Env, extraEnv...)
TellCommandNotToSpawnShell(p.cmd)

// This is required because some tools detects if the program is running
Expand All @@ -49,12 +52,12 @@ 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 environemnt vars and command line arguments.
cmaglie marked this conversation as resolved.
Show resolved Hide resolved
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
Expand Down Expand Up @@ -140,7 +143,9 @@ func (p *Process) Run() error {

// SetEnvironment set the enviroment for the running process. Each entry is of the form "key=value".
func (p *Process) SetEnvironment(values []string) {
p.cmd.Env = values
p.cmd.Env = nil
p.cmd.Env = append(p.cmd.Env, os.Environ()...)
p.cmd.Env = append(p.cmd.Env, values...)
cmaglie marked this conversation as resolved.
Show resolved Hide resolved
}

// RunWithinContext starts the specified command and waits for it to complete. If the given context
Expand Down
4 changes: 2 additions & 2 deletions executils/process_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
Loading