Skip to content

fix: discovery cleanup and watchers continuity upon core install #2062

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.

Sign up for GitHub

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

Merged
merged 9 commits into from
Mar 15, 2023
Merged

fix: discovery cleanup and watchers continuity upon core install #2062

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
a42a487
fix: clear discovery manager when spawing a new one
Feb 1, 2023
1b6d1c1
test: fix failing test
Feb 3, 2023
b73907b
chore: rm useless comment
Feb 13, 2023
27625c5
refactor: pr comments
Bikappa Feb 28, 2023
fbce838
chore: removed comment
Bikappa Mar 9, 2023
147179e
Update arduino/discovery/discoverymanager/discoverymanager.go
Bikappa Mar 9, 2023
35a4eea
fix: unused var
Bikappa Mar 9, 2023
61ac16a
fix: initialize in builder
Bikappa Mar 13, 2023
f661846
fix: merge error
Bikappa Mar 13, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
18 changes: 14 additions & 4 deletions arduino/cores/packagemanager/package_manager.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -98,15 +98,25 @@ func (pmb *Builder) BuildIntoExistingPackageManager(target *PackageManager) {
target.tempDir = pmb.tempDir
target.packagesCustomGlobalProperties = pmb.packagesCustomGlobalProperties
target.profile = pmb.profile
target.discoveryManager = pmb.discoveryManager
target.discoveryManager.Clear()
target.discoveryManager.AddAllDiscoveriesFrom(pmb.discoveryManager)
target.userAgent = pmb.userAgent
}

// Build builds a new PackageManager.
func (pmb *Builder) Build() *PackageManager {
res := &PackageManager{}
pmb.BuildIntoExistingPackageManager(res)
return res
return &PackageManager{
log: pmb.log,
packages: pmb.packages,
IndexDir: pmb.IndexDir,
PackagesDir: pmb.PackagesDir,
DownloadDir: pmb.DownloadDir,
tempDir: pmb.tempDir,
packagesCustomGlobalProperties: pmb.packagesCustomGlobalProperties,
profile: pmb.profile,
discoveryManager: pmb.discoveryManager,
userAgent: pmb.userAgent,
}
}

// NewBuilder creates a Builder with the same configuration
Expand Down
38 changes: 20 additions & 18 deletions arduino/cores/packagemanager/package_manager_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@
// Arduino software without disclosing the source code of your own applications.
// To purchase a commercial license, send an email to [email protected].

package packagemanager_test
package packagemanager

import (
"fmt"
Expand All @@ -24,7 +24,6 @@ import (
"testing"

"github.com/arduino/arduino-cli/arduino/cores"
"github.com/arduino/arduino-cli/arduino/cores/packagemanager"
"github.com/arduino/arduino-cli/configuration"
"github.com/arduino/go-paths-helper"
"github.com/arduino/go-properties-orderedmap"
Expand All @@ -39,7 +38,7 @@ var dataDir1 = paths.New("testdata", "data_dir_1")
var extraHardware = paths.New("testdata", "extra_hardware")

func TestFindBoardWithFQBN(t *testing.T) {
pmb := packagemanager.NewBuilder(customHardware, customHardware, customHardware, customHardware, "test")
pmb := NewBuilder(customHardware, customHardware, customHardware, customHardware, "test")
pmb.LoadHardwareFromDirectory(customHardware)
pm := pmb.Build()
pme, release := pm.NewExplorer()
Expand All @@ -57,7 +56,7 @@ func TestFindBoardWithFQBN(t *testing.T) {

func TestResolveFQBN(t *testing.T) {
// Pass nil, since these paths are only used for installing
pmb := packagemanager.NewBuilder(nil, nil, nil, nil, "test")
pmb := NewBuilder(nil, nil, nil, nil, "test")
// Hardware from main packages directory
pmb.LoadHardwareFromDirectory(dataDir1.Join("packages"))
// This contains the arduino:avr core
Expand Down Expand Up @@ -181,7 +180,7 @@ func TestResolveFQBN(t *testing.T) {
}

func TestBoardOptionsFunctions(t *testing.T) {
pmb := packagemanager.NewBuilder(customHardware, customHardware, customHardware, customHardware, "test")
pmb := NewBuilder(customHardware, customHardware, customHardware, customHardware, "test")
pmb.LoadHardwareFromDirectory(customHardware)
pm := pmb.Build()
pme, release := pm.NewExplorer()
Expand Down Expand Up @@ -221,13 +220,13 @@ func TestBoardOptionsFunctions(t *testing.T) {
}

func TestBoardOrdering(t *testing.T) {
pmb := packagemanager.NewBuilder(dataDir1, dataDir1.Join("packages"), nil, nil, "")
pmb := NewBuilder(dataDir1, dataDir1.Join("packages"), nil, nil, "")
_ = pmb.LoadHardwareFromDirectories(paths.NewPathList(dataDir1.Join("packages").String()))
pm := pmb.Build()
pme, release := pm.NewExplorer()
defer release()

pl := pme.FindPlatform(&packagemanager.PlatformReference{
pl := pme.FindPlatform(&PlatformReference{
Package: "arduino",
PlatformArchitecture: "avr",
})
Expand Down Expand Up @@ -273,7 +272,7 @@ func TestBoardOrdering(t *testing.T) {
func TestFindToolsRequiredForBoard(t *testing.T) {
os.Setenv("ARDUINO_DATA_DIR", dataDir1.String())
configuration.Settings = configuration.Init("")
pmb := packagemanager.NewBuilder(
pmb := NewBuilder(
dataDir1,
configuration.PackagesDir(configuration.Settings),
configuration.DownloadsDir(configuration.Settings),
Expand Down Expand Up @@ -314,7 +313,7 @@ func TestFindToolsRequiredForBoard(t *testing.T) {
})
require.NotNil(t, esptool0413)

testPlatform := pme.FindPlatformRelease(&packagemanager.PlatformReference{
testPlatform := pme.FindPlatformRelease(&PlatformReference{
Package: "test",
PlatformArchitecture: "avr",
PlatformVersion: semver.MustParse("1.1.0")})
Expand Down Expand Up @@ -407,7 +406,7 @@ func TestFindToolsRequiredForBoard(t *testing.T) {
}

func TestIdentifyBoard(t *testing.T) {
pmb := packagemanager.NewBuilder(customHardware, customHardware, customHardware, customHardware, "test")
pmb := NewBuilder(customHardware, customHardware, customHardware, customHardware, "test")
pmb.LoadHardwareFromDirectory(customHardware)
pm := pmb.Build()
pme, release := pm.NewExplorer()
Expand All @@ -434,12 +433,12 @@ func TestIdentifyBoard(t *testing.T) {

func TestPackageManagerClear(t *testing.T) {
// Create a PackageManager and load the harware
pmb := packagemanager.NewBuilder(customHardware, customHardware, customHardware, customHardware, "test")
pmb := NewBuilder(customHardware, customHardware, customHardware, customHardware, "test")
pmb.LoadHardwareFromDirectory(customHardware)
pm := pmb.Build()

// Creates another PackageManager but don't load the hardware
emptyPmb := packagemanager.NewBuilder(customHardware, customHardware, customHardware, customHardware, "test")
emptyPmb := NewBuilder(customHardware, customHardware, customHardware, customHardware, "test")
emptyPm := emptyPmb.Build()

// Verifies they're not equal
Expand All @@ -448,15 +447,18 @@ func TestPackageManagerClear(t *testing.T) {
// Clear the first PackageManager that contains loaded hardware
emptyPmb.BuildIntoExistingPackageManager(pm)

// Verifies both PackageManagers are now equal
// the discovery manager is maintained
require.NotEqual(t, pm.discoveryManager, emptyPm.discoveryManager)
// Verifies all other fields are assigned to target
pm.discoveryManager = emptyPm.discoveryManager
require.Equal(t, pm, emptyPm)
}

func TestFindToolsRequiredFromPlatformRelease(t *testing.T) {
// Create all the necessary data to load discoveries
fakePath := paths.New("fake-path")

pmb := packagemanager.NewBuilder(fakePath, fakePath, fakePath, fakePath, "test")
pmb := NewBuilder(fakePath, fakePath, fakePath, fakePath, "test")
pack := pmb.GetOrCreatePackage("arduino")

{
Expand Down Expand Up @@ -559,13 +561,13 @@ func TestFindToolsRequiredFromPlatformRelease(t *testing.T) {
}

func TestFindPlatformReleaseDependencies(t *testing.T) {
pmb := packagemanager.NewBuilder(nil, nil, nil, nil, "test")
pmb := NewBuilder(nil, nil, nil, nil, "test")
pmb.LoadPackageIndexFromFile(paths.New("testdata", "package_tooltest_index.json"))
pm := pmb.Build()
pme, release := pm.NewExplorer()
defer release()

pl, tools, err := pme.FindPlatformReleaseDependencies(&packagemanager.PlatformReference{Package: "test", PlatformArchitecture: "avr"})
pl, tools, err := pme.FindPlatformReleaseDependencies(&PlatformReference{Package: "test", PlatformArchitecture: "avr"})
require.NoError(t, err)
require.NotNil(t, pl)
require.Len(t, tools, 3)
Expand All @@ -574,7 +576,7 @@ func TestFindPlatformReleaseDependencies(t *testing.T) {

func TestLegacyPackageConversionToPluggableDiscovery(t *testing.T) {
// Pass nil, since these paths are only used for installing
pmb := packagemanager.NewBuilder(nil, nil, nil, nil, "test")
pmb := NewBuilder(nil, nil, nil, nil, "test")
// Hardware from main packages directory
pmb.LoadHardwareFromDirectory(dataDir1.Join("packages"))
pm := pmb.Build()
Expand Down Expand Up @@ -643,7 +645,7 @@ func TestLegacyPackageConversionToPluggableDiscovery(t *testing.T) {
}

func TestRunPostInstall(t *testing.T) {
pmb := packagemanager.NewBuilder(nil, nil, nil, nil, "test")
pmb := NewBuilder(nil, nil, nil, nil, "test")
pm := pmb.Build()
pme, release := pm.NewExplorer()
defer release()
Expand Down
1 change: 1 addition & 0 deletions arduino/discovery/discovery.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -381,6 +381,7 @@ func (disc *PluggableDiscovery) Quit() {
if _, err := disc.waitMessage(time.Second * 5); err != nil {
logrus.Errorf("Quitting discovery %s: %s", disc.id, err)
}
disc.stopSync()
disc.killProcess()
}

Expand Down
12 changes: 10 additions & 2 deletions arduino/discovery/discoverymanager/discoverymanager.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -194,12 +194,13 @@ func (dm *DiscoveryManager) startDiscovery(d *discovery.PluggableDiscovery) (dis
return fmt.Errorf("%s: %s", tr("starting discovery %s", d.GetID()), err)
}

go func() {
go func(d *discovery.PluggableDiscovery) {
// Transfer all incoming events from this discovery to the feed channel
for ev := range eventCh {
dm.feed <- ev
}
}()
logrus.Infof("Discovery event channel closed %s. Exiting goroutine.", d.GetID())
}(d)
return nil
}

Expand Down Expand Up @@ -276,3 +277,10 @@ func (dm *DiscoveryManager) List() []*discovery.Port {
}
return res
}

// AddAllDiscoveriesFrom transfers discoveries from src to the receiver
func (dm *DiscoveryManager) AddAllDiscoveriesFrom(src *DiscoveryManager) {
for _, d := range src.discoveries {
dm.Add(d)
}
}