From 1c1c3904398acd60bc1644120e18ce27e2b29cb6 Mon Sep 17 00:00:00 2001 From: Alexandre Archambault Date: Thu, 18 Jun 2015 00:25:11 +0200 Subject: [PATCH 1/5] Displaying errors in web app Fixes https://github.com/alexarchambault/coursier/issues/5 --- .../coursier/core/compatibility/package.scala | 13 ++++++ web/src/main/resources/index-dev.html | 4 ++ web/src/main/scala/coursier/web/Backend.scala | 44 ++++++++++++++----- 3 files changed, 49 insertions(+), 12 deletions(-) diff --git a/core-js/src/main/scala/coursier/core/compatibility/package.scala b/core-js/src/main/scala/coursier/core/compatibility/package.scala index 7766bc79b..86c0a614d 100644 --- a/core-js/src/main/scala/coursier/core/compatibility/package.scala +++ b/core-js/src/main/scala/coursier/core/compatibility/package.scala @@ -30,6 +30,16 @@ package object compatibility { js.Dynamic.newInstance(defn)() } + lazy val XMLSerializer = { + import js.Dynamic.{global => g} + import js.DynamicImplicits._ + + val defn = + if (js.isUndefined(g.XMLSerializer)) g.require("xmldom").XMLSerializer + else g.XMLSerializer + js.Dynamic.newInstance(defn)() + } + def fromNode(node: org.scalajs.dom.raw.Node): Xml.Node = { val node0 = node.asInstanceOf[js.Dynamic] @@ -52,6 +62,9 @@ package object compatibility { def isElement = option[Int](node0.nodeType) .exists(_ == 1) // org.scalajs.dom.raw.Node.ELEMENT_NODE + + override def toString = + XMLSerializer.serializeToString(node).asInstanceOf[String] } } diff --git a/web/src/main/resources/index-dev.html b/web/src/main/resources/index-dev.html index a0726ba04..648f446ee 100644 --- a/web/src/main/resources/index-dev.html +++ b/web/src/main/resources/index-dev.html @@ -27,6 +27,10 @@ .customButton { margin: 5px; } + + .popover { + max-width: 400px; + } diff --git a/web/src/main/scala/coursier/web/Backend.scala b/web/src/main/scala/coursier/web/Backend.scala index 4b1cb1d2e..975799fc9 100644 --- a/web/src/main/scala/coursier/web/Backend.scala +++ b/web/src/main/scala/coursier/web/Backend.scala @@ -175,6 +175,10 @@ class Backend($: BackendScope[Unit, State]) { } } + def enablePopover(e: ReactEventI) = { + g.$("[data-toggle='popover']").popover() + } + object options { def toggleOptional(e: ReactEventI) = { $.modState(s => s.copy(options = s.options.copy(followOptional = !s.options.followOptional))) @@ -189,8 +193,22 @@ object App { lazy val arbor = g.arbor - val resultDependencies = ReactComponentB[Resolution]("Result") - .render{ res => + val resultDependencies = ReactComponentB[(Resolution, Backend)]("Result") + .render{ T => + val (res, backend) = T + + def infoLabel(label: String) = + <.span(^.`class` := "label label-info", label) + def errorLabel(label: String, desc: String) = + <.button(^.`type` := "button", ^.`class` := "btn btn-xs btn-danger", + Attr("data-trigger") := "focus", + Attr("data-toggle") := "popover", Attr("data-placement") := "bottom", + Attr("data-content") := desc, + ^.onClick ==> backend.enablePopover, + ^.onMouseOver ==> backend.enablePopover, + label + ) + def depItem(dep: Dependency) = <.tr( ^.`class` := (if (res.errors.contains(dep.module)) "danger" else ""), @@ -198,10 +216,11 @@ object App { <.td(dep.module.name), <.td(dep.module.version), <.td(Seq[Seq[TagMod]]( - if (dep.scope == Scope.Compile) Seq() else Seq(<.span(^.`class` := "label label-info", dep.scope.name)), - if (dep.`type`.isEmpty || dep.`type` == "jar") Seq() else Seq(<.span(^.`class` := "label label-info", dep.`type`)), - if (dep.classifier.isEmpty) Seq() else Seq(<.span(^.`class` := "label label-info", dep.classifier)), - if (dep.optional) Seq(<.span(^.`class` := "label label-info", dep.classifier)) else Seq() + if (dep.scope == Scope.Compile) Seq() else Seq(infoLabel(dep.scope.name)), + if (dep.`type`.isEmpty || dep.`type` == "jar") Seq() else Seq(infoLabel(dep.`type`)), + if (dep.classifier.isEmpty) Seq() else Seq(infoLabel(dep.classifier)), + if (dep.optional) Seq(infoLabel("optional")) else Seq(), + res.errors.get(dep.module).map(errs => errorLabel("Error", errs.mkString("; "))).toSeq )), <.td(Seq[Seq[TagMod]]( res.projectsCache.get(dep.module) match { @@ -244,7 +263,6 @@ object App { <.tbody( sortedDeps.map(depItem) ) -// <.div(dangerouslySetInnerHtml("")) ) } .build @@ -406,21 +424,23 @@ object App { } .build - val resolution = ReactComponentB[Option[Resolution]]("Resolution") - .render(resOpt => + val resolution = ReactComponentB[(Option[Resolution], Backend)]("Resolution") + .render{ T => + val (resOpt, backend) = T + resOpt match { case Some(res) => <.div( <.div(^.`class` := "page-header", <.h1("Resolution") ), - resultDependencies(res) + resultDependencies((res, backend)) ) case None => <.div() } - ) + } .build val initialState = State(Nil, Seq(coursier.repository.mavenCentral), ResolutionOptions(), None, -1, resolving = false, reverseTree = false, log = Nil) @@ -494,7 +514,7 @@ object App { ), <.div(^.`class` := "tab-content", <.div(^.role := "tabpanel", ^.`class` := "tab-pane", ^.id := "resolution", - resolution(S.resolutionOpt) + resolution((S.resolutionOpt, B)) ), <.div(^.role := "tabpanel", ^.`class` := "tab-pane", ^.id := "log", <.button(^.`type` := "button", ^.`class` := "btn btn-default", From e65ef0ecfe08067dfeda8531c4e05b6ad8e907fd Mon Sep 17 00:00:00 2001 From: Alexandre Archambault Date: Thu, 18 Jun 2015 00:25:14 +0200 Subject: [PATCH 2/5] Displaying errors with cli tool Fixes https://github.com/alexarchambault/coursier/issues/19 --- cli/src/main/scala/coursier/cli/Coursier.scala | 17 +++++++++++++++-- 1 file changed, 15 insertions(+), 2 deletions(-) diff --git a/cli/src/main/scala/coursier/cli/Coursier.scala b/cli/src/main/scala/coursier/cli/Coursier.scala index 77b0c60c5..8f7b8ac22 100644 --- a/cli/src/main/scala/coursier/cli/Coursier.scala +++ b/cli/src/main/scala/coursier/cli/Coursier.scala @@ -91,9 +91,22 @@ case class Coursier(scope: List[String], def repr(dep: Dependency) = s"${dep.module.organization}:${dep.module.name}:${dep.`type`}:${Some(dep.classifier).filter(_.nonEmpty).map(_+":").mkString}${dep.module.version}" - val trDeps = res.dependencies.toList.map(repr).sorted + val trDeps = res.dependencies.toList.sortBy(repr) - println("\n" + trDeps.mkString("\n")) + println("\n" + trDeps.map(repr).mkString("\n")) + + if (res.conflicts.nonEmpty) { + // Needs test + println(s"${res.conflicts.size} conflict(s):\n ${res.conflicts.toList.map(repr).sorted.mkString(" \n")}") + } + + val errDeps = trDeps.filter(dep => res.errors.contains(dep.module)) + if (errDeps.nonEmpty) { + println(s"${errDeps.size} error(s):") + for (dep <- errDeps) { + println(s" ${dep.module}:\n ${res.errors(dep.module).mkString("\n").replace("\n", " \n")}") + } + } if (fetch) { println() From fd495cc9f8537ee31030f242ee6a0f3cb3039dea Mon Sep 17 00:00:00 2001 From: Alexandre Archambault Date: Thu, 18 Jun 2015 00:25:16 +0200 Subject: [PATCH 3/5] Looser property parsing (for scala-js) --- .../coursier/core/compatibility/package.scala | 5 +- core/src/main/scala/coursier/core/Xml.scala | 10 ++-- .../scala/coursier/test/PomParsingTests.scala | 56 +++++++++++++++++++ 3 files changed, 61 insertions(+), 10 deletions(-) diff --git a/core-jvm/src/main/scala/coursier/core/compatibility/package.scala b/core-jvm/src/main/scala/coursier/core/compatibility/package.scala index a1e051060..5eb8cbc97 100644 --- a/core-jvm/src/main/scala/coursier/core/compatibility/package.scala +++ b/core-jvm/src/main/scala/coursier/core/compatibility/package.scala @@ -17,10 +17,7 @@ package object compatibility { def label = node.label def child = node.child.map(fromNode) def isText = node match { case _: scala.xml.Text => true; case _ => false } - def textContent = node match { - case t: scala.xml.Text => t.data - case _ => throw new NoSuchElementException("text of non text node") - } + def textContent = node.text def isElement = node match { case _: scala.xml.Elem => true; case _ => false } } diff --git a/core/src/main/scala/coursier/core/Xml.scala b/core/src/main/scala/coursier/core/Xml.scala index 4449985de..70b3b3fcf 100644 --- a/core/src/main/scala/coursier/core/Xml.scala +++ b/core/src/main/scala/coursier/core/Xml.scala @@ -41,12 +41,10 @@ object Xml { .toRightDisjunction(s"$description not found") } - private def property(elem: Node): String \/ (String, String) = { - elem.child match { - case Seq() => \/-(elem.label -> "") - case Seq(Text(t)) => \/-(elem.label -> t) - case _ => -\/(s"Can't parse property $elem") - } + def property(elem: Node): String \/ (String, String) = { + // Not matching with Text, which fails on scala-js if the property value has xml comments + if (elem.isElement) \/-(elem.label -> elem.textContent) + else -\/(s"Can't parse property $elem") } // TODO Allow no version in some contexts diff --git a/core/src/test/scala/coursier/test/PomParsingTests.scala b/core/src/test/scala/coursier/test/PomParsingTests.scala index 568ae6be8..1582fdf1d 100644 --- a/core/src/test/scala/coursier/test/PomParsingTests.scala +++ b/core/src/test/scala/coursier/test/PomParsingTests.scala @@ -138,6 +138,62 @@ object PomParsingTests extends TestSuite { assert(result == expected) } + 'beFineWithCommentsInProperties{ + import scalaz._, Scalaz._ + + val properties = + """ + | + | 1.6 + | 1.6 + | io + | RC1 + | 2.4 + | (requires JDK 1.6+) + | 2.2 + | (requires JDK 1.5+) + | IO + | 12310477 + | + | + | org.apache.commons.io; + | org.apache.commons.io.comparator; + | org.apache.commons.io.filefilter; + | org.apache.commons.io.input; + | org.apache.commons.io.output;version=1.4.9999;-noimport:=true, + | + | org.apache.commons.io; + | org.apache.commons.io.comparator; + | org.apache.commons.io.filefilter; + | org.apache.commons.io.input; + | org.apache.commons.io.output; + | org.apache.commons.io.*;version=${project.version};-noimport:=true + | + | + | + """.stripMargin + + val parsed = core.compatibility.xmlParse(properties) + assert(parsed.isRight) + + val node = parsed.right.get + assert(node.label == "properties") + + val children = node.child.collect{case elem if elem.isElement => elem} + val props0 = children.toList.traverseU(Xml.property) + + assert(props0.isRight) + + val props = props0.getOrElse(???).toMap + + assert(props.contains("commons.release.2.desc")) + assert(props.contains("commons.osgi.export")) + + assert(props("commons.rc.version") == "RC1") + assert(props("commons.release.2.desc") == "(requires JDK 1.5+)") + assert(props("commons.osgi.export").contains("org.apache.commons.io.filefilter;")) + assert(props("commons.osgi.export").contains("org.apache.commons.io.input;")) + } } } From 8d27ab7150ad1f8ba72ce6d4b5b9850be8d486b8 Mon Sep 17 00:00:00 2001 From: Alexandre Archambault Date: Thu, 18 Jun 2015 00:25:17 +0200 Subject: [PATCH 4/5] Better properties substitution --- .../main/scala/coursier/core/Resolver.scala | 27 ++++++++++--------- .../scala/coursier/test/CentralTests.scala | 26 ++++++++++++++++-- 2 files changed, 38 insertions(+), 15 deletions(-) diff --git a/core/src/main/scala/coursier/core/Resolver.scala b/core/src/main/scala/coursier/core/Resolver.scala index 428e3d6c1..651dfa89b 100644 --- a/core/src/main/scala/coursier/core/Resolver.scala +++ b/core/src/main/scala/coursier/core/Resolver.scala @@ -350,21 +350,22 @@ object Resolver { // Here, we're substituting properties also in dependencies that come from parents // or dependency management. This may not be the right thing to do. + val properties = mergeProperties( + project.properties, + Map( + "project.groupId" -> project.module.organization, + "project.artifactId" -> project.module.name, + "project.version" -> project.module.version + ) + ) + val deps = withExclusions( - withProperties( - depsWithDependencyManagement( - project.dependencies, - project.dependencyManagement - ), - mergeProperties( - project.properties, - Map( - "project.groupId" -> project.module.organization, - "project.artifactId" -> project.module.name, - "project.version" -> project.module.version - ) - ) + depsWithDependencyManagement( + // important: properties have to be applied to both, so that dep mgmt can be matched properly + // See the added test with org.ow2.asm:asm-commons:5.0.2 + withProperties(project.dependencies, properties), + withProperties(project.dependencyManagement, properties) ), from.exclusions ) diff --git a/core/src/test/scala/coursier/test/CentralTests.scala b/core/src/test/scala/coursier/test/CentralTests.scala index ec8d65721..6699d2f0d 100644 --- a/core/src/test/scala/coursier/test/CentralTests.scala +++ b/core/src/test/scala/coursier/test/CentralTests.scala @@ -21,8 +21,7 @@ object CentralTests extends TestSuite { .runF) val res = res0.copy( - projectsCache = Map.empty, errors = Map.empty, // No validating these here - dependencies = res0.dependencies.filter(dep => dep.scope == Scope.Compile && !dep.optional) + projectsCache = Map.empty, errors = Map.empty // No validating these here ) val expected = Resolution( @@ -34,6 +33,29 @@ object CentralTests extends TestSuite { ) ) + assert(res == expected) + } + } + 'asm{ + async { + val dep = Dependency(Module("org.ow2.asm", "asm-commons", "5.0.2")) + val res0 = + await(resolve(Set(dep), fetchFrom(repositories)) + .runF) + + val res = res0.copy( + projectsCache = Map.empty, errors = Map.empty // No validating these here + ) + + val expected = Resolution( + rootDependencies = Set(dep.withCompileScope), + dependencies = Set( + dep.withCompileScope, + Dependency(Module("org.ow2.asm", "asm-tree", "5.0.2")).withCompileScope, + Dependency(Module("org.ow2.asm", "asm", "5.0.2")).withCompileScope + ) + ) + assert(res == expected) } } From 399922e204ef7d1d3a3f38d41234173d60669eb5 Mon Sep 17 00:00:00 2001 From: Alexandre Archambault Date: Thu, 18 Jun 2015 00:25:19 +0200 Subject: [PATCH 5/5] Fix --- .../src/main/scala/coursier/core/Remote.scala | 2 +- .../coursier/core/compatibility/package.scala | 25 ++++++++++++++----- 2 files changed, 20 insertions(+), 7 deletions(-) diff --git a/core-js/src/main/scala/coursier/core/Remote.scala b/core-js/src/main/scala/coursier/core/Remote.scala index 3963d5789..6e0376334 100644 --- a/core-js/src/main/scala/coursier/core/Remote.scala +++ b/core-js/src/main/scala/coursier/core/Remote.scala @@ -91,7 +91,7 @@ case class Remote(base: String, logger: Option[Logger] = None) extends Repositor for { xml <- \/.fromEither(eitherXml) _ = logger.foreach(_.other(url, "is XML")) - _ <- if (xml.label == "project") \/-(()) else -\/("Project definition not found") + _ <- if (xml.label == "project") \/-(()) else -\/(s"Project definition not found (got '${xml.label}')") _ = logger.foreach(_.other(url, "project definition found")) proj <- Xml.project(xml) _ = logger.foreach(_.other(url, "project definition ok")) diff --git a/core-js/src/main/scala/coursier/core/compatibility/package.scala b/core-js/src/main/scala/coursier/core/compatibility/package.scala index 86c0a614d..4c57573d7 100644 --- a/core-js/src/main/scala/coursier/core/compatibility/package.scala +++ b/core-js/src/main/scala/coursier/core/compatibility/package.scala @@ -40,6 +40,10 @@ package object compatibility { js.Dynamic.newInstance(defn)() } + // Can't find these from node + val ELEMENT_NODE = 1 // org.scalajs.dom.raw.Node.ELEMENT_NODE + val TEXT_NODE = 3 // org.scalajs.dom.raw.Node.TEXT_NODE + def fromNode(node: org.scalajs.dom.raw.Node): Xml.Node = { val node0 = node.asInstanceOf[js.Dynamic] @@ -53,15 +57,16 @@ package object compatibility { .map(l => List.tabulate(l.length)(l.item).map(fromNode)) .getOrElse(Nil) + // `exists` instead of `contains`, for scala 2.10 def isText = option[Int](node0.nodeType) - .exists(_ == 3) //org.scalajs.dom.raw.Node.TEXT_NODE + .exists(_ == TEXT_NODE) def textContent = option(node0.textContent) .getOrElse("") def isElement = option[Int](node0.nodeType) - .exists(_ == 1) // org.scalajs.dom.raw.Node.ELEMENT_NODE + .exists(_ == ELEMENT_NODE) override def toString = XMLSerializer.serializeToString(node).asInstanceOf[String] @@ -72,10 +77,18 @@ package object compatibility { def xmlParse(s: String): Either[String, Xml.Node] = { val doc = { if (s.isEmpty) None - else - dynOption(DOMParser.parseFromString(s, "text/xml")) - .flatMap(t => dynOption(t.childNodes)) - .flatMap(l => l.asInstanceOf[js.Array[js.Dynamic]].headOption.flatMap(option[org.scalajs.dom.raw.Node])) + else { + for { + xmlDoc <- dynOption(DOMParser.parseFromString(s, "text/xml")) + rootNodes <- dynOption(xmlDoc.childNodes) + // From node, rootNodes.head is sometimes just a comment instead of the main root node + // (tested with org.ow2.asm:asm-commons in CentralTests) + rootNode <- rootNodes.asInstanceOf[js.Array[js.Dynamic]] + .flatMap(option[org.scalajs.dom.raw.Node]) + .dropWhile(_.nodeType != ELEMENT_NODE) + .headOption + } yield rootNode + } } Right(doc.fold(Xml.Node.empty)(fromNode))