~jan0sch/smederee

Showing details for patch 1c6eb8e14f237c49bcfb7776ff318344dc1838a3.
2023-08-16 (Wed), 8:13 AM - Jens Grassel - 1c6eb8e14f237c49bcfb7776ff318344dc1838a3

VCS: Change download package code to fix unicode filename issues.

The darcs dist command has problems with unicode characters in filenames
producing archives that have wrongly named directories and files.

Therefore the `tar` command is now used to create an archive of a repository.

Fixes: https://tickets.smeder.ee/~jan0sch/smederee/tickets/5
Summary of changes
1 files modified with 41 lines added and 17 lines removed
  • modules/hub/src/main/scala/de/smederee/hub/VcsRepositoryRoutes.scala with 41 added and 17 removed lines
diff -rN -u old-smederee/modules/hub/src/main/scala/de/smederee/hub/VcsRepositoryRoutes.scala new-smederee/modules/hub/src/main/scala/de/smederee/hub/VcsRepositoryRoutes.scala
--- old-smederee/modules/hub/src/main/scala/de/smederee/hub/VcsRepositoryRoutes.scala	2025-01-15 12:57:21.289221454 +0000
+++ new-smederee/modules/hub/src/main/scala/de/smederee/hub/VcsRepositoryRoutes.scala	2025-01-15 12:57:21.293221461 +0000
@@ -175,8 +175,6 @@
 
   /** Logic for creating a distribution file and serving it as a download to the user.
     *
-    * @param csrf
-    *   An optional CSRF-Token that shall be used.
     * @param user
     *   An optional user account for whom the list of repositories shall be rendered.
     * @param repositoryOwnerName
@@ -187,10 +185,39 @@
     *   An option to the file name that can be streamed to the user.
     */
   private def doDownloadDistribution(
-      csrf: Option[CsrfToken]
-  )(
       user: Option[Account]
-  )(repositoryOwnerName: Username, repositoryName: VcsRepositoryName): F[Option[fs2.io.file.Path]] =
+  )(repositoryOwnerName: Username, repositoryName: VcsRepositoryName): F[Option[fs2.io.file.Path]] = {
+    // TODO: Clean this up and refactor into proper helpers.
+
+    /** Helper function to package a repository into a target directory because the darcs dist task has problems with
+      * unicode characters in filenames. The archive will be created with a temporary name and afterwards it will be
+      * moved to the given target directory and will be named `$repositoryName.tar.gz`.
+      *
+      * @param respositoryOwnerDirectory
+      *   The directory in which the repository resides and in which the command will be executed.
+      * @param repositoryName
+      *   The name of the repository (and the name of the directory to be packaged).
+      * @param targetDirectory
+      *   The target (destination) directory into which the archive will be created.
+      */
+    def packageRepository(
+        basePath: os.Path,
+        repositoryName: String,
+        targetDirectory: os.Path
+    ): F[(Int, Chain[String], Chain[String])] = {
+      val targetFile     = targetDirectory / s"${repositoryName}.tar.gz"
+      val tempTargetFile = Files.createTempFile(targetDirectory.toNIO, "download", s"${repositoryName}.tar.gz")
+      log.debug(s"Packaging repository $repositoryName into $targetFile.")
+      val tarOptions      = List("-czf", tempTargetFile.toString, "--exclude=_darcs", repositoryName.toString)
+      val externalCommand = os.proc("tar", tarOptions)
+      for {
+        _       <- Sync[F].delay(Files.createDirectories(targetDirectory.toNIO))
+        process <- Sync[F].delay(externalCommand.call(cwd = basePath, check = false))
+        _       <- Sync[F].delay(os.remove(targetFile))
+        _       <- Sync[F].delay(os.move(os.Path(tempTargetFile), targetFile))
+      } yield (process.exitCode, Chain(process.out.text()), Chain(process.err.text()))
+    }
+
     for {
       repoAndId <- loadRepo(user)(repositoryOwnerName, repositoryName)
       repo = repoAndId.map(_._1)
@@ -211,24 +238,21 @@
         )
       )
       createDist <- repo.traverse(repo =>
-        darcs.dist(repositoryOwnerDirectory.toNIO)(repositoryName.toString)(Chain.empty)
+        packageRepository(repositoryOwnerDirectory, repositoryName.toString, targetDirectory)
       )
-      _ <- Sync[F].delay(repo.map(_ => Files.createDirectories(targetDirectory.toNIO)))
-      _ <- Sync[F].delay(repo.map(_ => os.remove(targetDirectory / s"${repositoryName.toString}.tar.gz")))
-      moveDist <- repo.traverse(_ =>
-        Sync[F].delay(
-          os.move.into(
-            repositoryOwnerDirectory / repositoryName.toString / s"${repositoryName.toString}.tar.gz",
-            targetDirectory
+      _ = createDist.map { case (exitCode, stdout, stderr) =>
+        if (exitCode =!= 0)
+          log.error(
+            s"An error occured while packaging $repositoryName: ${stderr.toList.mkString}, ${stdout.toList.mkString}"
           )
-        )
-      )
+      }
       requestedFilePath <- repo.traverse(_ =>
         Sync[F].delay(
           fs2.io.file.Path.fromNioPath((targetDirectory / s"${repositoryName.toString}.tar.gz").toNIO)
         )
       )
     } yield requestedFilePath
+  }
 
   /** Logic for rendering a list of all repositories visible to the given user account.
     *
@@ -861,7 +885,7 @@
         ) / "download" as user =>
       for {
         csrf <- Sync[F].delay(ar.req.getCsrfToken)
-        file <- doDownloadDistribution(csrf)(user.some)(repositoryOwnerName, repositoryName)
+        file <- doDownloadDistribution(user.some)(repositoryOwnerName, repositoryName)
         resp <- file match {
           case None           => NotFound()
           case Some(filePath) => StaticFile.fromPath(filePath, ar.req.some).getOrElseF(NotFound())
@@ -875,7 +899,7 @@
         ) / "download" =>
       for {
         csrf <- Sync[F].delay(req.getCsrfToken)
-        file <- doDownloadDistribution(csrf)(None)(repositoryOwnerName, repositoryName)
+        file <- doDownloadDistribution(None)(repositoryOwnerName, repositoryName)
         resp <- file match {
           case None           => NotFound()
           case Some(filePath) => StaticFile.fromPath(filePath, req.some).getOrElseF(NotFound())