sbt / sbt-osgi

sbt plugin for creating OSGi bundles
Apache License 2.0
47 stars 43 forks source link

Fix packageWithJVMJar := true not working on Windows #115

Closed mdedetrich closed 7 months ago

mdedetrich commented 7 months ago

Resolves: https://github.com/sbt/sbt-osgi/issues/113

mdedetrich commented 7 months ago

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 commented 7 months ago

Ended up being a simple fix, we used a hardcoded "META-INF/MANIFEST.MF" as a string which obviously fails on Windows, the solution was to use the proper file/path API which takes into account the different separators for Windows.

@eed3si9n @romainreuillon Do you mind taking a quick look at this? Its a fairly simple fix and its currently breaking Windows users.

He-Pin commented 7 months ago

image It's tested with Windows.

romainreuillon commented 7 months ago

@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

He-Pin commented 7 months ago

@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:(

mdedetrich commented 7 months ago

@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

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.

mdedetrich commented 7 months ago

@Roiocam This has already been merged, https://github.com/apache/incubator-pekko/pull/1024 still needs to be reviewed to fix the issue in Pekko