From 4bdcc045fefa9456c08833a896a009492af96da1 Mon Sep 17 00:00:00 2001 From: per1234 Date: Thu, 19 Aug 2021 14:49:09 -0700 Subject: [PATCH 1/4] Make dedicated package for release archive handling This is the initial minimal refactoring for what will eventually be a more significant package. --- internal/command/sync/sync.go | 5 +++-- internal/libraries/{repoarchive.go => archive/archive.go} | 3 ++- internal/libraries/git_integration_test.go | 5 +++-- 3 files changed, 8 insertions(+), 5 deletions(-) rename internal/libraries/{repoarchive.go => archive/archive.go} (96%) diff --git a/internal/command/sync/sync.go b/internal/command/sync/sync.go index 4eed3072..ee5ca97b 100644 --- a/internal/command/sync/sync.go +++ b/internal/command/sync/sync.go @@ -37,6 +37,7 @@ import ( "github.com/arduino/libraries-repository-engine/internal/configuration" "github.com/arduino/libraries-repository-engine/internal/feedback" "github.com/arduino/libraries-repository-engine/internal/libraries" + "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/gitutils" "github.com/arduino/libraries-repository-engine/internal/libraries/hash" @@ -268,10 +269,10 @@ func syncLibraryTaggedRelease(logger *log.Logger, repo *libraries.Repository, ta releaseLog += formattedReport } - zipName := libraries.ZipFolderName(library) + zipName := archive.ZipFolderName(library) lib := filepath.Base(filepath.Clean(filepath.Join(repo.FolderPath, ".."))) host := filepath.Base(filepath.Clean(filepath.Join(repo.FolderPath, "..", ".."))) - zipFilePath, err := libraries.ZipRepo(repo.FolderPath, filepath.Join(config.LibrariesFolder, host, lib), zipName) + zipFilePath, err := archive.ZipRepo(repo.FolderPath, filepath.Join(config.LibrariesFolder, host, lib), zipName) if err != nil { return fmt.Errorf("Error while zipping library: %s", err) } diff --git a/internal/libraries/repoarchive.go b/internal/libraries/archive/archive.go similarity index 96% rename from internal/libraries/repoarchive.go rename to internal/libraries/archive/archive.go index ce9eeae5..9ea23607 100644 --- a/internal/libraries/repoarchive.go +++ b/internal/libraries/archive/archive.go @@ -21,7 +21,8 @@ // Arduino software without disclosing the source code of your own applications. // To purchase a commercial license, send an email to license@arduino.cc. -package libraries +// Package archive handles the library release archive. +package archive import ( "os" diff --git a/internal/libraries/git_integration_test.go b/internal/libraries/git_integration_test.go index a17748f7..d341707f 100644 --- a/internal/libraries/git_integration_test.go +++ b/internal/libraries/git_integration_test.go @@ -29,6 +29,7 @@ import ( "path/filepath" "testing" + "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/gitutils" "github.com/stretchr/testify/require" @@ -68,11 +69,11 @@ func TestUpdateLibraryJson(t *testing.T) { require.NoError(t, err) require.NotNil(t, library) - zipFolderName := ZipFolderName(library) + zipFolderName := archive.ZipFolderName(library) release := db.FromLibraryToRelease(library) - zipFilePath, err := ZipRepo(r.FolderPath, librariesRepo, zipFolderName) + zipFilePath, err := archive.ZipRepo(r.FolderPath, librariesRepo, zipFolderName) require.NoError(t, err) require.NotEmpty(t, zipFilePath) From 0027587209c53060f9a1f4dfc93bf50e2ed574ca Mon Sep 17 00:00:00 2001 From: per1234 Date: Thu, 19 Aug 2021 14:51:21 -0700 Subject: [PATCH 2/4] Move archive size/checksum code to archive package --- internal/command/sync/sync.go | 19 +------------------ internal/libraries/archive/archive.go | 18 ++++++++++++++++++ 2 files changed, 19 insertions(+), 18 deletions(-) diff --git a/internal/command/sync/sync.go b/internal/command/sync/sync.go index ee5ca97b..0f445c1f 100644 --- a/internal/command/sync/sync.go +++ b/internal/command/sync/sync.go @@ -40,7 +40,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/gitutils" - "github.com/arduino/libraries-repository-engine/internal/libraries/hash" "github.com/go-git/go-git/v5/plumbing" "github.com/spf13/cobra" ) @@ -277,7 +276,7 @@ func syncLibraryTaggedRelease(logger *log.Logger, repo *libraries.Repository, ta return fmt.Errorf("Error while zipping library: %s", err) } - size, checksum, err := getSizeAndCalculateChecksum(zipFilePath) + size, checksum, err := archive.GetSizeAndCalculateChecksum(zipFilePath) if err != nil { return fmt.Errorf("Error while calculating checksums: %s", err) } @@ -295,22 +294,6 @@ func syncLibraryTaggedRelease(logger *log.Logger, repo *libraries.Repository, ta return nil } -func getSizeAndCalculateChecksum(filePath string) (int64, string, error) { - info, err := os.Stat(filePath) - if err != nil { - return -1, "", err - } - - size := info.Size() - - checksum, err := hash.Checksum(filePath) - if err != nil { - return -1, "", err - } - - return size, checksum, nil -} - func outputLogFile(logger *log.Logger, repoMetadata *libraries.Repo, buffer *bytes.Buffer) error { if config.LogsFolder == "" { return nil diff --git a/internal/libraries/archive/archive.go b/internal/libraries/archive/archive.go index 9ea23607..2c1d0021 100644 --- a/internal/libraries/archive/archive.go +++ b/internal/libraries/archive/archive.go @@ -29,6 +29,7 @@ import ( "path/filepath" "regexp" + "github.com/arduino/libraries-repository-engine/internal/libraries/hash" "github.com/arduino/libraries-repository-engine/internal/libraries/metadata" "github.com/arduino/libraries-repository-engine/internal/libraries/zip" ) @@ -53,3 +54,20 @@ func ZipFolderName(library *metadata.LibraryMetadata) string { pattern := regexp.MustCompile("[^a-zA-Z0-9]") return pattern.ReplaceAllString(library.Name, "_") + "-" + library.Version } + +// GetSizeAndCalculateChecksum returns the size and SHA-256 checksum for the given file. +func GetSizeAndCalculateChecksum(filePath string) (int64, string, error) { + info, err := os.Stat(filePath) + if err != nil { + return -1, "", err + } + + size := info.Size() + + checksum, err := hash.Checksum(filePath) + if err != nil { + return -1, "", err + } + + return size, checksum, nil +} From d0f3044b1c06fe900c6c98f76ba6676fe4f1a886 Mon Sep 17 00:00:00 2001 From: per1234 Date: Fri, 20 Aug 2021 00:20:15 -0700 Subject: [PATCH 3/4] Make `archive` package object oriented The change resulted in an import cycle in `internal/libraries/git_integration_test.go`. I changed its package name as a workaround. --- internal/command/sync/sync.go | 20 ++--- internal/libraries/archive/archive.go | 71 ++++++++++++--- internal/libraries/archive/archive_test.go | 88 +++++++++++++++++++ .../gitclones/SomeRepository/SomeFile | 2 + internal/libraries/git_integration_test.go | 20 +++-- 5 files changed, 168 insertions(+), 33 deletions(-) create mode 100644 internal/libraries/archive/archive_test.go create mode 100644 internal/libraries/archive/testdata/gitclones/SomeRepository/SomeFile diff --git a/internal/command/sync/sync.go b/internal/command/sync/sync.go index 0f445c1f..e190bb96 100644 --- a/internal/command/sync/sync.go +++ b/internal/command/sync/sync.go @@ -268,23 +268,19 @@ func syncLibraryTaggedRelease(logger *log.Logger, repo *libraries.Repository, ta releaseLog += formattedReport } - zipName := archive.ZipFolderName(library) - lib := filepath.Base(filepath.Clean(filepath.Join(repo.FolderPath, ".."))) - host := filepath.Base(filepath.Clean(filepath.Join(repo.FolderPath, "..", ".."))) - zipFilePath, err := archive.ZipRepo(repo.FolderPath, filepath.Join(config.LibrariesFolder, host, lib), zipName) + archiveData, err := archive.New(repo, library, config) if err != nil { + 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) } - size, checksum, err := archive.GetSizeAndCalculateChecksum(zipFilePath) - if err != nil { - return fmt.Errorf("Error while calculating checksums: %s", err) - } release := db.FromLibraryToRelease(library) - release.URL = config.BaseDownloadURL + host + "/" + lib + "/" + zipName + ".zip" - release.ArchiveFileName = zipName + ".zip" - release.Size = size - release.Checksum = checksum + release.URL = archiveData.URL + release.ArchiveFileName = archiveData.FileName + release.Size = archiveData.Size + release.Checksum = archiveData.Checksum release.Log = releaseLog if err := libraries.UpdateLibrary(release, repo.URL, libraryDb); err != nil { diff --git a/internal/libraries/archive/archive.go b/internal/libraries/archive/archive.go index 2c1d0021..9e6fd7f1 100644 --- a/internal/libraries/archive/archive.go +++ b/internal/libraries/archive/archive.go @@ -25,38 +25,83 @@ package archive import ( + "net/url" "os" "path/filepath" "regexp" + "github.com/arduino/libraries-repository-engine/internal/configuration" + "github.com/arduino/libraries-repository-engine/internal/libraries" "github.com/arduino/libraries-repository-engine/internal/libraries/hash" "github.com/arduino/libraries-repository-engine/internal/libraries/metadata" "github.com/arduino/libraries-repository-engine/internal/libraries/zip" ) -// ZipRepo creates a ZIP archive of the repo folder and returns its path. -func ZipRepo(repoFolder string, baseFolder string, zipFolderName string) (string, error) { - err := os.MkdirAll(baseFolder, os.FileMode(0755)) +// Archive is the type for library release archive data. +type Archive struct { + SourcePath string + RootName string // Name of the root folder inside the archive. + FileName string + Path string // Full path of the archive. + URL string // URL the archive will have on the download server. + Size int64 + Checksum string +} + +// New initializes and returns an Archive object. +func New(repository *libraries.Repository, libraryMetadata *metadata.LibraryMetadata, config *configuration.Config) (*Archive, error) { + repositoryURLData, err := url.Parse(repository.URL) + if err != nil { + return nil, err + } + // e.g., https://github.com/arduino-libraries/Servo.git -> github.com + repositoryHost := repositoryURLData.Host + // e.g., https://github.com/arduino-libraries/Servo.git -> arduino-libraries + repositoryParent := filepath.Base(filepath.Dir(repositoryURLData.Path)) + + // Unlike the other path components, the filename is based on library name, not repository name URL. + fileName := zipFolderName(libraryMetadata) + ".zip" + + return &Archive{ + SourcePath: repository.FolderPath, + RootName: zipFolderName(libraryMetadata), + FileName: fileName, + Path: filepath.Join(config.LibrariesFolder, repositoryHost, repositoryParent, fileName), + URL: config.BaseDownloadURL + repositoryHost + "/" + repositoryParent + "/" + fileName, + }, nil +} + +// Create makes an archive file according to the data of the Archive object and updates the object with the size and +// checksum for the resulting file. +func (archive *Archive) Create() error { + err := os.MkdirAll(filepath.Dir(archive.Path), os.FileMode(0755)) if err != nil { - return "", err + return err + } + + if err := zip.Directory(archive.SourcePath, archive.RootName, archive.Path); err != nil { + os.Remove(archive.Path) + return err } - absoluteFileName := filepath.Join(baseFolder, zipFolderName+".zip") - if err := zip.Directory(repoFolder, zipFolderName, absoluteFileName); err != nil { - os.Remove(absoluteFileName) - return "", err + + size, checksum, err := getSizeAndCalculateChecksum(archive.Path) + if err != nil { + return err } + archive.Size = size + archive.Checksum = checksum - return absoluteFileName, nil + return nil } -// ZipFolderName returns the name to use for the folder. -func ZipFolderName(library *metadata.LibraryMetadata) string { +// zipFolderName returns the name to use for the folder. +func zipFolderName(library *metadata.LibraryMetadata) string { pattern := regexp.MustCompile("[^a-zA-Z0-9]") return pattern.ReplaceAllString(library.Name, "_") + "-" + library.Version } -// GetSizeAndCalculateChecksum returns the size and SHA-256 checksum for the given file. -func GetSizeAndCalculateChecksum(filePath string) (int64, string, error) { +// getSizeAndCalculateChecksum returns the size and SHA-256 checksum for the given file. +func getSizeAndCalculateChecksum(filePath string) (int64, string, error) { info, err := os.Stat(filePath) if err != nil { return -1, "", err diff --git a/internal/libraries/archive/archive_test.go b/internal/libraries/archive/archive_test.go new file mode 100644 index 00000000..109b6954 --- /dev/null +++ b/internal/libraries/archive/archive_test.go @@ -0,0 +1,88 @@ +// This file is part of libraries-repository-engine. +// +// Copyright 2021 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 . +// +// 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 license@arduino.cc. + +package archive + +import ( + "os" + "path/filepath" + "testing" + + "github.com/arduino/libraries-repository-engine/internal/configuration" + "github.com/arduino/libraries-repository-engine/internal/libraries" + "github.com/arduino/libraries-repository-engine/internal/libraries/metadata" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +var testDataPath string + +func init() { + workingDirectory, err := os.Getwd() + if err != nil { + panic(err) + } + testDataPath = filepath.Join(workingDirectory, "testdata") +} + +func TestNew(t *testing.T) { + repository := libraries.Repository{ + FolderPath: "/qux/repos/some-repo", + URL: "https://github.com/Foo/Bar.git", + } + libraryMetadata := metadata.LibraryMetadata{ + Name: "Foo Bar", + Version: "1.2.3", + } + config := configuration.Config{ + LibrariesFolder: "/baz/libs/", + BaseDownloadURL: "https://example/com/libraries/", + } + + archiveObject, err := New(&repository, &libraryMetadata, &config) + require.NoError(t, err) + assert.Equal(t, "/qux/repos/some-repo", archiveObject.SourcePath) + assert.Equal(t, "Foo_Bar-1.2.3", archiveObject.RootName) + assert.Equal(t, "Foo_Bar-1.2.3.zip", archiveObject.FileName) + assert.Equal(t, filepath.Join("/baz/libs/github.com/Foo/Foo_Bar-1.2.3.zip"), archiveObject.Path) + assert.Equal(t, "https://example/com/libraries/github.com/Foo/Foo_Bar-1.2.3.zip", archiveObject.URL) +} + +func TestCreate(t *testing.T) { + archiveDir := filepath.Join(os.TempDir(), "TestCreateArchiveDir") + defer os.RemoveAll(archiveDir) + archivePath := filepath.Join(archiveDir, "TestCreateArchive.zip") + + archiveObject := Archive{ + Path: archivePath, + SourcePath: filepath.Join(testDataPath, "gitclones", "SomeRepository"), + RootName: "SomeLibrary", + } + + err := archiveObject.Create() + require.NoError(t, err, "This test must be run as administrator on Windows to have symlink creation privilege.") + + assert.FileExists(t, archivePath) + assert.Greater(t, archiveObject.Size, int64(0)) + assert.NotEmpty(t, archiveObject.Checksum) +} diff --git a/internal/libraries/archive/testdata/gitclones/SomeRepository/SomeFile b/internal/libraries/archive/testdata/gitclones/SomeRepository/SomeFile new file mode 100644 index 00000000..4a647066 --- /dev/null +++ b/internal/libraries/archive/testdata/gitclones/SomeRepository/SomeFile @@ -0,0 +1,2 @@ +This is some arbitrary source content to stand in for a library release. It is not intended to be a Git repository +because that is not convenient to have in the repository and not relevant to testing this package. diff --git a/internal/libraries/git_integration_test.go b/internal/libraries/git_integration_test.go index d341707f..b0f1ef74 100644 --- a/internal/libraries/git_integration_test.go +++ b/internal/libraries/git_integration_test.go @@ -21,7 +21,7 @@ // Arduino software without disclosing the source code of your own applications. // To purchase a commercial license, send an email to license@arduino.cc. -package libraries +package libraries_test import ( "io/ioutil" @@ -29,6 +29,8 @@ import ( "path/filepath" "testing" + "github.com/arduino/libraries-repository-engine/internal/configuration" + "github.com/arduino/libraries-repository-engine/internal/libraries" "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/gitutils" @@ -36,7 +38,7 @@ import ( ) func TestUpdateLibraryJson(t *testing.T) { - repos, err := ListRepos("./testdata/git_test_repo.txt") + repos, err := libraries.ListRepos("./testdata/git_test_repo.txt") require.NoError(t, err) require.NotNil(t, repos) @@ -52,7 +54,7 @@ func TestUpdateLibraryJson(t *testing.T) { subfolder, err := repo.AsFolder() require.NoError(t, err) - r, err := CloneOrFetch(repo, filepath.Join("/tmp", subfolder)) + r, err := libraries.CloneOrFetch(repo, filepath.Join("/tmp", subfolder)) require.NoError(t, err) require.NotNil(t, r) @@ -65,19 +67,21 @@ func TestUpdateLibraryJson(t *testing.T) { err = gitutils.CheckoutTag(r.Repository, tag) - library, err := GenerateLibraryFromRepo(r) + library, err := libraries.GenerateLibraryFromRepo(r) require.NoError(t, err) require.NotNil(t, library) - zipFolderName := archive.ZipFolderName(library) + config := configuration.Config{LibrariesFolder: librariesRepo} + archiveData, err := archive.New(r, library, &config) + require.NoError(t, err) release := db.FromLibraryToRelease(library) - zipFilePath, err := archive.ZipRepo(r.FolderPath, librariesRepo, zipFolderName) + err = archiveData.Create() require.NoError(t, err) - require.NotEmpty(t, zipFilePath) + require.NotEmpty(t, archiveData.Path) - err = UpdateLibrary(release, r.URL, libraryDb) + err = libraries.UpdateLibrary(release, r.URL, libraryDb) require.NoError(t, err) } From 7434cd70e41e6003fabf3e2d2575cfd7e9377449 Mon Sep 17 00:00:00 2001 From: per1234 Date: Wed, 25 Aug 2021 02:21:39 -0700 Subject: [PATCH 4/4] Avoid redundant compilation of archive name regex Previously, the compilation of the regular expression used to convert the library name to the release archive root folder and file name was done inside the function, which caused it to be compiled redundantly for every subsequent call. Moving it to the package scope improves the function's efficiency. --- internal/libraries/archive/archive.go | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/internal/libraries/archive/archive.go b/internal/libraries/archive/archive.go index 9e6fd7f1..5a1bceed 100644 --- a/internal/libraries/archive/archive.go +++ b/internal/libraries/archive/archive.go @@ -94,10 +94,11 @@ func (archive *Archive) Create() error { return nil } +var zipFolderNamePattern = regexp.MustCompile("[^a-zA-Z0-9]") + // zipFolderName returns the name to use for the folder. func zipFolderName(library *metadata.LibraryMetadata) string { - pattern := regexp.MustCompile("[^a-zA-Z0-9]") - return pattern.ReplaceAllString(library.Name, "_") + "-" + library.Version + return zipFolderNamePattern.ReplaceAllString(library.Name, "_") + "-" + library.Version } // getSizeAndCalculateChecksum returns the size and SHA-256 checksum for the given file.