Enhance lib install from git url and zip file (#1145)

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.

Author: silvanocerza

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
 }
 

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 [email protected]")
 
+    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()