Skip to content

Commit

Permalink
warn if an inclusion/exclusion pattern did not match any files
Browse files Browse the repository at this point in the history
or if it is a malformed regex.
  • Loading branch information
memo33 committed May 7, 2024
1 parent 984e963 commit c33deee
Show file tree
Hide file tree
Showing 9 changed files with 75 additions and 35 deletions.
3 changes: 3 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
2 changes: 1 addition & 1 deletion build.sbt
Original file line number Diff line number Diff line change
Expand Up @@ -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")

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,10 +16,7 @@
},
"assets": [
{
"assetId": "memo-testing-first",
"include": [
"/file1.*"
]
"assetId": "memo-testing-first"
},
{
"assetId": "memo-testing-second"
Expand Down
2 changes: 1 addition & 1 deletion channel-testing/json/sc4pac-channel-contents.json
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
{
"scheme": 1,
"scheme": 4,
"contents": [
{
"group": "memo",
Expand Down
2 changes: 1 addition & 1 deletion channel-testing/yaml/templates/package-template-basic.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
4 changes: 2 additions & 2 deletions channel-testing/yaml/templates/package-template-variants.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -12,8 +12,8 @@ variants:
dependencies: []
assets:
- assetId: "memo-testing-first"
include:
- "/file1.*"
# include:
# - "/file1.*"
- assetId: "memo-testing-second"

- variant: { driveside: "left" }
Expand Down
51 changes: 42 additions & 9 deletions src/main/scala/sc4pac/Data.scala
Original file line number Diff line number Diff line change
Expand Up @@ -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}

Expand Down Expand Up @@ -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())
}
}

Expand Down
29 changes: 15 additions & 14 deletions src/main/scala/sc4pac/Sc4pac.scala
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
}

Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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)
Expand Down
12 changes: 9 additions & 3 deletions src/main/scala/sc4pac/extractor.scala
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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],
Expand All @@ -336,19 +339,22 @@ 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:")
val jarFiles = extractByPredicate(archive, jarsDir, Extractor.acceptNestedArchive, overwrite = false, flatten = true, fallbackFilename, hints, stagingRoot = stagingRoot) // overwrite=false, as we want to extract any given jar only once per staging process
// 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)
Expand Down

0 comments on commit c33deee

Please sign in to comment.