From 5332ffdafc951e96fb7576e4a5b520c1658fb015 Mon Sep 17 00:00:00 2001 From: Cristian Maglie Date: Tue, 26 Jul 2022 11:20:47 +0200 Subject: [PATCH] Fix regression: do not crash on `board list` if a discovery is not properly installed (#1814) * Added tests for #1669 * Fixed regession in discoveries startup Fix #1669 --- arduino/discovery/discovery.go | 17 ++++++++++------- test/test_board.py | 19 +++++++++++++++++++ 2 files changed, 29 insertions(+), 7 deletions(-) diff --git a/arduino/discovery/discovery.go b/arduino/discovery/discovery.go index f20c1533d97..40067cc3e43 100644 --- a/arduino/discovery/discovery.go +++ b/arduino/discovery/discovery.go @@ -247,17 +247,18 @@ func (disc *PluggableDiscovery) runProcess() error { return err } disc.outgoingCommandsPipe = stdin - disc.process = proc messageChan := make(chan *discoveryMessage) disc.incomingMessagesChan = messageChan go disc.jsonDecodeLoop(stdout, messageChan) - if err := disc.process.Start(); err != nil { + if err := proc.Start(); err != nil { return err } + disc.statusMutex.Lock() defer disc.statusMutex.Unlock() + disc.process = proc disc.state = Alive logrus.Infof("started discovery %s process", disc.id) return nil @@ -265,11 +266,13 @@ func (disc *PluggableDiscovery) runProcess() error { func (disc *PluggableDiscovery) killProcess() error { logrus.Infof("killing discovery %s process", disc.id) - if err := disc.process.Kill(); err != nil { - return err - } - if err := disc.process.Wait(); err != nil { - return err + if disc.process != nil { + if err := disc.process.Kill(); err != nil { + return err + } + if err := disc.process.Wait(); err != nil { + return err + } } disc.statusMutex.Lock() defer disc.statusMutex.Unlock() diff --git a/test/test_board.py b/test/test_board.py index 15e70789fdb..0f36b07c34a 100644 --- a/test/test_board.py +++ b/test/test_board.py @@ -14,6 +14,8 @@ # a commercial license, send an email to license@arduino.cc. from pathlib import Path from git import Repo +import os +import glob import simplejson as json import semver import pytest @@ -405,6 +407,23 @@ def test_board_list(run_command): assert "protocol_label" in port["port"] +def test_board_list_with_invalid_discovery(run_command, data_dir): + run_command(["core", "update-index"]) + result = run_command(["board", "list"]) + assert result.ok + + # check that the CLI do no crash if an invalid discovery is installed + # (for example if the installation fails midway). + # https://github.com/arduino/arduino-cli/issues/1669 + tool_dir = os.path.join(data_dir, "packages", "builtin", "tools", "serial-discovery") + for file_to_delete in glob.glob(tool_dir + "/*/*"): + os.remove(file_to_delete) + + result = run_command(["board", "list"]) + assert result.ok + assert "builtin:serial-discovery" in result.stderr + + def test_board_listall(run_command): assert run_command(["update"]) assert run_command(["core", "install", "arduino:avr@1.8.3"])