Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with HTTPS or Subversion.

Download ZIP

Loading…

accomodating windows style file separator in default MergeStrategy #51

Closed
wants to merge 3 commits into from

2 participants

@stejskal

just replaced hardcoded "/" in regular expressions and match cases with System.getProperty("file.separator"). May help other developers on windows utilize this awesome plugin easier out of the box.

Spencer Stej... added some commits
@eed3si9n
Owner

If we have to work so hard at the problem, maybe we're not solving it right. Should we consider using Path or some other way of tokenizing the path string systematically?

@stejskal

we could use an Extractor like this:

  object FileExtractor {
    def unapply(path: String) = {
      val split = path.toLowerCase.split(if (sysFileSep.equals( """\""")) """\\""" else sysFileSep)
      if (split.isEmpty)
        None
      else
        new Some((split.head, split.tail.headOption.getOrElse(""), split.last))
    }
  }

it still has the wonkiness of the \ in the split regex, but it cleans up the match and the other regular expressions quite a bit.

@eed3si9n
Owner

Yea anything that'll centralize the backslash handling would be nice. Also could you hide the variables under private since all fields under plugins gets imported automatically into build.sbt?

@stejskal

Ok i committed the PathExtractor, which I evidently called FileExtractor before, and made it private. I ran it on my project which utilizes about 2/3 of the cases in the match. I looked at your tests and read your post http://eed3si9n.com/testing-sbt-plugins but that is all new to me so I apologize for not running your test suite against the change.

@eed3si9n

Why not just return Some(split.toList) here? I don't see why we should limit to first, second and last.

@stejskal

I used just first second and last because they are sufficient for the matches being done and it makes the cases very succinct. The extractor is a very specialized extractor only used locally so I don't see any reason to make it more flexible then its usage requires if it will clutter up code that uses it. That being said, there may be some way to use extractors that use lists that I am not familiar with that may make the trade off more palatable.

@eed3si9n
Owner

The purpose of merge strategy system partially is to allow custom handling per build, not just the default rules that we ship.
Treating the first and second directories specially in my opinion makes it harder to understand. By using def unapplySeq you can define an extractor that can match arbitrary params. See http://daily-scala.blogspot.com/2009/09/extract-sequences-unapplyseq.html

@eed3si9n eed3si9n closed this
@stejskal

sorry, got side tracked. the unapplySeq was the piece i was missing. I think that was a good solution to allow flexibility as well as clean usage

@eed3si9n eed3si9n referenced this pull request from a commit
@eed3si9n eed3si9n unapplySeq. #51 4a8f320
@eed3si9n
Owner

I had forgotten about the unapplySeq myself when I hand merged it. I just put it in.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Commits on Jun 6, 2012
  1. updated default merge strategy to accommodate windows style file sepe…

    Spencer Stejskal authored
    …rators as well as linux ones
  2. updated default merge strategy to accommodate windows style file sepe…

    Spencer Stejskal authored
    …rators as well as linux ones
Commits on Jun 7, 2012
This page is out of date. Refresh to see the latest.
Showing with 25 additions and 24 deletions.
  1. +25 −24 src/main/scala/sbtassembly/Plugin.scala
View
49 src/main/scala/sbtassembly/Plugin.scala
@@ -241,19 +241,31 @@ object Plugin extends sbt.Plugin {
(key.? zipWith rhs)( (x,y) => (x :^: y :^: KNil) map Scoped.hf2( _ getOrElse _ ))
}
- private val LicenseFile = """(.*/)?(license|licence|notice|copying)([.]\w+)?$""".r
+ private val LicenseFile = ("""(license|licence|notice|copying)([.]\w+)?$""").r
+
private def isLicenseFile(fileName: String): Boolean =
fileName.toLowerCase match {
- case LicenseFile(_, _, ext) if ext != ".class" => true // DISLIKE
+ case LicenseFile(_, ext) if ext != ".class" => true // DISLIKE
case _ => false
}
- private val ReadMe = """(.*/)?(readme)([.]\w+)?$""".r
+ private val ReadMe = """(readme)([.]\w+)?$""".r
+
private def isReadme(fileName: String): Boolean =
fileName.toLowerCase match {
- case ReadMe(x, y, z) => true
+ case ReadMe(_, _) => true
case _ => false
}
+ private object PathExtractor {
+ private val sysFileSep = System.getProperty("file.separator")
+ def unapply(path: String) = {
+ val split = path.toLowerCase.split(if (sysFileSep.equals( """\""")) """\\""" else sysFileSep)
+ if (split.isEmpty)
+ None
+ else
+ new Some((split.head, split.tail.headOption.getOrElse(""), split.last))
+ }
+ }
lazy val baseAssemblySettings: Seq[sbt.Project.Setting[_]] = Seq(
assembly <<= (test in assembly, outputPath in assembly, packageOptions in assembly,
@@ -264,26 +276,15 @@ object Plugin extends sbt.Plugin {
assembledMappings in assembly <<= (assemblyOption in assembly, fullClasspath in assembly, dependencyClasspath in assembly,
excludedJars in assembly, streams) map {
(ao, cp, deps, ej, s) => (tempDir: File) => assemblyAssembledMappings(tempDir, cp, deps, ao, ej, s.log) },
-
- mergeStrategy in assembly := {
- case "reference.conf" =>
- MergeStrategy.concat
- case n if isReadme(n) || isLicenseFile(n) =>
- MergeStrategy.rename
- case inf if inf.startsWith("META-INF/") =>
- inf.slice("META-INF/".size, inf.size).toLowerCase match {
- case "manifest.mf" | "index.list" | "dependencies" =>
- MergeStrategy.discard
- case n if n.endsWith(".sf") || n.endsWith(".dsa") =>
- MergeStrategy.discard
- case n if n startsWith "plexus/" =>
- MergeStrategy.discard
- case n if n startsWith "services/" =>
- MergeStrategy.filterDistinctLines
- case "spring.schemas" | "spring.handlers" =>
- MergeStrategy.filterDistinctLines
- case _ => MergeStrategy.deduplicate
- }
+
+ mergeStrategy in assembly := {
+ case "reference.conf" => MergeStrategy.concat
+ case PathExtractor(_, _, filename) if isReadme(filename) || isLicenseFile(filename) => MergeStrategy.rename
+ case PathExtractor("meta-inf", "plexus", _) => MergeStrategy.discard
+ case PathExtractor("meta-inf", "services", _) => MergeStrategy.filterDistinctLines
+ case PathExtractor("meta-inf", _, "manifest.mf" | "index.list" | "dependencies") => MergeStrategy.discard
+ case PathExtractor("meta-inf", _, "spring.schemas" | "spring.handlers") => MergeStrategy.filterDistinctLines
+ case PathExtractor("meta-inf", _, filename) if filename.endsWith(".sf") || filename.endsWith(".dsa")=> MergeStrategy.discard
case _ => MergeStrategy.deduplicate
},
Something went wrong with that request. Please try again.