From 888a9a379f1119aa51485a9940f22c3a36020ddb Mon Sep 17 00:00:00 2001 From: Cristian Maglie 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 --- 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() { +}