From 888a9a379f1119aa51485a9940f22c3a36020ddb Mon Sep 17 00:00:00 2001
From: Cristian Maglie <c.maglie@arduino.cc>
Date: Tue, 4 May 2021 16:42:14 +0200
Subject: [PATCH] Improved lib detection: check for matching name in
library.properties (#1276)
* Improved lib detection: check for matching name in library.properties
The library may be stored in a directory that doesn't match the library
name, for example we had a case in the wild where the directories:
libraries/onewire_2_3_4/...
libraries/onewireng_1_2_3/...
were used instead of:
libraries/OneWire/...
libraries/OneWireNg/...
this lead to incorrect selection of onewireng_1_2_3 when using OneWire.h
(because the OneWireNg had an architecture=avr that had priority over
the architecture=* of onewire_2_3_4).
This commit will restore priority straight.
* Added test for lib resolve improvement
* Lib discovery: always prefer libraries with the correct directory name
* [skip changelog] Add integration test
Co-authored-by: Silvano Cerza <silvanocerza@gmail.com>
---
arduino/libraries/librariesresolver/cpp.go | 13 ++++---
.../libraries/librariesresolver/cpp_test.go | 15 ++++++++
test/test_compile.py | 35 ++++++++++++++++---
...tch_with_conflicting_libraries_include.ino | 7 ++++
4 files changed, 61 insertions(+), 9 deletions(-)
create mode 100644 test/testdata/sketch_with_conflicting_libraries_include/sketch_with_conflicting_libraries_include.ino
diff --git a/arduino/libraries/librariesresolver/cpp.go b/arduino/libraries/librariesresolver/cpp.go
index e21e5dd1061..c762aba55de 100644
--- a/arduino/libraries/librariesresolver/cpp.go
+++ b/arduino/libraries/librariesresolver/cpp.go
@@ -121,6 +121,7 @@ func computePriority(lib *libraries.Library, header, arch string) int {
header = strings.TrimSuffix(header, filepath.Ext(header))
header = simplify(header)
name := simplify(lib.Name)
+ realName := simplify(lib.RealName)
priority := 0
@@ -137,15 +138,17 @@ func computePriority(lib *libraries.Library, header, arch string) int {
priority += 0
}
- if name == header {
+ if realName == header && name == header {
+ priority += 600
+ } else if realName == header || name == header {
priority += 500
- } else if name == header+"-master" {
+ } else if realName == header+"-master" || name == header+"-master" {
priority += 400
- } else if strings.HasPrefix(name, header) {
+ } else if strings.HasPrefix(realName, header) || strings.HasPrefix(name, header) {
priority += 300
- } else if strings.HasSuffix(name, header) {
+ } else if strings.HasSuffix(realName, header) || strings.HasSuffix(name, header) {
priority += 200
- } else if strings.Contains(name, header) {
+ } else if strings.Contains(realName, header) || strings.Contains(name, header) {
priority += 100
}
diff --git a/arduino/libraries/librariesresolver/cpp_test.go b/arduino/libraries/librariesresolver/cpp_test.go
index 0bea2dcbe0f..b9acb54b503 100644
--- a/arduino/libraries/librariesresolver/cpp_test.go
+++ b/arduino/libraries/librariesresolver/cpp_test.go
@@ -143,3 +143,18 @@ func TestCppHeaderResolver(t *testing.T) {
require.Equal(t, "Calculus Unified Lib", resolve("calculus_lib.h", l6, l7))
require.Equal(t, "Calculus Unified Lib", resolve("calculus_lib.h", l7, l6))
}
+
+func TestCppHeaderResolverWithLibrariesInStrangeDirectoryNames(t *testing.T) {
+ resolver := NewCppResolver()
+ librarylist := libraries.List{}
+ librarylist.Add(&libraries.Library{Name: "onewire_2_3_4", RealName: "OneWire", Architectures: []string{"*"}})
+ librarylist.Add(&libraries.Library{Name: "onewireng_2_3_4", RealName: "OneWireNg", Architectures: []string{"avr"}})
+ resolver.headers["OneWire.h"] = librarylist
+ require.Equal(t, "onewire_2_3_4", resolver.ResolveFor("OneWire.h", "avr").Name)
+
+ librarylist2 := libraries.List{}
+ librarylist2.Add(&libraries.Library{Name: "OneWire", RealName: "OneWire", Architectures: []string{"*"}})
+ librarylist2.Add(&libraries.Library{Name: "onewire_2_3_4", RealName: "OneWire", Architectures: []string{"avr"}})
+ resolver.headers["OneWire.h"] = librarylist2
+ require.Equal(t, "OneWire", resolver.ResolveFor("OneWire.h", "avr").Name)
+}
diff --git a/test/test_compile.py b/test/test_compile.py
index ef9fec12c8f..eebb47881b6 100644
--- a/test/test_compile.py
+++ b/test/test_compile.py
@@ -998,6 +998,33 @@ def test_recompile_with_different_library(run_command, data_dir):
assert f"Using previously compiled file: {obj_path}" not in res.stdout
+def test_compile_with_conflicting_libraries_include(run_command, data_dir, copy_sketch):
+ assert run_command("update")
+
+ assert run_command("core install arduino:avr@1.8.3")
+
+ # Install conflicting libraries
+ git_url = "https://github.com/pstolarz/OneWireNg.git"
+ one_wire_ng_lib_path = Path(data_dir, "libraries", "onewireng_0_8_1")
+ assert Repo.clone_from(git_url, one_wire_ng_lib_path, multi_options=["-b 0.8.1"])
+
+ git_url = "https://github.com/PaulStoffregen/OneWire.git"
+ one_wire_lib_path = Path(data_dir, "libraries", "onewire_2_3_5")
+ assert Repo.clone_from(git_url, one_wire_lib_path, multi_options=["-b v2.3.5"])
+
+ sketch_path = copy_sketch("sketch_with_conflicting_libraries_include")
+ fqbn = "arduino:avr:uno"
+
+ res = run_command(f"compile -b {fqbn} {sketch_path} --verbose")
+ assert res.ok
+ expected_output = [
+ 'Multiple libraries were found for "OneWire.h"',
+ f" Used: {one_wire_lib_path}",
+ f" Not used: {one_wire_ng_lib_path}",
+ ]
+ assert "\n".join(expected_output) in res.stdout
+
+
def test_compile_with_invalid_build_options_json(run_command, data_dir):
assert run_command("update")
@@ -1051,7 +1078,7 @@ def test_compile_with_esp32_bundled_libraries(run_command, data_dir, copy_sketch
fqbn = "esp32:esp32:esp32"
res = run_command(f"compile -b {fqbn} {sketch_path} --verbose")
- assert res.ok
+ assert res.failed
core_bundled_lib_path = Path(data_dir, "packages", "esp32", "hardware", "esp32", core_version, "libraries", "SD")
cli_installed_lib_path = Path(data_dir, "libraries", "SD")
@@ -1060,7 +1087,7 @@ def test_compile_with_esp32_bundled_libraries(run_command, data_dir, copy_sketch
f" Used: {core_bundled_lib_path}",
f" Not used: {cli_installed_lib_path}",
]
- assert "\n".join(expected_output) in res.stdout
+ assert "\n".join(expected_output) not in res.stdout
def test_compile_with_esp8266_bundled_libraries(run_command, data_dir, copy_sketch):
@@ -1090,7 +1117,7 @@ def test_compile_with_esp8266_bundled_libraries(run_command, data_dir, copy_sket
fqbn = "esp8266:esp8266:generic"
res = run_command(f"compile -b {fqbn} {sketch_path} --verbose")
- assert res.ok
+ assert res.failed
core_bundled_lib_path = Path(
data_dir, "packages", "esp8266", "hardware", "esp8266", core_version, "libraries", "SD"
@@ -1101,4 +1128,4 @@ def test_compile_with_esp8266_bundled_libraries(run_command, data_dir, copy_sket
f" Used: {core_bundled_lib_path}",
f" Not used: {cli_installed_lib_path}",
]
- assert "\n".join(expected_output) in res.stdout
+ assert "\n".join(expected_output) not in res.stdout
diff --git a/test/testdata/sketch_with_conflicting_libraries_include/sketch_with_conflicting_libraries_include.ino b/test/testdata/sketch_with_conflicting_libraries_include/sketch_with_conflicting_libraries_include.ino
new file mode 100644
index 00000000000..d9f812d6c3f
--- /dev/null
+++ b/test/testdata/sketch_with_conflicting_libraries_include/sketch_with_conflicting_libraries_include.ino
@@ -0,0 +1,7 @@
+#include "OneWire.h"
+
+void setup() {
+}
+
+void loop() {
+}