Skip to content

Commit 10d0790

Browse files
authored
Fix platform tool dependency determination (#1020)
* Fix platform tool dependency determination The platform tool dependency check was giving false negatives due to comparing pointers instead of version values. This caused tools to be removed during platform uninstallation even when another installed platform had a dependency on that tool. * Add integration test for tool removal during platform uninstall Tool dependency by `arduino-cli core uninstall` is somewhat complex because it must only uninstall the tools that no other platforms have a dependency on. If it doesn't, it breaks the other platform and the cause of this breakage would likely not be obvious to the user. So it's important to test to ensure this functionality continues to work correctly.
1 parent 37ba57e commit 10d0790

File tree

3 files changed

+79
-1
lines changed

3 files changed

+79
-1
lines changed

Diff for: arduino/cores/cores.go

+1-1
Original file line numberDiff line numberDiff line change
@@ -218,7 +218,7 @@ func (release *PlatformRelease) RequiresToolRelease(toolRelease *ToolRelease) bo
218218
for _, toolDep := range release.Dependencies {
219219
if toolDep.ToolName == toolRelease.Tool.Name &&
220220
toolDep.ToolPackager == toolRelease.Tool.Package.Name &&
221-
toolDep.ToolVersion == toolRelease.Version {
221+
toolDep.ToolVersion.Equal(toolRelease.Version) {
222222
return true
223223
}
224224
}

Diff for: arduino/cores/cores_test.go

+57
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,57 @@
1+
// This file is part of arduino-cli.
2+
//
3+
// Copyright 2020 ARDUINO SA (http://www.arduino.cc/)
4+
//
5+
// This software is released under the GNU General Public License version 3,
6+
// which covers the main part of arduino-cli.
7+
// The terms of this license can be found at:
8+
// https://www.gnu.org/licenses/gpl-3.0.en.html
9+
//
10+
// You can be released from the requirements of the above licenses by purchasing
11+
// a commercial license. Buying such a license is mandatory if you want to
12+
// modify or otherwise use the software for commercial activities involving the
13+
// Arduino software without disclosing the source code of your own applications.
14+
// To purchase a commercial license, send an email to [email protected].
15+
16+
package cores
17+
18+
import (
19+
"testing"
20+
21+
"github.com/stretchr/testify/require"
22+
semver "go.bug.st/relaxed-semver"
23+
)
24+
25+
func TestRequiresToolRelease(t *testing.T) {
26+
toolDependencyName := "avr-gcc"
27+
toolDependencyVersion := "7.3.0-atmel3.6.1-arduino7"
28+
toolDependencyPackager := "arduino"
29+
30+
release := PlatformRelease{
31+
Dependencies: ToolDependencies{
32+
{
33+
ToolName: toolDependencyName,
34+
ToolVersion: semver.ParseRelaxed(toolDependencyVersion),
35+
ToolPackager: toolDependencyPackager,
36+
},
37+
},
38+
}
39+
40+
toolRelease := &ToolRelease{
41+
Version: semver.ParseRelaxed(toolDependencyVersion + "not"),
42+
Tool: &Tool{
43+
Name: toolDependencyName + "not",
44+
Package: &Package{
45+
Name: toolDependencyPackager + "not",
46+
},
47+
},
48+
}
49+
50+
require.False(t, release.RequiresToolRelease(toolRelease))
51+
toolRelease.Tool.Name = toolDependencyName
52+
require.False(t, release.RequiresToolRelease(toolRelease))
53+
toolRelease.Tool.Package.Name = toolDependencyPackager
54+
require.False(t, release.RequiresToolRelease(toolRelease))
55+
toolRelease.Version = semver.ParseRelaxed(toolDependencyVersion)
56+
require.True(t, release.RequiresToolRelease(toolRelease))
57+
}

Diff for: test/test_core.py

+21
Original file line numberDiff line numberDiff line change
@@ -236,6 +236,27 @@ def test_core_uninstall(run_command):
236236
assert not _in(result.stdout, "arduino:avr")
237237

238238

239+
def test_core_uninstall_tool_dependency_removal(run_command, data_dir):
240+
# These platforms both have a dependency on the arduino:[email protected] tool
241+
# arduino:[email protected] has a dependency on arduino:[email protected]
242+
assert run_command("core install arduino:[email protected]")
243+
# arduino:[email protected] has a dependency on arduino:[email protected]
244+
assert run_command("core install arduino:[email protected]")
245+
assert run_command("core uninstall arduino:avr")
246+
247+
arduino_tools_path = Path(data_dir, "packages", "arduino", "tools")
248+
249+
avr_gcc_binaries_path = arduino_tools_path.joinpath("avr-gcc", "7.3.0-atmel3.6.1-arduino5", "bin")
250+
# The tool arduino:[email protected] that is a dep of another installed platform should remain
251+
assert avr_gcc_binaries_path.joinpath("avr-gcc").exists() or avr_gcc_binaries_path.joinpath("avr-gcc.exe").exists()
252+
253+
avrdude_binaries_path = arduino_tools_path.joinpath("avrdude", "6.3.0-arduino17", "bin")
254+
# The tool arduino:[email protected] that is only a dep of arduino:avr should have been removed
255+
assert (
256+
avrdude_binaries_path.joinpath("avrdude").exists() or avrdude_binaries_path.joinpath("avrdude.exe").exists()
257+
) is False
258+
259+
239260
def test_core_zipslip(run_command):
240261
url = "https://raw.githubusercontent.com/arduino/arduino-cli/master/test/testdata/test_index.json"
241262
assert run_command("core update-index --additional-urls={}".format(url))

0 commit comments

Comments
 (0)