Replace ipcsocket with junixsocket library #1852
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
lefou
reviewed
Apr 29, 2022
There was a problem hiding this comment.
Very nice. I like it. Thank you, @sake92 !
There are some failing CI tests.
mill.main.ClientServerTests.hello.envVars is most likely caused by your change.
-------------------------------- Running Tests --------------------------------
Error: Exception in thread "Thread-0" java.lang.NoClassDefFoundError: Could not initialize class org.newsclub.net.unix.AFUNIXSocketAddress
at mill.main.Server.$anonfun$run$2(MillServerMain.scala:101)
at scala.runtime.java8.JFunction0$mcV$sp.apply(JFunction0$mcV$sp.scala:18)
at mill.main.Server$.lockBlock(MillServerMain.scala:226)
at mill.main.Server.$anonfun$run$1(MillServerMain.scala:98)
at scala.runtime.java8.JFunction0$mcV$sp.apply(JFunction0$mcV$sp.scala:18)
at mill.main.Server$.tryLockBlock(MillServerMain.scala:234)
at mill.main.Server.run(MillServerMain.scala:95)
at mill.main.ClientServerTests$.$anonfun$spawnEchoServer$1(ClientServerTests.scala:67)
at java.base/java.lang.Thread.run(Thread.java:829)
X mill.main.ClientServerTests.hello.envVars 5030ms
java.lang.Exception: Failed to connect to server
mill.main.client.MillClientMain.run(MillClientMain.java:197)
mill.main.ClientServerTests$.$anonfun$runClientAux$1(ClientServerTests.scala:86)
mill.main.Server$.lockBlock(MillServerMain.scala:226)
mill.main.ClientServerTests$.runClientAux(ClientServerTests.scala:77)
mill.main.ClientServerTests$.runClient$1(ClientServerTests.scala:96)
mill.main.ClientServerTests$.$anonfun$tests$2(ClientServerTests.scala:107)
java.lang.NoClassDefFoundError: Could not initialize class org.newsclub.net.unix.AFUNIXSocketAddress
mill.main.client.MillClientMain.run(MillClientMain.java:189)
mill.main.ClientServerTests$.$anonfun$runClientAux$1(ClientServerTests.scala:86)
mill.main.Server$.lockBlock(MillServerMain.scala:226)
mill.main.ClientServerTests$.runClientAux(ClientServerTests.scala:77)
mill.main.ClientServerTests$.runClient$1(ClientServerTests.scala:96)
mill.main.ClientServerTests$.$anonfun$tests$2(ClientServerTests.scala:107)
But I have no idea about these:
-------------------------------- Running Tests --------------------------------
X mill.integration.thirdparty.local.JawnTests.scala21111 19ms
java.lang.Exception: Resource amm-dependencies.txt not found
ammonite.main.Defaults$.alreadyLoadedDependencies(Defaults.scala:43)
ammonite.MainRunner.initMain(MainRunner.scala:141)
mill.main.MainRunner.initMain(MainRunner.scala:167)
mill.main.MainRunner.watchLoop2(MainRunner.scala:67)
mill.main.MainRunner.runScript(MainRunner.scala:92)
mill.util.ScriptTestSuite.eval(ScriptTestSuite.scala:61)
mill.integration.thirdparty.JawnTests.check$1(JawnTests.scala:13)
mill.integration.thirdparty.JawnTests.$anonfun$tests$11(JawnTests.scala:31)
X mill.integration.thirdparty.local.JawnTests.scala2123 20ms
java.lang.Exception: Resource amm-dependencies.txt not found
ammonite.main.Defaults$.alreadyLoadedDependencies(Defaults.scala:43)
ammonite.MainRunner.initMain(MainRunner.scala:141)
mill.main.MainRunner.initMain(MainRunner.scala:167)
mill.main.MainRunner.watchLoop2(MainRunner.scala:67)
mill.main.MainRunner.runScript(MainRunner.scala:92)
mill.util.ScriptTestSuite.eval(ScriptTestSuite.scala:61)
mill.integration.thirdparty.JawnTests.check$1(JawnTests.scala:13)
mill.integration.thirdparty.JawnTests.$anonfun$tests$12(JawnTests.scala:32)
-------------------------------- Running Tests --------------------------------
X mill.integration.thirdparty.local.AcyclicTests.scala2118 18ms
java.lang.Exception: Resource amm-dependencies.txt not found
ammonite.main.Defaults$.alreadyLoadedDependencies(Defaults.scala:43)
ammonite.MainRunner.initMain(MainRunner.scala:141)
mill.main.MainRunner.initMain(MainRunner.scala:167)
mill.main.MainRunner.watchLoop2(MainRunner.scala:67)
mill.main.MainRunner.runScript(MainRunner.scala:92)
mill.util.ScriptTestSuite.eval(ScriptTestSuite.scala:61)
mill.integration.thirdparty.AcyclicTests.check$1(AcyclicTests.scala:11)
mill.integration.thirdparty.AcyclicTests.$anonfun$tests$12(AcyclicTests.scala:27)
scala.runtime.java8.JFunction0$mcV$sp.apply(JFunction0$mcV$sp.scala:18)
mill.util.TestUtil$.disableInJava9OrAbove(TestUtil.scala:84)
mill.integration.thirdparty.AcyclicTests.$anonfun$tests$11(AcyclicTests.scala:27)
X mill.integration.thirdparty.local.AcyclicTests.scala2125 10ms
java.lang.Exception: Resource amm-dependencies.txt not found
ammonite.main.Defaults$.alreadyLoadedDependencies(Defaults.scala:43)
ammonite.MainRunner.initMain(MainRunner.scala:141)
mill.main.MainRunner.initMain(MainRunner.scala:167)
mill.main.MainRunner.watchLoop2(MainRunner.scala:67)
mill.main.MainRunner.runScript(MainRunner.scala:92)
mill.util.ScriptTestSuite.eval(ScriptTestSuite.scala:61)
mill.integration.thirdparty.AcyclicTests.check$1(AcyclicTests.scala:11)
mill.integration.thirdparty.AcyclicTests.$anonfun$tests$13(AcyclicTests.scala:28)
I saw these before, but sporadicly. Maybe, we can ignore them for now. Might be self-fixed in next CI run.
|
@lefou thanks! |
|
@sake92 Can you rebase on latest main branch (or merge)? I made some changes that should fix the tests which fail with the weird missing |
|
This is the error message from CI: |
lefou
reviewed
May 2, 2022
| if (scala.util.Properties.isWin) { | ||
| // workaround for CI issue | ||
| // https://github.com/com-lihaoyi/mill/pull/1852#issuecomment-1114332274 | ||
| System.setProperty("os.name", "Windows10") |
There was a problem hiding this comment.
Seems legit, although I'd like to find a solution, which does not tamper with JVM default properties. Maybe, setting the org.newsclub.net.unix.library.override instead also works around the wrong detection on Windows?
There was a problem hiding this comment.
Might not work that easily, as this property is meant to point to an unpacked library.
I will merge as is. If you by any change find a better solution, please open a new PR.
There was a problem hiding this comment.
Yeah, thought about it but it requires a full path. I will make an issue or fix upstream.
|
Thank you, @sake92 ! |
Unfortunately, this error also occurs for all GitHub workflows / actions that test with "windows" or "windows-latest", even in |
|
I made a PR upstream, hope it gets merged soon kohlschutter/junixsocket#105 |
|
Great, although I wonder why this issue even happens when we run in |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
https://github.com/kohlschutter/junixsocket seems much more robust, especially for Windows.
sbt-ipcsocket implements windows support via named pipes, which are similar to unix pipes.
junixsocket uses named pipes directly on windows, since they are supported now in Win10 (from 2017).
https://kohlschutter.github.io/junixsocket/unixsockets.html
https://devblogs.microsoft.com/commandline/af_unix-comes-to-windows/
Fixes #1310