Abstract away some code repetition

gap-packages · Sep 9, 2024 · e4324af · e4324af
1 parent 89616bd
commit e4324af
Show file tree

Hide file tree

Showing 6 changed files with 51 additions and 40 deletions.
diff --git a/gap/archive.gd b/gap/archive.gd
@@ -7,4 +7,6 @@
 #!   <K>true</K> or <K>false</K>
 DeclareGlobalFunction("InstallPackageFromArchive");
 
+DeclareGlobalFunction("PKGMAN_TarTopDirectory");
+
 PKGMAN_ArchiveFormats := [".tar.gz", ".tar.bz2"];
diff --git a/gap/archive.gi b/gap/archive.gi
@@ -1,7 +1,7 @@
 InstallGlobalFunction(InstallPackageFromArchive,
 function(url)
-  local get, user_pkg_dir, url_parts, filename, path, tar, options, exec,
-  files, topdir, dir, movedname;
+  local get, user_pkg_dir, url_parts, filename, path, exec, topdir, dir,
+        movedname;
 
   # Download archive
   Info(InfoPackageManager, 3, "Downloading archive from URL ", url, " ...");
@@ -18,27 +18,11 @@ function(url)
   FileString(path, get.result);
   Info(InfoPackageManager, 2, "Saved archive to ", path);
 
-  # Check which version of tar we are using
-  tar := PKGMAN_Exec(".", "tar", "--version");
-  if StartsWith(tar.output, "tar (GNU tar)") then
-    options := "--warning=none";
-  else
-    options := "";
-  fi;
-
-  # Check contents
-  exec := PKGMAN_Exec(".", "tar", options, "-tf", path);
-  if exec.code <> 0 then
-    Info(InfoPackageManager, 1, "Could not inspect tarball contents");
+  # Find the name of the directory in the archive
+  topdir := PKGMAN_TarTopDirectory(path);
+  if topdir = fail then
     return false;
   fi;
-  files := SplitString(exec.output, "", "\n");
-  topdir := Set(files, f -> SplitString(f, "/")[1]);
-  if Length(topdir) <> 1 then
-    Info(InfoPackageManager, 1, "Archive should contain 1 directory (not ", Length(topdir), ")");
-    return false;
-  fi;
-  topdir := topdir[1];
 
   # Check availability of target location
   dir := Filename(Directory(user_pkg_dir), topdir);
@@ -70,20 +54,45 @@ function(url)
   # Install dependencies
   if PKGMAN_InstallDependencies(dir) <> true then
     Info(InfoPackageManager, 1, "Dependencies not satisfied for ", topdir);
-    if ValueOption("keepDirectory") <> true then
-      PKGMAN_RemoveDir(dir);
-    fi;
+    PKGMAN_RemoveDirOptional(dir);
     return false;
   fi;
 
   # Check validity
   if PKGMAN_CheckPackage(dir) = false then
-    if ValueOption("keepDirectory") <> true then
-      PKGMAN_RemoveDir(dir);
-    fi;
+    PKGMAN_RemoveDirOptional(dir);
     return false;
   fi;
-  PKGMAN_RefreshPackageInfo();
 
+  PKGMAN_RefreshPackageInfo();
   return true;
 end);
+
+InstallGlobalFunction(PKGMAN_TarTopDirectory,
+function(path)
+  local tar, options, exec, files, topdir;
+  # Check which version of tar we are using
+  tar := PKGMAN_Exec(".", "tar", "--version");
+  if StartsWith(tar.output, "tar (GNU tar)") then
+    options := "--warning=none";
+  else
+    options := "";
+  fi;
+
+  # Check contents
+  exec := PKGMAN_Exec(".", "tar", options, "-tf", path);
+  if exec.code <> 0 then
+    Info(InfoPackageManager, 1, "Could not inspect tarball contents");
+    return fail;
+  fi;
+
+  # Expect to find a single directory and nothing else
+  files := SplitString(exec.output, "", "\n");
+  topdir := Set(files, f -> SplitString(f, "/")[1]);
+  if Length(topdir) <> 1 then
+    Info(InfoPackageManager, 1, "Archive should contain 1 directory (not ", Length(topdir), ")");
+    return fail;
+  fi;
+
+  return topdir[1];
+end);
diff --git a/gap/directories.gd b/gap/directories.gd
@@ -3,6 +3,7 @@ DeclareGlobalFunction("PKGMAN_SetCustomPackageDir");
 DeclareGlobalFunction("PKGMAN_CreateDirRecursively");
 DeclareGlobalFunction("PKGMAN_InsertPackageDirectory");
 DeclareGlobalFunction("PKGMAN_IsValidTargetDir");
+DeclareGlobalFunction("PKGMAN_RemoveDirOptional");
 DeclareGlobalFunction("PKGMAN_RemoveDir");
 
 PKGMAN_CustomPackageDir := "";

diff --git a/gap/directories.gi b/gap/directories.gi
@@ -100,6 +100,13 @@ function(dir)
   return true;
 end);
 
+InstallGlobalFunction(PKGMAN_RemoveDirOptional,
+function(dir)
+  if ValueOption("keepDirectory") <> true then
+    PKGMAN_RemoveDir(dir);
+  fi;
+end);
+
 InstallGlobalFunction(PKGMAN_RemoveDir,
 function(dir)
   # this 'if' statement is a paranoid check - it should always be true

diff --git a/gap/git.gi b/gap/git.gi
@@ -66,18 +66,14 @@ function(url, args...)
   info := Filename(Directory(dir), "PackageInfo.g");
   if not IsReadableFile(info) then
     Info(InfoPackageManager, 1, "Could not find PackageInfo.g");
-    if ValueOption("keepDirectory") <> true then
-      PKGMAN_RemoveDir(dir);
-    fi;
+    PKGMAN_RemoveDirOptional(dir);
     return false;
   fi;
 
   # Install dependencies
   if PKGMAN_InstallDependencies(dir) <> true then
     Info(InfoPackageManager, 1, "Dependencies not satisfied for ", name);
-    if ValueOption("keepDirectory") <> true then
-      PKGMAN_RemoveDir(dir);
-    fi;
+    PKGMAN_RemoveDirOptional(dir);
     return false;
   fi;
 

diff --git a/gap/hg.gi b/gap/hg.gi
@@ -66,18 +66,14 @@ function(url, args...)
   info := Filename(Directory(dir), "PackageInfo.g");
   if not IsReadableFile(info) then
     Info(InfoPackageManager, 1, "Could not find PackageInfo.g");
-    if ValueOption("keepDirectory") <> true then
-      PKGMAN_RemoveDir(dir);
-    fi;
+    PKGMAN_RemoveDirOptional(dir);
     return false;
   fi;
 
   # Install dependencies
   if PKGMAN_InstallDependencies(dir) <> true then
     Info(InfoPackageManager, 1, "Dependencies not satisfied for ", name);
-    if ValueOption("keepDirectory") <> true then
-      PKGMAN_RemoveDir(dir);
-    fi;
+    PKGMAN_RemoveDirOptional(dir);
     return false;
   fi;