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

Co-authored-by: per1234 <[email protected]>

Author: silvanocerza

Diff for: 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,
+	}
 )

Diff for: 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())

Diff for: arduino/libraries/librariesmanager/install.go

@@ -23,6 +23,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"
@@ -134,7 +135,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
 	}
 
@@ -224,7 +225,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
@@ -254,22 +255,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"))
 }

Diff for: 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)
+}

Diff for: i18n/data/en.po

@@ -2268,10 +2268,10 @@ msgstr "Use %s for more information about a command."
 msgid "Use the specified programmer to upload."
 msgstr "Use the specified programmer to upload."
 
-#: arduino/libraries/librariesmanager/install.go:62
-#: arduino/libraries/librariesmanager/install.go:78
-#: arduino/libraries/librariesmanager/install.go:100
-#: arduino/libraries/librariesmanager/install.go:184
+#: arduino/libraries/librariesmanager/install.go:63
+#: arduino/libraries/librariesmanager/install.go:79
+#: arduino/libraries/librariesmanager/install.go:101
+#: arduino/libraries/librariesmanager/install.go:185
 msgid "User directory not set"
 msgstr "User directory not set"
 
@@ -2397,7 +2397,7 @@ msgstr "[DEPRECATED] %s"
 msgid "archive hash differs from hash in index"
 msgstr "archive hash differs from hash in index"
 
-#: arduino/libraries/librariesmanager/install.go:131
+#: arduino/libraries/librariesmanager/install.go:132
 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"
 
@@ -2547,10 +2547,14 @@ msgstr "dependency '%s' is not available"
 msgid "destination already exists"
 msgstr "destination already exists"
 
-#: arduino/libraries/librariesmanager/install.go:69
+#: arduino/libraries/librariesmanager/install.go:70
 msgid "destination dir %s already exists, cannot install"
 msgstr "destination dir %s already exists, cannot install"
 
+#: arduino/libraries/librariesmanager/install.go:263
+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"
@@ -2615,7 +2619,7 @@ msgstr "error: %s and %s flags cannot be used together"
 msgid "extracting archive: %s"
 msgstr "extracting archive: %s"
 
-#: arduino/libraries/librariesmanager/install.go:119
+#: arduino/libraries/librariesmanager/install.go:120
 msgid "extracting archive: %w"
 msgstr "extracting archive: %w"
 
@@ -2714,7 +2718,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:86
+#: arduino/libraries/librariesmanager/install.go:87
 msgid "install directory not set"
 msgstr "install directory not set"
 
@@ -2775,7 +2779,7 @@ msgstr "invalid empty library version: %s"
 msgid "invalid empty option found"
 msgstr "invalid empty option found"
 
-#: arduino/libraries/librariesmanager/install.go:252
+#: arduino/libraries/librariesmanager/install.go:253
 msgid "invalid git url"
 msgstr "invalid git url"
 
@@ -2839,22 +2843,18 @@ msgstr "key not found in settings"
 msgid "keywords"
 msgstr "keywords"
 
-#: arduino/libraries/librariesmanager/install.go:157
-#: arduino/libraries/librariesmanager/install.go:200
+#: arduino/libraries/librariesmanager/install.go:158
+#: arduino/libraries/librariesmanager/install.go:201
 msgid "library %s already installed"
 msgstr "library %s already installed"
 
-#: arduino/libraries/librariesmanager/install.go:38
+#: arduino/libraries/librariesmanager/install.go:39
 msgid "library already installed"
 msgstr "library already installed"
 
-#: arduino/libraries/librariesmanager/install.go:271
-msgid "library is not valid: missing file \"library.properties\""
-msgstr "library is not valid: missing file \"library.properties\""
-
-#: arduino/libraries/librariesmanager/install.go:266
-msgid "library is not valid: missing header file \"%s\""
-msgstr "library is not valid: missing header file \"%s\""
+#: arduino/libraries/librariesmanager/install.go:300
+msgid "library not valid"
+msgstr "library not valid"
 
 #: arduino/libraries/librariesmanager/librariesmanager.go:226
 msgid "library path does not exist: %s"
@@ -2933,7 +2933,7 @@ msgstr "missing platform %[1]s:%[2]s referenced by board %[3]s"
 msgid "missing platform release %[1]s:%[2]s referenced by board %[3]s"
 msgstr "missing platform release %[1]s:%[2]s referenced by board %[3]s"
 
-#: arduino/libraries/librariesmanager/install.go:174
+#: arduino/libraries/librariesmanager/install.go:175
 #: arduino/resources/install.go:94
 msgid "moving extracted archive to destination dir: %s"
 msgstr "moving extracted archive to destination dir: %s"
@@ -3091,6 +3091,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:273
+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"
@@ -3107,11 +3111,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"
 
@@ -3148,7 +3152,7 @@ msgstr "release not found"
 msgid "removing corrupted archive file: %s"
 msgstr "removing corrupted archive file: %s"
 
-#: arduino/libraries/librariesmanager/install.go:89
+#: arduino/libraries/librariesmanager/install.go:90
 msgid "removing lib directory: %s"
 msgstr "removing lib directory: %s"
 

Diff for: i18n/rice-box.go

@@ -925,7 +925,7 @@ def test_install_zip_invalid_library(run_command, data_dir, downloads_dir):
     # Test zip-path install
     res = run_command(f"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(f"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(f"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(f"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()