From fe1daf0f01a087fc11275f9db42bfb0d2f94958b Mon Sep 17 00:00:00 2001 From: Silvano Cerza Date: Wed, 20 Jan 2021 16:34:57 +0100 Subject: [PATCH] Enhance lib install from git url and zip file Previously installing from a zip file would extract the content directly into the user library folder, this could have caused problem if the zip file wasn't structured correctly. To handle this we now extract to a temp directory to verify that the zip is structured correctly and to infer the name of the library being installed. If everything is fine we then copy it over to the user library folder to finalize the installation. Installing from a git repository has been enhanced too, after cloning the repository to the user library folder we delete the .git folder to make it a plain folder. Cloning should be faster too since we now clone using depth of one commit to avoid downloading useless files. Both when installing from a zip file or from a git repository we delete the installed library folder if one with the same name is already installed. --- arduino/libraries/librariesmanager/install.go | 76 ++++++++++++++++++- test/test_lib.py | 67 ++++++++++++++-- 2 files changed, 136 insertions(+), 7 deletions(-) diff --git a/arduino/libraries/librariesmanager/install.go b/arduino/libraries/librariesmanager/install.go index bcf13345050..4e69eb9323f 100644 --- a/arduino/libraries/librariesmanager/install.go +++ b/arduino/libraries/librariesmanager/install.go @@ -28,6 +28,7 @@ import ( "github.com/arduino/arduino-cli/arduino/utils" paths "github.com/arduino/go-paths-helper" "github.com/codeclysm/extract/v3" + "github.com/sirupsen/logrus" "gopkg.in/src-d/go-git.v4" ) @@ -99,15 +100,62 @@ func (lm *LibrariesManager) InstallZipLib(ctx context.Context, archivePath strin return fmt.Errorf("User directory not set") } + tmpDir, err := paths.MkTempDir(paths.TempDir().String(), "arduino-cli-lib-") + if err != nil { + return err + } + file, err := os.Open(archivePath) if err != nil { return err } defer file.Close() - if err := extract.Archive(ctx, file, libsDir.String(), nil); err != nil { - return fmt.Errorf("extracting archive: %s", err) + // Extract to a temporary directory so we can check if the zip is structured correctly. + // We also use the top level folder from the archive to infer the library name. + if err := extract.Archive(ctx, file, tmpDir.String(), nil); err != nil { + return fmt.Errorf("extracting archive: %w", err) + } + + paths, err := tmpDir.ReadDir() + if err != nil { + return err + } + + if len(paths) > 1 { + return fmt.Errorf("archive is not valid: multiple files found in zip file top level") + } + + libraryName := paths[0].Base() + installPath := libsDir.Join(libraryName) + + // Deletes libraries folder if already installed + if _, ok := lm.Libraries[libraryName]; ok { + logrus. + WithField("library name", libraryName). + WithField("install path", installPath). + Trace("Deleting library") + installPath.RemoveAll() } + + logrus. + WithField("library name", libraryName). + WithField("install path", installPath). + WithField("zip file", archivePath). + Trace("Installing library") + + files, err := tmpDir.Join(libraryName).ReadDirRecursive() + files.FilterOutDirs() + for _, f := range files { + finalPath := installPath.Join(f.Base()) + if err := finalPath.Parent().MkdirAll(); err != nil { + return fmt.Errorf("creating directory: %w", err) + } + if err := f.CopyTo(finalPath); err != nil { + return fmt.Errorf("copying library: %w", err) + } + } + return nil } @@ -120,18 +168,42 @@ func (lm *LibrariesManager) InstallGitLib(gitURL string) error { libraryName, err := parseGitURL(gitURL) if err != nil { + logrus. + WithError(err). + Warn("Parsing git URL") return err } installPath := libsDir.Join(libraryName) + // Deletes libraries folder if already installed + if _, ok := lm.Libraries[libraryName]; ok { + logrus. + WithField("library name", libraryName). + WithField("install path", installPath). + Trace("Deleting library") + installPath.RemoveAll() + } + + logrus. + WithField("library name", libraryName). + WithField("install path", installPath). + WithField("git url", gitURL). + Trace("Installing library") + _, err = git.PlainClone(installPath.String(), false, &git.CloneOptions{ URL: gitURL, + Depth: 1, Progress: os.Stdout, }) if err != nil { + logrus. + WithError(err). + Warn("Cloning git repository") return err } + // We don't want the installed library to be a git repository thus we delete this folder + installPath.Join(".git").RemoveAll() return nil } diff --git a/test/test_lib.py b/test/test_lib.py index 2464df84061..6dfe0495dde 100644 --- a/test/test_lib.py +++ b/test/test_lib.py @@ -225,15 +225,25 @@ def test_install_with_git_url(run_command, data_dir, downloads_dir): } assert run_command("config init --dest-dir .", custom_env=env) + lib_install_dir = Path(data_dir, "libraries", "WiFi101") + # Verifies library is not already installed + assert not lib_install_dir.exists() + + git_url = "https://github.com/arduino-libraries/WiFi101.git" + # Test git-url library install - res = run_command("lib install --git-url https://github.com/arduino-libraries/WiFi101.git") + res = run_command(f"lib install --git-url {git_url}") assert res.ok assert "--git-url and --zip-path flags allow installing untrusted files, use it at your own risk." in res.stdout - # Test failing-install as repository already exists - res = run_command("lib install --git-url https://github.com/arduino-libraries/WiFi101.git") - assert "--git-url and --zip-path flags allow installing untrusted files, use it at your own risk." in res.stdout - assert "Error installing Git Library: repository already exists" in res.stderr + # Verifies library is installed in expected path + assert lib_install_dir.exists() + + # Reinstall library + assert run_command(f"lib install --git-url {git_url}") + + # Verifies library remains installed + assert lib_install_dir.exists() def test_install_with_zip_path(run_command, data_dir, downloads_dir): @@ -249,12 +259,25 @@ def test_install_with_zip_path(run_command, data_dir, downloads_dir): # Download a specific lib version assert run_command("lib download AudioZero@1.0.0") + lib_install_dir = Path(data_dir, "libraries", "AudioZero-1.0.0") + # Verifies library is not already installed + assert not lib_install_dir.exists() + zip_path = Path(downloads_dir, "libraries", "AudioZero-1.0.0.zip") # Test zip-path install res = run_command(f"lib install --zip-path {zip_path}") assert res.ok assert "--git-url and --zip-path flags allow installing untrusted files, use it at your own risk." in res.stdout + # Verifies library is installed in expected path + assert lib_install_dir.exists() + + # Reinstall library + assert run_command(f"lib install --zip-path {zip_path}") + + # Verifies library remains installed + assert lib_install_dir.exists() + def test_update_index(run_command): result = run_command("lib update-index") @@ -440,6 +463,9 @@ def test_install_with_git_url_local_file_uri(run_command, downloads_dir, data_di assert run_command(f"lib install --git-url {repo_dir.as_uri()}", custom_env=env) + # Verifies library is installed + assert lib_install_dir.exists() + def test_install_with_git_local_url(run_command, downloads_dir, data_dir): assert run_command("update") @@ -462,6 +488,9 @@ def test_install_with_git_local_url(run_command, downloads_dir, data_dir): assert run_command(f"lib install --git-url {repo_dir}", custom_env=env) + # Verifies library is installed + assert lib_install_dir.exists() + def test_install_with_git_url_relative_path(run_command, downloads_dir, data_dir): assert run_command("update") @@ -483,3 +512,31 @@ def test_install_with_git_url_relative_path(run_command, downloads_dir, data_dir assert Repo.clone_from(git_url, repo_dir) assert run_command("lib install --git-url ./WiFi101", custom_working_dir=data_dir, custom_env=env) + + # Verifies library is installed + assert lib_install_dir.exists() + + +def test_install_with_git_url_does_not_create_git_repo(run_command, downloads_dir, data_dir): + assert run_command("update") + + env = { + "ARDUINO_DATA_DIR": data_dir, + "ARDUINO_DOWNLOADS_DIR": downloads_dir, + "ARDUINO_SKETCHBOOK_DIR": data_dir, + "ARDUINO_ENABLE_UNSAFE_LIBRARY_INSTALL": "true", + } + + lib_install_dir = Path(data_dir, "libraries", "WiFi101") + # Verifies library is not installed + assert not lib_install_dir.exists() + + # Clone repository locally + git_url = "https://github.com/arduino-libraries/WiFi101.git" + repo_dir = Path(data_dir, "WiFi101") + assert Repo.clone_from(git_url, repo_dir) + + assert run_command(f"lib install --git-url {repo_dir}", custom_env=env) + + # Verifies installed library is not a git repository + assert not Path(lib_install_dir, ".git").exists()