Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

compatibility with sbt 1.4.7 and scala 2.12.13 #376

Closed
wants to merge 2 commits into from

Conversation

domizei385
Copy link
Contributor

As suggested in #375 here is a PR for my changes.

@@ -17,13 +17,16 @@ libraryDependencies ++= Seq(

TaskKey[Unit]("verify-classpath-xml") := {
val dir = baseDirectory.value
val repo = "/.cache/coursier/v1/https/repo1.maven.org/maven2"

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

will this work on Macs?
my coursier cache lives in ~/Library/Caches/Coursier/v1

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is only really relevant while building and testing the plug-in prior to its release. of course if you are doing just that it will fail.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yah I figured it might be an issue because im on a Mac M1 and wanted to help diagnose the failing test

@bpossolo
Copy link

can you provide some guidance on how to reproduce the failed test?
I checked out your PR, then launched the sbt console and ran test

sbt:sbteclipse-plugin> test
[info] ScalaVersionSpec:
[info] ScalaVersion
[info] - should parse Scala version "2.12.0"
[info] - should parse Scala version "2.12.0-SNAPSHOT"
[info] - should parse Scala version "2.12.0-RC10"
[info] - should parse Scala version "2.12.0-M1"
[info] - should parse Scala version "2.12.0-51e77037f2adc4ffa7421aa36803a5874292b70d"
[info] - should fail to parse "2.12"
[info] Run completed in 72 milliseconds.
[info] Total number of tests run: 6
[info] Suites: completed 1, aborted 0
[info] Tests: succeeded 6, failed 0, canceled 0, ignored 0, pending 0
[info] All tests passed.
[success] Total time: 1 s, completed Feb 11, 2021, 4:39:16 PM

@domizei385
Copy link
Contributor Author

It is actually the integration test that is failing. You have to run sbt scripted to start those, but they should give plenty of errors on your machine.

@bpossolo
Copy link

thanks. I see it now

[error] (scripted) Failed tests:
[error] 	sbteclipse/02-contents
[error] 	sbteclipse/03-sbt-dependency-withsources
[error] 	sbteclipse/04-dependency-options
[error] Total time: 55 s, completed Feb 11, 2021, 6:38:49 PM

@domizei385
Copy link
Contributor Author

I have added the path mapping for macOS, so you should be left with less issues (assuming the path is correct :)

@bpossolo
Copy link

[error] (scripted) Failed tests:
[error] 	sbteclipse/02-contents
[error] Total time: 55 s, completed Feb 14, 2021, 9:53:47 AM

🌞

@bpossolo
Copy link

The only remaining failure appears to be due to a relative/absolute path difference

expected

<classpathentry kind="lib" path="./lib_managed/biz/aQute/bnd/biz.aQute.bndlib/3.4.0/biz.aQute.bndlib-3.4.0.jar" />

actual

<classpathentry path="/Users/benjamin/Library/Caches/Coursier/v1/https/repo1.maven.org/maven2/biz/aQute/bnd/biz.aQute.bndlib/3.4.0/biz.aQute.bndlib-3.4.0.jar" kind="lib"/>

also the lib and path attributes are reversed... I'm not sure if its checking for exact string matches or just that they're semantically equivalent.

@bpossolo
Copy link

bpossolo commented Mar 1, 2021

whatever is creating the Lib (EclipseClasspathEntry) objects is passing an absolute path into the constructor.

case class Lib(
  path: String,
  sourcePath: Option[String] = None,
  javadocPath: Option[String] = None
) extends EclipseClasspathEntry {
  override def toXml = {
    val classpathentry = sourcePath.foldLeft(<classpathentry kind="lib" path={ path }/>)((xml, sp) => {
      println("path:" + path)
      xml % Attribute("sourcepath", Text(sp), Null)
    })
  ...
}

console:

path:/Users/benjamin/Library/Caches/Coursier/v1/https/repo1.maven.org/maven2/biz/aQute/bnd/biz.aQute.bndlib/3.4.0/biz.aQute.bndlib-3.4.0.jar

I can't tell where those EclipseClasspathEntry objects are being created.
also, this broken test seems unrelated to your changes...

edit: I see they're being created here: https://github.com/sbt/sbteclipse/blob/master/src/main/scala/com/typesafe/sbteclipse/core/Eclipse.scala

@normana400
Copy link

is this PR delaying the generation of the 6.0 release?

@bpossolo
Copy link

bpossolo commented Nov 9, 2021

@normana400 this project isn't really maintained anymore (last activity was over 2 years ago).

@mkurz
Copy link
Member

mkurz commented Nov 12, 2021

I am pretty sure I know were the problem is and just need to find some time to finally make sbteclipse work with latest sbt 1.4+.
Actually it's on my TODO list already.
Anyway, I just want to let you know, that if you want to financially support Play, you can do that now via https://opencollective.com/playframework - that would allow contributors like me to spend more time working on it. Thanks!

Copy link
Contributor

@benmccann benmccann left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I merged a bunch of scala-steward changes, so you don't need a lot of the changes to the project setup files like build.sbt anymore

It might be nice to break this PR into a few like refactoring out the method to get the artifact home directory. Upgrading first to sbt 1.3. And then upgrading to sbt 1.4

@@ -8,15 +8,15 @@ Installation and Basic Usage

- Open your plugin definition file (or create one if doesn't exist). You can use either:

- the global file (for version 0.13 and up) at *~/.sbt/SBT_VERSION/plugins/plugins.sbt*
- the global file (for version 1.0 and up) at *~/.sbt/SBT_VERSION/plugins/plugins.sbt*
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

was this statement wrong?

@@ -1,6 +1,6 @@
import _root_.bintray.BintrayPlugin.bintrayPublishSettings

val baseVersion = "6.0.0-SNAPSHOT"
val baseVersion = "6.0.0-M1"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

let's not change this. I think it's handled by the release process and it will throw things off if it's manually changed

@mkurz
Copy link
Member

mkurz commented May 6, 2022

@benmccann I can work on this soon, it's not difficult to fix. btw, are you still using Play?

@benmccann
Copy link
Contributor

Hey, I'm not really using Play anymore. I got a new computer a year or two ago and I just started trying to get an old personal project that's using Play running on it and happened to notice there was a bit of a backlog here. I don't really have time to devote to this project, but figured I'd merge all the PRs since they were just easy library upgrades.

I just setup a CI to make it easier to merge PRs without worrying about breaking things. A few of them seemed to work on my computer, but not on the CI, so I had to revert a couple of the upgrades I just merged. I'd recommend rebasing this PR against master so that it runs against the CI before merging anything.

@benmccann benmccann linked an issue May 6, 2022 that may be closed by this pull request
@mkurz
Copy link
Member

mkurz commented Sep 19, 2022

Closing in favor of #402

@mkurz mkurz closed this Sep 19, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Error using sbteclipse with SBT 1.4.0
5 participants