Fix packageWithJVMJar := true not working on Windows #115
Conversation
mdedetrich
force-pushed
the
add-test-for-packageWithJvmJar
branch
from
34ff6ca
to
51b4952
Compare
|
Found the most likely root cause, https://github.com/sbt/sbt-osgi/pull/81/files#diff-2530a9c505dd4cb34217eb195d5db3cab065049f033db306f485078da72edd5dR89-R90 would not work on Windows, trying a fix now. |
mdedetrich
force-pushed
the
add-test-for-packageWithJvmJar
branch
from
51b4952
to
ab55102
Compare
mdedetrich
changed the title
mdedetrich
force-pushed
the
add-test-for-packageWithJvmJar
branch
2 times, most recently
from
8a78930
to
d1d7403
Compare
mdedetrich
force-pushed
the
add-test-for-packageWithJvmJar
branch
from
d1d7403
to
02341db
Compare
|
Ended up being a simple fix, we used a hardcoded @eed3si9n @romainreuillon Do you mind taking a quick look at this? Its a fairly simple fix and its currently breaking Windows users. |
| @@ -181,7 +181,9 @@ private object Osgi { | |||
| import _root_.java.nio.file._ | |||
| import _root_.scala.collection.JavaConverters._ | |||
| val path = tmpArtifactDirectoryPath.toPath | |||
| Files.walk(path).iterator.asScala.map(f => f.toFile -> path.relativize(f).toString).filterNot { case (_, p) => p == "META-INF/MANIFEST.MF" }.toTraversable | |||
| Files.walk(path).iterator.asScala.map(f => f.toFile -> path.relativize(f)) | |||
| .collect { case (f, p) if p != (file("META-INF") / "MANIFEST.MF").toPath => (f, p.toString) } | |||
There was a problem hiding this comment.
So the fix is using the / operator?
There was a problem hiding this comment.
This kind of problem I was fixed in pekko/akka spec too:)
There was a problem hiding this comment.
So the fix is using the
/operator?
Yes its part of the sbt API which is just a wrapper over new java.io.File(a, b)
There was a problem hiding this comment.
I guess I understand the reason, Because line.separator in Windows cannot be represented with /.
|
|
|
@mdedetrich Sorry I am under a lot of load at work right now, but the fix seems okay to me. If I remember correctly, we filter of that file to avoid a duplicate entry error with the manifest provided as parameter of to IO.jar |
|
@romainreuillon Take care, me too, heavy work..., I can take a try locally when I find time, still testing the Virtual thread reafactory at work:( |
Thanks, even though you didn't make an actual approval via Github UI I will accept this as a yes and use admin rights to merge given its currently its broken for Windows users. |
| @@ -181,7 +181,9 @@ private object Osgi { | |||
| import _root_.java.nio.file._ | |||
| import _root_.scala.collection.JavaConverters._ | |||
| val path = tmpArtifactDirectoryPath.toPath | |||
| Files.walk(path).iterator.asScala.map(f => f.toFile -> path.relativize(f).toString).filterNot { case (_, p) => p == "META-INF/MANIFEST.MF" }.toTraversable | |||
| Files.walk(path).iterator.asScala.map(f => f.toFile -> path.relativize(f)) | |||
| .collect { case (f, p) if p != (file("META-INF") / "MANIFEST.MF").toPath => (f, p.toString) } | |||
There was a problem hiding this comment.
I guess I understand the reason, Because line.separator in Windows cannot be represented with /.
|
@Roiocam This has already been merged, apache/pekko#1024 still needs to be reviewed to fix the issue in Pekko |
Resolves: #113