Skip to content

Conversation

@nightscape
Copy link

No description provided.

@nightscape
Copy link
Author

@eed3si9n the PR for the 1.x branch only has two errors. In the merging / merging test there are additional unexpected newlines.
In sbt-assembly / piecemeal there's a java.lang.ClassNotFoundException: Main, potentially caused by java.lang.ClassNotFoundException: scala.Predef$.
Would you have a pointer of what could be causing these?

@nightscape nightscape mentioned this pull request Jun 27, 2022
@fnqista
Copy link

fnqista commented Jul 11, 2022

@nightscape It seems like even the 1.x version doesn't work on windows? Might be we need to fix more than what you mentioned in #472

@nightscape
Copy link
Author

@fnqista yes, the two errors I mentioned above are unrelated to the Illegal character issue that I actually wanted to expose.
Unfortunately I don't have much time left to dig into this, so I was hoping that one of the maintainers could help out or point me into the right direction.

@fnqista
Copy link

fnqista commented Oct 4, 2022

@nightscape I have had a chance to look at this (tbh I almost didn't want to because I dislike Windows development, but I am responsible for the 2.0 changes so I feel obligated to help):

  • merging / merging fails in both in both 1.x and 2.x because we use System.getProperty(lineSeparator) and that causes the file to be written with CRLF endings in Windows. It can be fixed by using just \n
  • sbt-assembly / piecemeal fails in both in both 1.x and 2.x because of the wrong classpath syntax used by the jar runner in src/sbt-test/sbt-assembly/piecemeal/build.sbt. The separator for the classpath java -cp in Windows is ; instead of :
  • all other tests fail only in 2.x because of the error you reported in IllegalArgumentException: Illegal character in opaque part at index 11: jar:file:C:\... #472

Edit: There is another failure in 2.x, where we use System.getProperty(file.separator) for PathList.unapply, which we should not. We should just use / since every class file goes through the shader that converts all \ to /.

I can prepare a new PR with all the fixes for the above. Can you get Eugene to merge your PR first?

@nightscape
Copy link
Author

@fnqista the other PR is merged 👍

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.

2 participants