Fix library verification when installing from git repo or zip file (#1466)

* Fix library verification when installing from git repo or zip file

* Enhance code comments and unit tests
Co-authored-by: per1234 <accounts@perglass.com>
Co-authored-by: per1234 <accounts@perglass.com>

arduino/globals/globals.go

@@ -49,4 +49,11 @@ var (
 		".cpp": empty,
 		".S":   empty,
 	}
+
+	// HeaderFilesValidExtensions lists valid extensions for header files
+	HeaderFilesValidExtensions = map[string]struct{}{
+		".h":   empty,
+		".hpp": empty,
+		".hh":  empty,
+	}
 )

arduino/libraries/libraries.go

@@ -19,6 +19,7 @@ import (
 	"fmt"
 
 	"github.com/arduino/arduino-cli/arduino/cores"
+	"github.com/arduino/arduino-cli/arduino/globals"
 	"github.com/arduino/arduino-cli/i18n"
 	rpc "github.com/arduino/arduino-cli/rpc/cc/arduino/cli/commands/v1"
 	paths "github.com/arduino/go-paths-helper"
@@ -233,7 +234,11 @@ func (library *Library) SourceHeaders() ([]string, error) {
 		if err != nil {
 			return nil, fmt.Errorf(tr("reading lib src dir: %s"), err)
 		}
-		cppHeaders.FilterSuffix(".h", ".hpp", ".hh")
+		headerExtensions := []string{}
+		for k := range globals.HeaderFilesValidExtensions {
+			headerExtensions = append(headerExtensions, k)
+		}
+		cppHeaders.FilterSuffix(headerExtensions...)
 		res := []string{}
 		for _, cppHeader := range cppHeaders {
 			res = append(res, cppHeader.Base())

arduino/libraries/librariesmanager/install.go

@@ -22,6 +22,7 @@ import (
 	"os"
 	"strings"
 
+	"github.com/arduino/arduino-cli/arduino/globals"
 	"github.com/arduino/arduino-cli/arduino/libraries"
 	"github.com/arduino/arduino-cli/arduino/libraries/librariesindex"
 	"github.com/arduino/arduino-cli/arduino/utils"
@@ -139,7 +140,7 @@ func (lm *LibrariesManager) InstallZipLib(ctx context.Context, archivePath strin
 	extractionPath := paths[0]
 	libraryName := extractionPath.Base()
 
-	if err := validateLibrary(libraryName, extractionPath); err != nil {
+	if err := validateLibrary(extractionPath); err != nil {
 		return err
 	}
 
@@ -229,7 +230,7 @@ func (lm *LibrariesManager) InstallGitLib(gitURL string, overwrite bool) error {
 		return err
 	}
 
-	if err := validateLibrary(libraryName, installPath); err != nil {
+	if err := validateLibrary(installPath); err != nil {
 		// Clean up installation directory since this is not a valid library
 		installPath.RemoveAll()
 		return err
@@ -259,22 +260,47 @@ func parseGitURL(gitURL string) (string, error) {
 	return res, nil
 }
 
-// validateLibrary verifies the dir contains a valid library, meaning it has both
-// an header <name>.h, either in src or root folder, and a library.properties file
-func validateLibrary(name string, dir *paths.Path) error {
-	// Verify library contains library header.
-	// Checks also root folder for legacy reasons.
-	// For more info see:
+// validateLibrary verifies the dir contains a valid library, meaning it has either
+// library.properties file and an header in src/ or an header in its root folder.
+// Returns nil if dir contains a valid library, error on all other cases.
+func validateLibrary(dir *paths.Path) error {
+	if dir.NotExist() {
+		return fmt.Errorf(tr("directory doesn't exist: %s", dir))
+	}
+
+	searchHeaderFile := func(d *paths.Path) (bool, error) {
+		if d.NotExist() {
+			// A directory that doesn't exist can't obviously contain any header file
+			return false, nil
+		}
+		dirContent, err := d.ReadDir()
+		if err != nil {
+			return false, fmt.Errorf(tr("reading directory %s content: %w", dir, err))
+		}
+		dirContent.FilterOutDirs()
+		headerExtensions := []string{}
+		for k := range globals.HeaderFilesValidExtensions {
+			headerExtensions = append(headerExtensions, k)
+		}
+		dirContent.FilterSuffix(headerExtensions...)
+		return len(dirContent) > 0, nil
+	}
+
+	// Recursive library layout
 	// https://arduino.github.io/arduino-cli/latest/library-specification/#source-code
-	libraryHeader := name + ".h"
-	if !dir.Join("src", libraryHeader).Exist() && !dir.Join(libraryHeader).Exist() {
-		return fmt.Errorf(tr(`library is not valid: missing header file "%s"`), libraryHeader)
+	if headerFound, err := searchHeaderFile(dir.Join("src")); err != nil {
+		return err
+	} else if dir.Join("library.properties").Exist() && headerFound {
+		return nil
 	}
 
-	// Verifies library contains library.properties
-	if !dir.Join("library.properties").Exist() {
-		return fmt.Errorf(tr(`library is not valid: missing file "library.properties"`))
+	// Flat library layout
+	// https://arduino.github.io/arduino-cli/latest/library-specification/#source-code
+	if headerFound, err := searchHeaderFile(dir); err != nil {
+		return err
+	} else if headerFound {
+		return nil
 	}
 
-	return nil
+	return fmt.Errorf(tr("library not valid"))
 }

arduino/libraries/librariesmanager/install_test.go

@@ -18,6 +18,7 @@ package librariesmanager
 import (
 	"testing"
 
+	"github.com/arduino/go-paths-helper"
 	"github.com/stretchr/testify/require"
 )
 
@@ -62,3 +63,43 @@ func TestParseGitURL(t *testing.T) {
 	require.Equal(t, "arduino-lib", libraryName)
 	require.NoError(t, err)
 }
+
+func TestValidateLibrary(t *testing.T) {
+	tmpDir := paths.New(t.TempDir())
+
+	nonExistingDirLib := tmpDir.Join("nonExistingDirLib")
+	err := validateLibrary(nonExistingDirLib)
+	require.Errorf(t, err, "directory doesn't exist: %s", nonExistingDirLib)
+
+	emptyLib := tmpDir.Join("emptyLib")
+	emptyLib.Mkdir()
+	err = validateLibrary(emptyLib)
+	require.Errorf(t, err, "library not valid")
+
+	onlyPropertiesLib := tmpDir.Join("onlyPropertiesLib")
+	onlyPropertiesLib.Mkdir()
+	onlyPropertiesLib.Join("library.properties").WriteFile([]byte{})
+	err = validateLibrary(onlyPropertiesLib)
+	require.Errorf(t, err, "library not valid")
+
+	missingPropertiesLib := tmpDir.Join("missingPropertiesLib")
+	missingPropertiesLibSourceDir := missingPropertiesLib.Join("src")
+	missingPropertiesLibSourceDir.MkdirAll()
+	missingPropertiesLibSourceDir.Join("some_file.hpp").WriteFile([]byte{})
+	err = validateLibrary(missingPropertiesLib)
+	require.Errorf(t, err, "library not valid")
+
+	validLib := tmpDir.Join("valiLib")
+	validLibSourceDir := validLib.Join("src")
+	validLibSourceDir.MkdirAll()
+	validLibSourceDir.Join("some_file.hpp").WriteFile([]byte{})
+	validLib.Join("library.properties").WriteFile([]byte{})
+	err = validateLibrary(validLib)
+	require.NoError(t, err)
+
+	validLegacyLib := tmpDir.Join("validLegacyLib")
+	validLegacyLib.Mkdir()
+	validLegacyLib.Join("some_file.hpp").WriteFile([]byte{})
+	err = validateLibrary(validLib)
+	require.NoError(t, err)
+}

i18n/data/en.po

@@ -2238,10 +2238,10 @@ msgstr "Use the specified programmer to upload."
 msgid "Used: {0}"
 msgstr "Used: {0}"
 
-#: arduino/libraries/librariesmanager/install.go:67
-#: arduino/libraries/librariesmanager/install.go:83
-#: arduino/libraries/librariesmanager/install.go:105
-#: arduino/libraries/librariesmanager/install.go:189
+#: arduino/libraries/librariesmanager/install.go:68
+#: arduino/libraries/librariesmanager/install.go:84
+#: arduino/libraries/librariesmanager/install.go:106
+#: arduino/libraries/librariesmanager/install.go:190
 msgid "User directory not set"
 msgstr "User directory not set"
 
@@ -2355,7 +2355,7 @@ msgstr "Writing config file: %v"
 msgid "archive hash differs from hash in index"
 msgstr "archive hash differs from hash in index"
 
-#: arduino/libraries/librariesmanager/install.go:136
+#: arduino/libraries/librariesmanager/install.go:137
 msgid "archive is not valid: multiple files found in zip file top level"
 msgstr "archive is not valid: multiple files found in zip file top level"
 
@@ -2533,10 +2533,14 @@ msgstr "dependency '%s' is not available"
 msgid "destination already exists"
 msgstr "destination already exists"
 
-#: arduino/libraries/librariesmanager/install.go:74
+#: arduino/libraries/librariesmanager/install.go:75
 msgid "destination dir %s already exists, cannot install"
 msgstr "destination dir %s already exists, cannot install"
 
+#: arduino/libraries/librariesmanager/install.go:268
+msgid "directory doesn't exist: %s"
+msgstr "directory doesn't exist: %s"
+
 #: arduino/discovery/discoverymanager/discoverymanager.go:112
 msgid "discovery %[1]s process not started: %[2]w"
 msgstr "discovery %[1]s process not started: %[2]w"
@@ -2599,7 +2603,7 @@ msgstr "error: %s and %s flags cannot be used together"
 msgid "extracting archive: %s"
 msgstr "extracting archive: %s"
 
-#: arduino/libraries/librariesmanager/install.go:124
+#: arduino/libraries/librariesmanager/install.go:125
 msgid "extracting archive: %w"
 msgstr "extracting archive: %w"
 
@@ -2694,7 +2698,7 @@ msgstr "getting tool dependencies for platform %[1]s: %[2]s"
 msgid "importing sketch metadata: %s"
 msgstr "importing sketch metadata: %s"
 
-#: arduino/libraries/librariesmanager/install.go:91
+#: arduino/libraries/librariesmanager/install.go:92
 msgid "install directory not set"
 msgstr "install directory not set"
 
@@ -2755,7 +2759,7 @@ msgstr "invalid empty library version: %s"
 msgid "invalid empty option found"
 msgstr "invalid empty option found"
 
-#: arduino/libraries/librariesmanager/install.go:257
+#: arduino/libraries/librariesmanager/install.go:258
 msgid "invalid git url"
 msgstr "invalid git url"
 
@@ -2819,22 +2823,18 @@ msgstr "key not found in settings"
 msgid "keywords"
 msgstr "keywords"
 
-#: arduino/libraries/librariesmanager/install.go:162
-#: arduino/libraries/librariesmanager/install.go:205
+#: arduino/libraries/librariesmanager/install.go:163
+#: arduino/libraries/librariesmanager/install.go:206
 msgid "library %s already installed"
 msgstr "library %s already installed"
 
-#: arduino/libraries/librariesmanager/install.go:37
+#: arduino/libraries/librariesmanager/install.go:38
 msgid "library already installed"
 msgstr "library already installed"
 
-#: arduino/libraries/librariesmanager/install.go:276
-msgid "library is not valid: missing file \"library.properties\""
-msgstr "library is not valid: missing file \"library.properties\""
-
-#: arduino/libraries/librariesmanager/install.go:271
-msgid "library is not valid: missing header file \"%s\""
-msgstr "library is not valid: missing header file \"%s\""
+#: arduino/libraries/librariesmanager/install.go:305
+msgid "library not valid"
+msgstr "library not valid"
 
 #: arduino/libraries/librariesmanager/librariesmanager.go:226
 msgid "library path does not exist: %s"
@@ -2917,7 +2917,7 @@ msgstr "missing platform release %[1]s:%[2]s referenced by board %[3]s"
 msgid "monitor release not found: %s"
 msgstr "monitor release not found: %s"
 
-#: arduino/libraries/librariesmanager/install.go:179
+#: arduino/libraries/librariesmanager/install.go:180
 #: arduino/resources/install.go:94
 msgid "moving extracted archive to destination dir: %s"
 msgstr "moving extracted archive to destination dir: %s"
@@ -3077,6 +3077,10 @@ msgstr "reading dir %[1]s: %[2]s"
 msgid "reading directory %[1]s: %[2]s"
 msgstr "reading directory %[1]s: %[2]s"
 
+#: arduino/libraries/librariesmanager/install.go:278
+msgid "reading directory %s content: %w"
+msgstr "reading directory %s content: %w"
+
 #: arduino/builder/sketch.go:76
 msgid "reading file %[1]s: %[2]s"
 msgstr "reading file %[1]s: %[2]s"
@@ -3093,11 +3097,11 @@ msgstr "reading inventory file: %w"
 msgid "reading lib headers: %s"
 msgstr "reading lib headers: %s"
 
-#: arduino/libraries/libraries.go:234
+#: arduino/libraries/libraries.go:235
 msgid "reading lib src dir: %s"
 msgstr "reading lib src dir: %s"
 
-#: arduino/libraries/libraries.go:114
+#: arduino/libraries/libraries.go:115
 msgid "reading library headers: %w"
 msgstr "reading library headers: %w"
 
@@ -3135,7 +3139,7 @@ msgstr "release not found"
 msgid "removing corrupted archive file: %s"
 msgstr "removing corrupted archive file: %s"
 
-#: arduino/libraries/librariesmanager/install.go:94
+#: arduino/libraries/librariesmanager/install.go:95
 msgid "removing lib directory: %s"
 msgstr "removing lib directory: %s"
 

test/test_lib.py

@@ -925,7 +925,7 @@ def test_install_zip_invalid_library(run_command, data_dir, downloads_dir):
     # Test zip-path install
     res = run_command(["lib", "install", "--zip-path", zip_path])
     assert res.failed
-    assert 'library is not valid: missing header file "lib-without-header.h"' in res.stderr
+    assert "library not valid" in res.stderr
 
     lib_install_dir = Path(data_dir, "libraries", "lib-without-properties")
     # Verifies library is not already installed
@@ -935,7 +935,7 @@ def test_install_zip_invalid_library(run_command, data_dir, downloads_dir):
     # Test zip-path install
     res = run_command(["lib", "install", "--zip-path", zip_path])
     assert res.failed
-    assert 'library is not valid: missing file "library.properties"' in res.stderr
+    assert "library not valid" in res.stderr
 
 
 def test_install_git_invalid_library(run_command, data_dir, downloads_dir):
@@ -962,7 +962,7 @@ def test_install_git_invalid_library(run_command, data_dir, downloads_dir):
 
     res = run_command(["lib", "install", "--git-url", repo_dir], custom_env=env)
     assert res.failed
-    assert 'library is not valid: missing header file "lib-without-header.h"' in res.stderr
+    assert "library not valid" in res.stderr
     assert not lib_install_dir.exists()
 
     # Create another fake library repository
@@ -980,5 +980,5 @@ def test_install_git_invalid_library(run_command, data_dir, downloads_dir):
 
     res = run_command(["lib", "install", "--git-url", repo_dir], custom_env=env)
     assert res.failed
-    assert 'library is not valid: missing file "library.properties"' in res.stderr
+    assert "library not valid" in res.stderr
     assert not lib_install_dir.exists()