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

Drop support for sbt < 1.4 in code #11803

Open
mkurz opened this issue May 11, 2023 · 4 comments
Open

Drop support for sbt < 1.4 in code #11803

mkurz opened this issue May 11, 2023 · 4 comments

Comments

@mkurz
Copy link
Member

mkurz commented May 11, 2023

In PRs like #10648 we introduced lots of reflection code to handle sbt 0.13 - 1.3 and 1.4+
I think after 2.9 we can refactor lots of this code, e.g things like:

val name = vf.getClass.getSimpleName
name == "BasicVirtualFileRef" || name == "MappedVirtualFile"
}
}
def sourceMap(analysis: Analysis): Map[String, Source] = {
analysis.relations.classes.reverseMap.flatMap {
case (name, files) =>
files.headOption match { // This is typically a set containing a single file, so we can use head here.
case None => Map.empty[String, Source]
case JFile(file) => // sbt < 1.4
Map(name -> Source(file, MaybeGeneratedSource.unapply(file).flatMap(_.source)))
case VirtualFile(vf) => // sbt 1.4+ virtual file, see #10486
val names = vf.getClass.getMethod("names").invoke(vf).asInstanceOf[Array[String]]
val path =
if (names.head.startsWith("${")) { // check for ${BASE} or similar (in case it changes)
// It's an relative path, skip the first element (which usually is "${BASE}")
Paths.get(names.drop(1).head, names.drop(2): _*)
} else {
// It's an absolute path, sbt uses them e.g. for subprojects located outside of the base project
val id = vf.getClass.getMethod("id").invoke(vf).asInstanceOf[String]
// In Windows the sbt virtual file id does not start with a slash, but absolute paths in Java URIs need that
val extraSlash = if (id.startsWith("/")) "" else "/"
val prefix = "file://" + extraSlash
// The URI will be like file:///home/user/project/SomeClass.scala (Linux/Mac) or file:///C:/Users/user/project/SomeClass.scala (Windows)
Paths.get(URI.create(s"$prefix$id"));
}
Map(name -> Source(path.toFile, MaybeGeneratedSource.unapply(path.toFile).flatMap(_.source)))

@mkurz mkurz added this to the 2.10.0 milestone May 11, 2023
@Nezisi
Copy link

Nezisi commented May 12, 2023

I think it would be better to make SBT 1.8 mandatory for Play 2.9.

SBT 1.8 fixed a lot of critical bugs, plus the improved Scala 3 support and the Scala XML bump.

I think it would reduce a lot of burden on maintenance for the Play 2.9 release long-term, too.

I'll have a look at your mentioned code and will try to get a PR together, with additional removal of deprecated SBT syntax.

If you could give any more pointers, I'd be glad - otherwise I'll try to give an update over the course of the week.

Thanks for all your efforts :)

@mkurz
Copy link
Member Author

mkurz commented May 12, 2023

We don't need to change anything now, there is no pressure. Actually, I am planning to only officially support sbt 1.9.0+ for Play 2.9. We will mention that in the Release notes, so if people don't upgrade to 1.9.0 but report bugs, we tell them anything below sbt 1.9.0 is not supported so we don't fix any bugs related to that.

@Nezisi
Copy link

Nezisi commented May 12, 2023

That sounds like a really good plan.

Sorry, I misunderstood the bug report, it sounded like you wanted to keep the pre SBT 1.9 support alive for 2.9 .

@mkurz
Copy link
Member Author

mkurz commented May 12, 2023

it sounded like you wanted to keep the pre SBT 1.9 support alive for 2.9 .

It will be kept alive, and it will very likely work, but only bugs related to 1.9.0+ will be looked at and fixed, anything below we will just tell people to upgrade.

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

No branches or pull requests

2 participants