From c33deee7a9f35dd4edda8119638954ad4807e525 Mon Sep 17 00:00:00 2001 From: memo Date: Tue, 7 May 2024 18:05:58 +0200 Subject: [PATCH] warn if an inclusion/exclusion pattern did not match any files or if it is a malformed regex. --- CHANGELOG.md | 3 ++ build.sbt | 2 +- .../package-template-variants/1.0/pkg.json | 5 +- .../json/sc4pac-channel-contents.json | 2 +- .../templates/package-template-basic.yaml | 2 +- .../templates/package-template-variants.yaml | 4 +- src/main/scala/sc4pac/Data.scala | 51 +++++++++++++++---- src/main/scala/sc4pac/Sc4pac.scala | 29 ++++++----- src/main/scala/sc4pac/extractor.scala | 12 +++-- 9 files changed, 75 insertions(+), 35 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 79aea7d..e8b9d87 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,9 @@ ## [Unreleased] +### Added +- warning about outdated metadata, in case an inclusion/exclusion pattern does not match any files in an Asset anymore. + ## [0.4.2] - 2024-04-18 ### Added diff --git a/build.sbt b/build.sbt index 6384b18..01b36d0 100644 --- a/build.sbt +++ b/build.sbt @@ -2,7 +2,7 @@ name := "sc4pac" ThisBuild / organization := "io.github.memo33" -ThisBuild / version := "0.4.2" +ThisBuild / version := "0.4.3-SNAPSHOT" // ThisBuild / versionScheme := Some("early-semver") diff --git a/channel-testing/json/metadata/memo/package-template-variants/1.0/pkg.json b/channel-testing/json/metadata/memo/package-template-variants/1.0/pkg.json index 5ab804e..a0d0d38 100644 --- a/channel-testing/json/metadata/memo/package-template-variants/1.0/pkg.json +++ b/channel-testing/json/metadata/memo/package-template-variants/1.0/pkg.json @@ -16,10 +16,7 @@ }, "assets": [ { - "assetId": "memo-testing-first", - "include": [ - "/file1.*" - ] + "assetId": "memo-testing-first" }, { "assetId": "memo-testing-second" diff --git a/channel-testing/json/sc4pac-channel-contents.json b/channel-testing/json/sc4pac-channel-contents.json index 90e2fc6..16b6fb1 100644 --- a/channel-testing/json/sc4pac-channel-contents.json +++ b/channel-testing/json/sc4pac-channel-contents.json @@ -1,5 +1,5 @@ { - "scheme": 1, + "scheme": 4, "contents": [ { "group": "memo", diff --git a/channel-testing/yaml/templates/package-template-basic.yaml b/channel-testing/yaml/templates/package-template-basic.yaml index f7c88a7..db1fed7 100644 --- a/channel-testing/yaml/templates/package-template-basic.yaml +++ b/channel-testing/yaml/templates/package-template-basic.yaml @@ -46,7 +46,7 @@ assets: # Optional list of assets from which to extract files (zero or more) - "/file3.*" - "^/full/path/to/file4\\.dat$" # Optional RegEx exclude patterns tested after includes. - # If absent or empty, then by default all file types other than .dat/.sc4model/.sc4lot/.sc4desc/.sc4 are excluded. + # If absent or empty, then by default all file types other than .dat/.sc4model/.sc4lot/.sc4desc/.sc4/.dll are excluded. exclude: [] # Additional descriptive information, entirely optional. diff --git a/channel-testing/yaml/templates/package-template-variants.yaml b/channel-testing/yaml/templates/package-template-variants.yaml index 871e70e..1aaa0d9 100644 --- a/channel-testing/yaml/templates/package-template-variants.yaml +++ b/channel-testing/yaml/templates/package-template-variants.yaml @@ -12,8 +12,8 @@ variants: dependencies: [] assets: - assetId: "memo-testing-first" - include: - - "/file1.*" + # include: + # - "/file1.*" - assetId: "memo-testing-second" - variant: { driveside: "left" } diff --git a/src/main/scala/sc4pac/Data.scala b/src/main/scala/sc4pac/Data.scala index c21648e..3d67088 100644 --- a/src/main/scala/sc4pac/Data.scala +++ b/src/main/scala/sc4pac/Data.scala @@ -7,6 +7,7 @@ import upickle.default.{ReadWriter, readwriter, stringKeyRW, macroRW} import java.nio.file.{Path as NioPath} import zio.{ZIO, IO, Task, RIO, URIO} import java.util.regex.Pattern +import scala.collection.mutable.Builder import sc4pac.Resolution.{Dep, DepModule, DepAsset} @@ -195,26 +196,58 @@ object JsonData extends SharedData { } case class InstallRecipe(include: Seq[Pattern], exclude: Seq[Pattern]) { - def accepts(path: String): Boolean = { - include.exists(_.matcher(path).find()) && !exclude.exists(_.matcher(path).find()) + def makeAcceptancePredicate(): (Builder[Pattern, Set[Pattern]], os.SubPath => Boolean) = { + val usedPatternsBuilder = Set.newBuilder[Pattern] += InstallRecipe.defaultExcludePattern // default exclude pattern is not required to match anything + + val accepts: os.SubPath => Boolean = { path => + val pathString = path.segments.mkString("/", "/", "") // paths are checked with leading / and with / as separator + include.find(_.matcher(pathString).find()) match { + case None => false + case Some(matchedPattern) => + usedPatternsBuilder += matchedPattern + exclude.find(_.matcher(pathString).find()) match { + case None => true + case Some(matchedPattern) => + usedPatternsBuilder += matchedPattern + false + } + } + } + + (usedPatternsBuilder, accepts) + } + + def usedPatternWarnings(usedPatterns: Set[Pattern]): Seq[Warning] = { + val unused: Seq[Pattern] = (include.iterator ++ exclude).filter(p => !usedPatterns.contains(p)).toSeq + if (unused.isEmpty) { + Seq.empty + } else { + Seq( + "The package metadata seems to be out-of-date, so the package may not have been fully installed. " + + "Please report this to the maintainers of the package metadata. " + + "These inclusion/exclusion patterns did not match any files in the asset: " + unused.mkString(" ")) + } } - def accepts(path: os.SubPath): Boolean = accepts(path.segments.mkString("/", "/", "")) // paths are checked with leading / and with / as separator } object InstallRecipe { - def fromAssetReference(data: AssetReference): InstallRecipe = { - val mkPattern = Pattern.compile(_, Pattern.CASE_INSENSITIVE) + private val mkPattern = Pattern.compile(_, Pattern.CASE_INSENSITIVE) + private val defaultIncludePattern = mkPattern(Constants.defaultInclude) + private val defaultExcludePattern = mkPattern(Constants.defaultExclude) + + def fromAssetReference(data: AssetReference): (InstallRecipe, Seq[Warning]) = { + val warnings = Seq.newBuilder[Warning] def toRegex(s: String): Option[Pattern] = try { Some(mkPattern(s)) } catch { case e: java.util.regex.PatternSyntaxException => - System.err.println(s"include/exclude pattern contains invalid regex: $e") // TODO logger + warnings += s"The package metadata contains a malformed regex: $e" None } val include = data.include.flatMap(toRegex) val exclude = data.exclude.flatMap(toRegex) - InstallRecipe( - include = if (include.isEmpty) Seq(mkPattern(Constants.defaultInclude)) else include, - exclude = if (exclude.isEmpty) Seq(mkPattern(Constants.defaultExclude)) else exclude) + (InstallRecipe( + include = if (include.isEmpty) Seq(defaultIncludePattern) else include, + exclude = if (exclude.isEmpty) Seq(defaultExcludePattern) else exclude), warnings.result()) } } diff --git a/src/main/scala/sc4pac/Sc4pac.scala b/src/main/scala/sc4pac/Sc4pac.scala index 9680827..661468f 100644 --- a/src/main/scala/sc4pac/Sc4pac.scala +++ b/src/main/scala/sc4pac/Sc4pac.scala @@ -178,24 +178,24 @@ trait UpdateService { this: Sc4pac => val id = BareAsset(ModuleName(assetData.assetId)) artifactsById.get(id) match { case None => - val w = s"An artifact is missing and has been skipped, so it must be installed manually: ${id.orgName}" - logger.warn(w) - Seq(w) + Seq(s"An asset is missing and has been skipped, so it needs to be installed manually: ${id.orgName}. " + + "Please report this to the maintainers of the package metadata.") case Some(art, archive, depAsset) => - val recipe = JD.InstallRecipe.fromAssetReference(assetData) + val (recipe, regexWarnings) = JD.InstallRecipe.fromAssetReference(assetData) val extractor = new Extractor(logger) val fallbackFilename = cache.getFallbackFilename(archive) val jarsRoot = stagingRoot / "jars" - extractor.extract( - archive, - fallbackFilename, - tempPluginsRoot / pkgFolder, - recipe, - Some(Extractor.JarExtraction.fromUrl(art.url, cache, jarsRoot = jarsRoot, profileRoot = profileRoot)), - hints = depAsset.archiveType, - stagingRoot) + val usedPatterns = + extractor.extract( + archive, + fallbackFilename, + tempPluginsRoot / pkgFolder, + recipe, + Some(Extractor.JarExtraction.fromUrl(art.url, cache, jarsRoot = jarsRoot, profileRoot = profileRoot)), + hints = depAsset.archiveType, + stagingRoot) // TODO catch IOExceptions - Seq.empty + regexWarnings ++ recipe.usedPatternWarnings(usedPatterns) } } @@ -237,6 +237,7 @@ trait UpdateService { this: Sc4pac => _ <- ZIO.attemptBlocking(os.makeDir.all(tempPluginsRoot / pkgFolder)) // create folder even if package does not have any assets or files ws <- ZIO.foreach(variant.assets)(extract(_, pkgFolder)) } yield ws.flatten) + _ <- ZIO.foreach(artifactWarnings)(w => ZIO.attempt(logger.warn(w))) // to avoid conflicts with spinner animation, we print warnings after extraction already finished dlls <- linkDlls(pkgFolder) warnings <- if (pkgData.info.warning.nonEmpty) ZIO.attempt { val w = pkgData.info.warning; logger.warn(w); Seq(w) } else ZIO.succeed(Seq.empty) @@ -305,7 +306,7 @@ trait UpdateService { this: Sc4pac => (stagedFiles, warnings) <- ZIO.foreach(deps.zipWithIndex) { case (dep, idx) => // sequentially stages each package stage(tempPluginsRoot, dep, artifactsById, stagingRoot, Sc4pac.Progress(idx+1, numDeps)) }.map(_.unzip) - pkgWarnings = deps.zip(warnings).collect { case (dep, ws) if ws.nonEmpty => (dep.toBareDep, ws) } + pkgWarnings = deps.zip(warnings: Seq[Seq[Warning]]).collect { case (dep, ws) if ws.nonEmpty => (dep.toBareDep, ws) } _ <- ZIO.serviceWithZIO[Prompter](_.confirmInstallationWarnings(pkgWarnings)) .filterOrFail(_ == true)(error.Sc4pacAbort()) } yield StageResult(tempPluginsRoot, deps.zip(stagedFiles), stagingRoot) diff --git a/src/main/scala/sc4pac/extractor.scala b/src/main/scala/sc4pac/extractor.scala index 405e5ac..07eee43 100644 --- a/src/main/scala/sc4pac/extractor.scala +++ b/src/main/scala/sc4pac/extractor.scala @@ -6,6 +6,7 @@ import org.apache.commons.compress.archivers.zip.{ZipFile, ZipArchiveEntry} import org.apache.commons.compress.archivers.sevenz.{SevenZFile, SevenZArchiveEntry} import org.apache.commons.compress.utils.IOUtils import java.nio.file.StandardOpenOption +import java.util.regex.Pattern import sc4pac.JsonData as JD import sc4pac.error.ExtractionFailed @@ -328,6 +329,8 @@ class Extractor(logger: Logger) { } } + /** Extracts files from the archive that match the inclusion/exclusion recipe, including one level of nested archives. + * Returns the patterns that have matched. */ def extract( archive: java.io.File, fallbackFilename: Option[String], @@ -336,9 +339,10 @@ class Extractor(logger: Logger) { jarExtractionOpt: Option[Extractor.JarExtraction], hints: Option[JD.ArchiveType], stagingRoot: os.Path, - ): Unit = try { + ): Set[Pattern] = try { + val (usedPatternsBuilder, predicate) = recipe.makeAcceptancePredicate() // tracks used patterns to warn about unused patterns // first extract just the main files - extractByPredicate(archive, destination, recipe.accepts, overwrite = true, flatten = false, fallbackFilename, hints, stagingRoot = stagingRoot) + extractByPredicate(archive, destination, predicate, overwrite = true, flatten = false, fallbackFilename, hints, stagingRoot = stagingRoot) // additionally, extract jar files contained in the zip file to a temporary location for (Extractor.JarExtraction(jarsDir) <- jarExtractionOpt) { logger.debug(s"Searching for nested archives:") @@ -346,9 +350,11 @@ class Extractor(logger: Logger) { // finally extract the jar files themselves (without recursively extracting jars contained inside) for (jarFile <- jarFiles if Extractor.acceptNestedArchive(jarFile)) { logger.debug(s"Extracting nested archive ${jarFile.last}") - extract(jarFile.toIO, fallbackFilename = None, destination, recipe, jarExtractionOpt = None, hints, stagingRoot = stagingRoot) + usedPatternsBuilder ++= + extract(jarFile.toIO, fallbackFilename = None, destination, recipe, jarExtractionOpt = None, hints, stagingRoot = stagingRoot) } } + usedPatternsBuilder.result() } catch { case e: ExtractionFailed => throw e case e: java.io.IOException => logger.debugPrintStackTrace(e); throw new ExtractionFailed(s"Failed to extract $archive.", e.getMessage)