Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Some cleanups to the codebase #293

Merged
merged 10 commits into from
Jan 29, 2025
Merged
Show file tree
Hide file tree
Changes from 4 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 0 additions & 1 deletion internal/command/modify/modify.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
)
Expand Down
1 change: 0 additions & 1 deletion internal/command/remove/remove.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
)

Expand Down
20 changes: 10 additions & 10 deletions internal/command/sync/sync.go
Original file line number Diff line number Diff line change
Expand Up @@ -27,8 +27,8 @@ package sync
import (
"bytes"
"encoding/json"
"errors"
"fmt"
"io/ioutil"
"log"
"os"
"path/filepath"
Expand Down Expand Up @@ -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)
}

Expand Down Expand Up @@ -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())
}

Expand Down Expand Up @@ -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

Expand Down Expand Up @@ -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)
Expand All @@ -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
}
Expand All @@ -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
Expand Down
30 changes: 3 additions & 27 deletions internal/libraries/clamav.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
4 changes: 2 additions & 2 deletions internal/libraries/git_integration_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,6 @@
package libraries_test

import (
"io/ioutil"
"os"
"path/filepath"
"testing"
Expand All @@ -43,7 +42,7 @@ func TestUpdateLibraryJson(t *testing.T) {
require.NoError(t, err)
require.NotNil(t, repos)

librariesRepo, err := ioutil.TempDir("", "libraries")
librariesRepo, err := os.MkdirTemp("", "libraries")
require.NoError(t, err)
defer os.RemoveAll(librariesRepo)

Expand All @@ -66,6 +65,7 @@ func TestUpdateLibraryJson(t *testing.T) {
require.NoError(t, err)

err = gitutils.CheckoutTag(r.Repository, tag)
require.NoError(t, err)

library, err := libraries.GenerateLibraryFromRepo(r)
require.NoError(t, err)
Expand Down
5 changes: 3 additions & 2 deletions internal/libraries/lint.go
Original file line number Diff line number Diff line change
Expand Up @@ -72,8 +72,9 @@ func RunArduinoLint(arduinoLintPath string, folder string, metadata *Repo) ([]by
folder,
)
// See: https://arduino.github.io/arduino-lint/latest/#environment-variables
cmd.Env = modifyEnv(os.Environ(), "ARDUINO_LINT_LIBRARY_MANAGER_INDEXING", "true")
cmd.Env = modifyEnv(cmd.Env, "ARDUINO_LINT_OFFICIAL", fmt.Sprintf("%t", official(metadata)))
cmd.Env = append(os.Environ(),
"ARDUINO_LINT_LIBRARY_MANAGER_INDEXING=true",
fmt.Sprintf("ARDUINO_LINT_OFFICIAL=%t", official(metadata)))

textReport, lintErr := cmd.CombinedOutput()
if lintErr != nil {
Expand Down
7 changes: 3 additions & 4 deletions internal/libraries/repoclone.go
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,6 @@
package libraries

import (
"io/ioutil"
"os"
"path/filepath"

Expand Down Expand Up @@ -92,7 +91,7 @@ func CloneOrFetch(repoMeta *Repo, folderName string) (*Repository, error) {

// GenerateLibraryFromRepo parses a repository and returns the library metadata.
func GenerateLibraryFromRepo(repo *Repository) (*metadata.LibraryMetadata, error) {
bytes, err := ioutil.ReadFile(filepath.Join(repo.FolderPath, "library.properties"))
bytes, err := os.ReadFile(filepath.Join(repo.FolderPath, "library.properties"))
if err != nil {
return nil, fmt.Errorf("can't read library.properties: %s", err)
}
Expand Down Expand Up @@ -154,10 +153,10 @@ func BackupAndDeleteGitClone(config *configuration.Config, repoMeta *Repo) error
}
if gitClonePathExists {
if err := backup.Backup(gitClonePath); err != nil {
return fmt.Errorf("While backing up library's Git clone: %w", err)
return fmt.Errorf("backing up library's Git clone: %w", err)
}
if err := gitClonePath.RemoveAll(); err != nil {
return fmt.Errorf("While removing library Git clone: %s", err)
return fmt.Errorf("removing library Git clone: %s", err)
}
} else {
feedback.Warningf("Library Git clone folder %s not present", gitClonePath)
Expand Down
104 changes: 104 additions & 0 deletions internal/libraries/repolist_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -59,3 +59,107 @@ func TestRepoFolderPathDetermination(t *testing.T) {
require.NoError(t, err)
require.Equal(t, "bitbucket.org/bjoern/arduino_osc", f)
}

func TestLoadRepoListFromFile(t *testing.T) {
_, err := LoadRepoListFromFile("./testdata/nonexistent.txt")
assert.Error(t, err, "Attempt to load non-existent registry data file")

repos, err := LoadRepoListFromFile("./testdata/git_test_repos.txt")
require.NoError(t, err)

reposAssertion := []*Repo{
{
URL: "https://github.com/arduino-libraries",
Types: []string{"Arduino"},
LibraryName: "libraries",
},
{
URL: "[email protected]:PaulStoffregen/Audio.git",
Types: []string{"Contributed"},
LibraryName: "Audio",
},
{
URL: "https://github.com/PaulStoffregen/OctoWS2811.git",
Types: []string{"Arduino", "Contributed"},
LibraryName: "OctoWS2811",
},
{
URL: "https://github.com/PaulStoffregen/AltSoftSerial.git",
Types: []string{"Contributed"},
LibraryName: "AltSoftSerial",
},
{
URL: "https://github.com/Cheong2K/ble-sdk-arduino.git",
Types: []string{"Contributed"},
LibraryName: "ble-sdk-arduino",
},
{
URL: "https://github.com/arduino-libraries/Bridge.git",
Types: []string{"Contributed"},
LibraryName: "Bridge",
},
{
URL: "https://github.com/adafruit/Adafruit_ADS1X15.git",
Types: []string{"Recommended"},
LibraryName: "Adafruit_ADS1X15",
},
{
URL: "https://github.com/adafruit/Adafruit_ADXL345.git",
Types: []string{"Recommended"},
LibraryName: "Adafruit_ADXL345",
},
{
URL: "https://github.com/adafruit/Adafruit_AHRS.git",
Types: []string{"Recommended"},
LibraryName: "Adafruit_AHRS",
},
{
URL: "https://github.com/adafruit/Adafruit_AM2315.git",
Types: []string{"Recommended"},
LibraryName: "Adafruit_AM2315",
},
{
URL: "https://github.com/arduino-libraries/Scheduler.git",
Types: []string{"Arduino"},
LibraryName: "Scheduler",
},
{
URL: "https://github.com/arduino-libraries/SD.git",
Types: []string{"Arduino"},
LibraryName: "SD",
},
{
URL: "https://github.com/arduino-libraries/Servo.git",
Types: []string{"Arduino"},
LibraryName: "Servo",
},
}

assert.Equal(t, reposAssertion, repos)
}

func TestListRepos(t *testing.T) {
repos, err := ListRepos("testdata/git_test_repos.txt")
require.Error(t, err)

require.Equal(t, 11, len(repos))

require.Equal(t, "https://github.com/PaulStoffregen/OctoWS2811.git", repos[0].URL)
require.Equal(t, "https://github.com/PaulStoffregen/AltSoftSerial.git", repos[1].URL)

require.Equal(t, "https://github.com/Cheong2K/ble-sdk-arduino.git", repos[2].URL)
require.Equal(t, "https://github.com/arduino-libraries/Bridge.git", repos[3].URL)
require.Equal(t, "https://github.com/adafruit/Adafruit_ADS1X15.git", repos[4].URL)
require.Equal(t, "https://github.com/adafruit/Adafruit_ADXL345.git", repos[5].URL)
require.Equal(t, "https://github.com/adafruit/Adafruit_AHRS.git", repos[6].URL)
require.Equal(t, "https://github.com/adafruit/Adafruit_AM2315.git", repos[7].URL)
require.Equal(t, "https://github.com/arduino-libraries/Scheduler.git", repos[8].URL)
require.Equal(t, "https://github.com/arduino-libraries/SD.git", repos[9].URL)
require.Equal(t, "https://github.com/arduino-libraries/Servo.git", repos[10].URL)
require.Error(t, err)

error, ok := err.(GitURLsError)
require.True(t, ok)
require.Equal(t, "https://github.com/arduino-libraries", error.Repos[0].URL)
require.Equal(t, "[email protected]:PaulStoffregen/Audio.git", error.Repos[1].URL)
}
14 changes: 5 additions & 9 deletions internal/libraries/zip/ziphelper.go
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,6 @@ package zip

import (
"fmt"
"io/ioutil"
"os"
"os/exec"
"path/filepath"
Expand All @@ -37,15 +36,12 @@ import (
// Inside the archive "rootFolder" will be renamed to "zipRootFolderName".
func Directory(rootFolder string, zipRootFolderName string, zipFile string) error {
checks := func(path string, info os.FileInfo, err error) error {
info, err = os.Lstat(path)
if err != nil {
if lstat, err := os.Lstat(path); err != nil { // TODO: is calling Lstat here necessary? we already have the FileInfo from the function args
return err
}
if (info.Mode() & os.ModeSymlink) != 0 {
} else if (lstat.Mode() & os.ModeSymlink) != 0 {
dest, _ := os.Readlink(path)
return fmt.Errorf("Symlink not allowed: %s -> %s", path, dest)
}
if file.IsSCCS(info.Name()) {
return fmt.Errorf("symlink not allowed: %s -> %s", path, dest)
} else if file.IsSCCS(lstat.Name()) {
return filepath.SkipDir
}
return nil
Expand All @@ -60,7 +56,7 @@ func Directory(rootFolder string, zipRootFolderName string, zipFile string) erro
return err
}

tmpdir, err := ioutil.TempDir("", "ziphelper")
tmpdir, err := os.MkdirTemp("", "ziphelper")
if err != nil {
return fmt.Errorf("creating temp dir for zip archive: %s", err)
}
Expand Down
3 changes: 1 addition & 2 deletions internal/libraries/zip/ziphelper_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -25,15 +25,14 @@ package zip

import (
"archive/zip"
"io/ioutil"
"os"
"testing"

"github.com/stretchr/testify/require"
)

func TestZip(t *testing.T) {
zipFile, err := ioutil.TempFile("", "ziphelper*.zip")
zipFile, err := os.CreateTemp("", "ziphelper*.zip")
require.NoError(t, err)
require.NotNil(t, zipFile)
zipFileName := zipFile.Name()
Expand Down
Loading
Loading