sbt / sbt-pom-reader

Translates xml -> awesome. Maven-ish support for sbt.
Other
76 stars 26 forks source link

HTTP TransporterFactory #48

Closed blast-hardcheese closed 6 years ago

blast-hardcheese commented 6 years ago

HTTP parent poms were unable to be resolved, since it seems as though the hacked wheel resolver delegated to HTTP and HTTPS

This manifested as:

[error] java.lang.NullPointerException
[error]         at org.eclipse.aether.internal.impl.DefaultRepositoryConnectorProvider.newRepositoryConnector(DefaultRepositoryConnectorProvider.java:106)
[error]         at org.eclipse.aether.internal.impl.DefaultArtifactResolver.performDownloads(DefaultArtifactResolver.java:494)
[error]         at org.eclipse.aether.internal.impl.DefaultArtifactResolver.resolve(DefaultArtifactResolver.java:399)
[error]         at org.eclipse.aether.internal.impl.DefaultArtifactResolver.resolveArtifacts(DefaultArtifactResolver.java:224)
[error]         at org.eclipse.aether.internal.impl.DefaultArtifactResolver.resolveArtifact(DefaultArtifactResolver.java:201)
[error]         at org.eclipse.aether.internal.impl.DefaultRepositorySystem.resolveArtifact(DefaultRepositorySystem.java:260)
[error]         at com.typesafe.sbt.pom.MyModelResolver.resolveModel(MyModelResolver.scala:51)
[error]         at com.typesafe.sbt.pom.MyModelResolver.resolveModel(MyModelResolver.scala:36)
[error]         at org.apache.maven.model.building.DefaultModelBuilder.readParentExternally(DefaultModelBuilder.java:1051)
[error]         at org.apache.maven.model.building.DefaultModelBuilder.readParent(DefaultModelBuilder.java:829)
[error]         at org.apache.maven.model.building.DefaultModelBuilder.build(DefaultModelBuilder.java:331)
[error]         at com.typesafe.sbt.pom.MvnPomResolver.loadEffectivePom(MavenPomResolver.scala:59)
[error]         at com.typesafe.sbt.pom.package$.loadEffectivePom(package.scala:32)
[error]         at com.typesafe.sbt.pom.MavenProjectHelper$.makeProjectTree(MavenProjectHelper.scala:120)
[error]         at com.typesafe.sbt.pom.MavenProjectHelper$.makeReactorProject(MavenProjectHelper.scala:47)
[error]         at com.typesafe.sbt.pom.PomBuild.projectDefinitions(PomBuild.scala:23)
[error]         at com.typesafe.sbt.pom.PomBuild.projectDefinitions$(PomBuild.scala:20)
[error]         at MyBuild$.projectDefinitions(build.scala:2)
[error]         at sbt.internal.Load$.projectsFromBuild(Load.scala:790)
[error]         at sbt.internal.Load$.$anonfun$loadUnit$7(Load.scala:696)
[error]         at scala.collection.immutable.Stream.flatMap(Stream.scala:486)
[error]         at sbt.internal.Load$.$anonfun$loadUnit$1(Load.scala:696)
[error]         at sbt.internal.Load$.timed(Load.scala:1343)
[error]         at sbt.internal.Load$.loadUnit(Load.scala:677)
[error]         at sbt.internal.Load$.$anonfun$builtinLoader$4(Load.scala:477)
blast-hardcheese commented 6 years ago

Pulling in some changes from the scalacenter branch as well; I rebased scalacenter/master ontop of this branch as well, to make it easier to move forward going forward: blast-hardcheese/fixing-deps...integrating-scalacenter-changes

jvican commented 6 years ago

IIRC the magic part is to add the basic repository plexus component, not sure if all the changes to maven Pom resolver are required.

One thing I’ve never understood is why this repository doesn’t resolve dependencies at all... I also had to run integration tests to catch the NPEs I was getting.

blast-hardcheese commented 6 years ago

I can reorder the commits if you'd prefer, I was trying to get our two branches to converge

blast-hardcheese commented 6 years ago

I also did a few tests -- Only registering RepositoryConnectorFactory maintained the NPE, but once I added the http transporter everything started working as before. Renaming those files definitely adds noise though, so I can abandon that if you'd prefer

jvican commented 6 years ago

Don’t waste your time :wink: these changes look great as they are. Is it possible that you add just one test that triggers resolution to make sure we don’t regress?

blast-hardcheese commented 6 years ago

@jvican Is there an upstream POM I can target, possibly an LTS from scalacenter? I was looking at spinning up a webserver to host an HTTP parent pom in the tests, but it looked to be getting more and more complex

jvican commented 6 years ago

@blast-hardcheese Do you mean a pom.xmlfor the Maven build or a *.pom file? If it's the latter, you can target http://search.maven.org/#artifactdetails%7Corg.scala-sbt%7Czinc_2.12%7C1.1.0%7Cjar, it's not from Scala Center's but it's a LTS that should be always accessible.

Thanks for your time on this @blast-hardcheese.

blast-hardcheese commented 6 years ago

@jvican Hey, sorry for the late reply here -- I just took a look, it turns out we already have a test using remote parent poms:

From src/sbt-test/simple-pom/can-extract-from-remote-parent-pom/pom.xml:

  <parent>
    <groupId>org.apache</groupId>
    <artifactId>apache</artifactId>
    <version>13</version>
  </parent>

I'm not sure why this didn't surface the error.

jvican commented 6 years ago

Alright, shall we merge then? If you've checked this works I'm happy to push the button. By the way, I'm sending you a collaboration invite.

blast-hardcheese commented 6 years ago

@jvican Yeah, let me open an issue to track the broken test. If you're able to release, that would be fantastic.

jvican commented 6 years ago

You've been doing great work. I think you deserve merge rights.

@eed3si9n @dwijnand Could you add @blast-hardcheese to the sbt-pom-reader github admin group and give him bintray rights to publish? Thank you in advance.