Merge pull request #293 from arduino/cleanups

Some cleanups to the codebase

libraries/repolist.go → internal/cli/check-registry.go

@@ -1,6 +1,6 @@
 // This file is part of libraries-repository-engine.
 //
-// Copyright 2021 ARDUINO SA (http://www.arduino.cc/)
+// Copyright 2025 ARDUINO SA (http://www.arduino.cc/)
 //
 // This program is free software: you can redistribute it and/or modify
 // it under the terms of the GNU Affero General Public License as published
@@ -21,21 +21,26 @@
 // Arduino software without disclosing the source code of your own applications.
 // To purchase a commercial license, send an email to [email protected].
 
-package libraries
+package cli
 
 import (
-	"github.com/arduino/libraries-repository-engine/internal/libraries"
+	checkregistry "github.com/arduino/libraries-repository-engine/internal/command/check-registry"
+	"github.com/spf13/cobra"
 )
 
-// LoadRepoListFromFile returns an unfiltered list of library registry entries loaded from the given data file.
-func LoadRepoListFromFile(filename string) ([]*Repo, error) {
-	return libraries.LoadRepoListFromFile(filename)
-}
-
-// Repo is the type for the library repository data.
-type Repo = libraries.Repo
+func init() {
+	// checkRegistryCmd defines the `check-registry` CLI subcommand.
+	var checkRegistryCmd = &cobra.Command{
+		Short:                 "Check the registry.txt file format",
+		Long:                  "Check the registry.txt file format",
+		DisableFlagsInUseLine: true,
+		Use: `check-registry FLAG... /path/to/registry.txt
 
-// ListRepos returns a filtered list of library registry entries loaded from the given data file.
-func ListRepos(reposFilename string) ([]*Repo, error) {
-	return libraries.ListRepos(reposFilename)
+Validate the registry.txt format and correctness.`,
+		Args: cobra.ExactArgs(1),
+		Run: func(cmd *cobra.Command, args []string) {
+			checkregistry.CheckRegistry(args[0])
+		},
+	}
+	rootCmd.AddCommand(checkRegistryCmd)
 }

internal/command/check-registry/check-registry.go

@@ -0,0 +1,94 @@
+// This file is part of libraries-repository-engine.
+//
+// Copyright 2025 ARDUINO SA (http://www.arduino.cc/)
+//
+// This program is free software: you can redistribute it and/or modify
+// it under the terms of the GNU Affero General Public License as published
+// by the Free Software Foundation, either version 3 of the License, or
+// (at your option) any later version.
+//
+// This program is distributed in the hope that it will be useful,
+// but WITHOUT ANY WARRANTY; without even the implied warranty of
+// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+// GNU Affero General Public License for more details.
+//
+// You should have received a copy of the GNU Affero General Public License
+// along with this program.  If not, see <https://www.gnu.org/licenses/>.
+//
+// You can be released from the requirements of the above licenses by purchasing
+// a commercial license. Buying such a license is mandatory if you want to
+// modify or otherwise use the software for commercial activities involving the
+// Arduino software without disclosing the source code of your own applications.
+// To purchase a commercial license, send an email to [email protected].
+
+package checkregistry
+
+import (
+	"errors"
+	"fmt"
+	"os"
+	"reflect"
+
+	"github.com/arduino/libraries-repository-engine/internal/libraries"
+)
+
+// CheckRegistry runs the check-registry action
+func CheckRegistry(reposFile string) {
+	if err := runcheck(reposFile); err != nil {
+		fmt.Fprintf(os.Stderr, "Error: %s\n", err.Error())
+		os.Exit(1)
+	}
+}
+
+func runcheck(reposFile string) error {
+	info, err := os.Stat(reposFile)
+	if err != nil {
+		return fmt.Errorf("while loading registry data file: %w", err)
+	}
+
+	if info.IsDir() {
+		return fmt.Errorf("registry data file argument %s is a folder, not a file", reposFile)
+	}
+
+	rawRepos, err := libraries.LoadRepoListFromFile(reposFile)
+	if err != nil {
+		return fmt.Errorf("while loading registry data file: %w", err)
+	}
+
+	filteredRepos, err := libraries.ListRepos(reposFile)
+	if err != nil {
+		return fmt.Errorf("while filtering registry data file: %w", err)
+	}
+
+	if !reflect.DeepEqual(rawRepos, filteredRepos) {
+		return errors.New("registry data file contains duplicate URLs")
+	}
+
+	validTypes := map[string]bool{
+		"Arduino":     true,
+		"Contributed": true,
+		"Partner":     true,
+		"Recommended": true,
+		"Retired":     true,
+	}
+
+	nameMap := make(map[string]bool)
+	for _, entry := range rawRepos {
+		// Check entry types
+		if len(entry.Types) == 0 {
+			return fmt.Errorf("type not specified for library '%s'", entry.LibraryName)
+		}
+		for _, entryType := range entry.Types {
+			if _, valid := validTypes[entryType]; !valid {
+				return fmt.Errorf("invalid type '%s' used by library '%s'", entryType, entry.LibraryName)
+			}
+		}
+
+		// Check library name of the entry
+		if _, found := nameMap[entry.LibraryName]; found {
+			return fmt.Errorf("registry data file contains duplicates of name '%s'", entry.LibraryName)
+		}
+		nameMap[entry.LibraryName] = true
+	}
+	return nil
+}

internal/command/check-registry/check-registry_test.go

@@ -0,0 +1,60 @@
+// This file is part of libraries-repository-engine.
+//
+// Copyright 2025 ARDUINO SA (http://www.arduino.cc/)
+//
+// This program is free software: you can redistribute it and/or modify
+// it under the terms of the GNU Affero General Public License as published
+// by the Free Software Foundation, either version 3 of the License, or
+// (at your option) any later version.
+//
+// This program is distributed in the hope that it will be useful,
+// but WITHOUT ANY WARRANTY; without even the implied warranty of
+// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+// GNU Affero General Public License for more details.
+//
+// You should have received a copy of the GNU Affero General Public License
+// along with this program.  If not, see <https://www.gnu.org/licenses/>.
+//
+// You can be released from the requirements of the above licenses by purchasing
+// a commercial license. Buying such a license is mandatory if you want to
+// modify or otherwise use the software for commercial activities involving the
+// Arduino software without disclosing the source code of your own applications.
+// To purchase a commercial license, send an email to [email protected].
+
+package checkregistry
+
+import (
+	"path/filepath"
+	"testing"
+
+	"github.com/stretchr/testify/require"
+)
+
+func TestRegistryValidation(t *testing.T) {
+	type testcase struct {
+		Name           string
+		TestFile       string
+		ExpectedResult string
+	}
+	tests := []testcase{
+		{"EmptyArg", "", "registry data file argument testdata is a folder, not a file"},
+		{"NonExistentFile", "nonexistent.txt", "while loading registry data file: stat testdata/nonexistent.txt: no such file or directory"},
+		{"InvalidDataFormat", "invalid-data-format.txt", "while loading registry data file: invalid line format (3 fields are required): https://github.com/arduino-libraries/SD.git|Partner;SD"},
+		{"InvalidUrlFormat", "invalid-url-format.txt", "while filtering registry data file: Following URL are unknown or unsupported git repos:\nhttps://github.com/arduino-libraries/SD"},
+		{"MissingType", "no-type.txt", "invalid type '' used by library 'SD'"},
+		{"InvalidType", "invalid-type.txt", "invalid type 'foo' used by library 'SD'"},
+		{"DuplicateRepoURL", "duplicate-url.txt", "registry data file contains duplicate URLs"},
+		{"DuplicateLibName", "duplicate-name.txt", "registry data file contains duplicates of name 'SD'"},
+		{"ValidList", "valid.txt", ""},
+	}
+	for _, test := range tests {
+		t.Run(test.Name, func(t *testing.T) {
+			err := runcheck(filepath.Join("testdata", test.TestFile))
+			if test.ExpectedResult == "" {
+				require.NoError(t, err)
+			} else {
+				require.EqualError(t, err, test.ExpectedResult)
+			}
+		})
+	}
+}

internal/command/check-registry/testdata/duplicate-name.txt

@@ -0,0 +1,4 @@
+https://github.com/arduino-libraries/Scheduler.git|Arduino|Scheduler
+https://github.com/arduino-libraries/SD.git|Partner|SD
+https://github.com/arduino-libraries/Servo.git|Recommended|Servo
+https://github.com/arduino-libraries/Foo.git|Contributed|SD

internal/command/check-registry/testdata/duplicate-url.txt

@@ -0,0 +1,4 @@
+https://github.com/arduino-libraries/Scheduler.git|Arduino|Scheduler
+https://github.com/arduino-libraries/SD.git|Partner|SD
+https://github.com/arduino-libraries/Servo.git|Recommended|Servo
+https://github.com/arduino-libraries/SD.git|Contributed|Foo

internal/command/check-registry/testdata/invalid-data-format.txt

@@ -0,0 +1,3 @@
+https://github.com/arduino-libraries/Scheduler.git|Arduino|Scheduler
+https://github.com/arduino-libraries/SD.git|Partner;SD
+https://github.com/arduino-libraries/Servo.git|Recommended|Servo

internal/command/check-registry/testdata/invalid-type.txt

@@ -0,0 +1,3 @@
+https://github.com/arduino-libraries/Scheduler.git|Arduino|Scheduler
+https://github.com/arduino-libraries/SD.git|foo|SD
+https://github.com/arduino-libraries/Servo.git|Recommended|Servo

internal/command/check-registry/testdata/invalid-url-format.txt

@@ -0,0 +1,3 @@
+https://github.com/arduino-libraries/Scheduler.git|Arduino|Scheduler
+https://github.com/arduino-libraries/SD|Partner|SD
+https://github.com/arduino-libraries/Servo.git|Recommended|Servo

internal/command/check-registry/testdata/no-type.txt

@@ -0,0 +1,3 @@
+https://github.com/arduino-libraries/Scheduler.git|Arduino|Scheduler
+https://github.com/arduino-libraries/SD.git||SD
+https://github.com/arduino-libraries/Servo.git|Recommended|Servo

internal/command/check-registry/testdata/valid.txt

@@ -0,0 +1,3 @@
+https://github.com/arduino-libraries/Scheduler.git|Arduino|Scheduler
+https://github.com/arduino-libraries/SD.git|Partner|SD
+https://github.com/arduino-libraries/Servo.git|Recommended|Servo

internal/command/modify/modify.go

@@ -37,7 +37,6 @@ import (
 	"github.com/arduino/libraries-repository-engine/internal/libraries/archive"
 	"github.com/arduino/libraries-repository-engine/internal/libraries/db"
 	"github.com/arduino/libraries-repository-engine/internal/libraries/metadata"
-
 	"github.com/spf13/cobra"
 	"github.com/spf13/pflag"
 )

internal/command/remove/remove.go

@@ -37,7 +37,6 @@ import (
 	"github.com/arduino/libraries-repository-engine/internal/libraries/archive"
 	"github.com/arduino/libraries-repository-engine/internal/libraries/db"
 	"github.com/arduino/libraries-repository-engine/internal/libraries/metadata"
-
 	"github.com/spf13/cobra"
 )
 

internal/command/sync/sync.go

@@ -27,8 +27,8 @@ package sync
 import (
 	"bytes"
 	"encoding/json"
+	"errors"
 	"fmt"
-	"io/ioutil"
 	"log"
 	"os"
 	"path/filepath"
@@ -61,7 +61,7 @@ func Run(command *cobra.Command, cliArguments []string) {
 	}
 
 	if len(cliArguments) > 1 {
-		feedback.LogError(fmt.Errorf("Multiple arguments are not supported"))
+		feedback.LogError(errors.New("multiple arguments are not supported"))
 		os.Exit(1)
 	}
 
@@ -102,7 +102,7 @@ func syncLibraries(reposFile string) {
 				syncLibrary(logger, repo, libraryDb)
 
 				// Output log to file
-				if err := outputLogFile(logger, repo, buffer); err != nil {
+				if err := outputLogFile(repo, buffer); err != nil {
 					logger.Printf("Error writing log file: %s", err.Error())
 				}
 
@@ -200,13 +200,13 @@ func syncLibraryTaggedRelease(logger *log.Logger, repo *libraries.Repository, ta
 	// Checkout desired tag
 	logger.Printf("Checking out tag: %s", tag.Name().Short())
 	if err := gitutils.CheckoutTag(repo.Repository, tag); err != nil {
-		return fmt.Errorf("Error checking out repo: %s", err)
+		return fmt.Errorf("error checking out repo: %s", err)
 	}
 
 	// Create library metadata from library.properties
 	library, err := libraries.GenerateLibraryFromRepo(repo)
 	if err != nil {
-		return fmt.Errorf("Error generating library from repo: %s", err)
+		return fmt.Errorf("error generating library from repo: %s", err)
 	}
 	library.Types = repoMeta.Types
 
@@ -257,10 +257,10 @@ func syncLibraryTaggedRelease(logger *log.Logger, repo *libraries.Repository, ta
 
 	archiveData, err := archive.New(repo, library, config)
 	if err != nil {
-		return fmt.Errorf("Error while configuring library release archive: %s", err)
+		return fmt.Errorf("error while configuring library release archive: %s", err)
 	}
 	if err := archiveData.Create(); err != nil {
-		return fmt.Errorf("Error while zipping library: %s", err)
+		return fmt.Errorf("error while zipping library: %s", err)
 	}
 
 	release := db.FromLibraryToRelease(library)
@@ -271,13 +271,13 @@ func syncLibraryTaggedRelease(logger *log.Logger, repo *libraries.Repository, ta
 	release.Log = releaseLog
 
 	if err := libraries.UpdateLibrary(release, repo.URL, libraryDb); err != nil {
-		return fmt.Errorf("Error while updating library DB: %s", err)
+		return fmt.Errorf("error while updating library DB: %s", err)
 	}
 
 	return nil
 }
 
-func outputLogFile(logger *log.Logger, repoMetadata *libraries.Repo, buffer *bytes.Buffer) error {
+func outputLogFile(repoMetadata *libraries.Repo, buffer *bytes.Buffer) error {
 	if config.LogsFolder == "" {
 		return nil
 	}
@@ -294,7 +294,7 @@ func outputLogFile(logger *log.Logger, repoMetadata *libraries.Repo, buffer *byt
 	}
 	logFile := filepath.Join(logFolder, "index.html")
 	output := "<pre>\n" + buffer.String() + "\n</pre>"
-	if err := ioutil.WriteFile(logFile, []byte(output), 0644); err != nil {
+	if err := os.WriteFile(logFile, []byte(output), 0644); err != nil {
 		return fmt.Errorf("write log to file: %s", err.Error())
 	}
 	return nil

internal/libraries/clamav.go

@@ -30,43 +30,19 @@ import (
 	"strings"
 )
 
-func envSliceToMap(env []string) map[string]string {
-	envMap := make(map[string]string)
-	for _, value := range env {
-		key := value[:strings.Index(value, "=")]
-		value = value[strings.Index(value, "=")+1:]
-		envMap[key] = value
-	}
-	return envMap
-}
-
-func envMapToSlice(envMap map[string]string) []string {
-	var env []string
-	for key, value := range envMap {
-		env = append(env, key+"="+value)
-	}
-	return env
-}
-
-func modifyEnv(env []string, key, value string) []string {
-	envMap := envSliceToMap(env)
-	envMap[key] = value
-	return envMapToSlice(envMap)
-}
-
 // RunAntiVirus scans the folder for viruses.
 func RunAntiVirus(folder string) ([]byte, error) {
 	cmd := exec.Command("clamdscan", "--fdpass", "-i", folder)
-	cmd.Env = modifyEnv(os.Environ(), "LANG", "en")
+	cmd.Env = append(os.Environ(), "LANG=en")
 
 	out, err := cmd.CombinedOutput()
 	if err != nil {
 		return out, err
 	}
 
 	output := string(out)
-	if strings.Index(output, "Infected files: 0") == -1 {
-		return out, errors.New("Infected files found")
+	if !strings.Contains(output, "Infected files: 0") {
+		return out, errors.New("infected files found")
 	}
 
 	return out, nil