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 <[email protected]>

Author: cmaglie

Diff for: 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 for: 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 for: 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:[email protected]")
+
+    # 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 for: test/testdata/sketch_with_conflicting_libraries_include/sketch_with_conflicting_libraries_include.ino

@@ -0,0 +1,7 @@
+#include "OneWire.h"
+
+void setup() {
+}
+
+void loop() {
+}