Fix compilation on FreeBSD. #3625
Conversation
If this will work on all platforms, you could just change it here.
I am not sure here but it should be in the javadocs and should be true on all JVMs, not just FreeBSD for "write once run anywhere". If this is the case then I think an issue specific to this problem should be added. Others more familiar should weigh in. |
|
I don't think the change to link to I'll post another comment later about the networking issue. Anyway, these comments are just to let everyone be aware that there are still things to improve/fix, but they are independent from what this PR fixes. |
Yes, this does not work on macOS so I am guessing Linux as well. Maybe it can be installed differently or built easily for better instructions/docs? |
|
@alexdupre Thank you for the FreeBSD work here. It would be nice to get FreeBSD TL; DR - There may be a fire here, but I believe it is not a barn fire. The problems you report sound strange. In January or so of 2023(?) I had FreeBSD building and passing almost all of the Two or so months ago, I had a private FreeBSD building on, I think FreeBSD 13.0 (on AMD) (13.2?). I would have to review my work, but to my memory, no special linking was required. I will have to re-check my now FreeBSD 14.0 system to see which JDK I was using, probably I will read thru the changes of this PR. |
| @@ -1,5 +1,5 @@ | |||
|
|
|||
| #if defined(__x86_64__) && (defined(__linux__) || defined(__APPLE__)) | |||
| #if defined(__x86_64__) && (defined(__linux__) || defined(__APPLE__) || defined(__FreeBSD__)) | |||
There was a problem hiding this comment.
I concur with this change.
I never tried running on 32 bit FreeBSD. If the intent is
to support and & test such (which I doubt SN should be doing in the absence of a champion/advocate). does the
corresponding setjmp_amd32.s need a corresponding change?
nativelib/src/main/resources/scala-native/gc/shared/Safepoint.c
Outdated
Show resolved
Hide resolved
nativelib/src/main/resources/scala-native/gc/shared/Safepoint.c
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
CI failures
-
Needs
scripts/clangfmtto be executed, so that the lint-check is happy. -
Two Windows runs are not happy.
-
one is still running after many hours, probably hung in a broken exception handler.
Later: the test finally finished with a "timeout", looks like a socket read hung for some reason, -
the other one has a hard failure in what appears to be
Safepoint.c. On first reading of the log
I do not know if that failure is related to this PR or preexisting. (Changes of this PR look
innocent enough, but that is why we run CI).
-
Fixing the first problem will cause a new, full CI to run. We can monitor the fresh run and see what happens with
the Windows tests.
Once CI passes
Otherwise the changes of this PR Looks Good To Me (LGTM).
This PR contains the changes non-Test changes I made in private code in October, 2023 to get FreeBSD (13.2?) running on AMD hardware (well, VM on AMD hardware).
I expect the 13 or so Test files (unit-tests and ./tools/src/test/scala/scala/scalanative/linker/MinimalRequiredSymbolsTest.scala.original) to continue to fail.
Having this PR merged will make feeding fixed FreeBSD 14 versions of those tests
back into mainline, perhaps a few at a time.
There are a couple of point suggestions, not requirements. They should
not keep this PR from moving forward.
Because SN CI does not exercise FreeBSD (long story), it would be handy if the
base PR description described exactly which FreeBSD versions and architectures
were tested.
|
The failure of these tests should not keep this PR from progressing. To co-ordinate work: I expect at least the following tests to continue to fail. I have fixed copies of many of these and will try Least I forget my manners, @alexdupre, which tests would be most useful to you first (after this PR?) |
Could you create seperate Issues for each of these so that they do not get lost? That will also make it easier Each will require focused study. The first issue deals with multithreading, which a whole other box. Is the FreeBSD |
|
Some additional context:
so I guess FreeBSD/i386 is not actually supported. I don't see it as a problem, since it's now a tier2 platform and I don't think anyone will ever want to run scala-native on it, so we should probably create a separate branch which errors out in this scenario.
This is due to the missing Linking it with the correct library make it working, I'll get in touch with the FreeBSD port maintainer to understand the reason of this separation between non-threaded and threaded.
|
|
Given the differences in Java 8 vs Java 11, 17, and 21 I would document that FreeBSD should use Java 11+. Most projects are dropping Java 8 and there is no sense in trying to support old stuff any way. Scala Native still uses Java 8 in CI but I would be for dropping it entirely. I am sure you are doing |
|
I'm fine with requiring java 11+, but we need to find a good/agreed solution for the I see two solutions:
I think the former is better, since it doesn't require any action by the user. It might break on an IPv6-only machine, though (not quite common). |
|
@alexdupre You have been busy & productive. Good to see. There are a number of topic or work-items in this discussion so far. I think we need some way of My time is limited right now, but let me give some quick, and I hope helpful & encouraging, thoughts.
|
FreeBSD/i386 doesn't seem to be supported.
Simplify the logic and improve the explanation of the differences.
|
I think to have addressed all your concerns/suggestions in my latest commits. The change to TestMain should make it working OOTB on FreeBSD with stock config and any java version. For the records, with these changes and the boehm-threaded hack I was able to successfully run all tests of https://github.com/lampepfl/gears As noted in another github issue some native tests don't currently terminate on FreeBSD. The reason to have "java.net.preferIPv4Stack=true" by default on BSD derives from the fact that IPv4-mapped IPv6 addresses are disabled by default for security reasons (https://datatracker.ietf.org/doc/html/draft-cmetz-v6ops-v4mapped-api-harmful-01). On OpenBSD there isn't even the ability to re-enable them, AFAIK. |
|
Please refer to our contributing docs for info about Java source code. https://scala-native.org/en/stable/contrib/contributing.html |
If you are referring to the Scala CLA I think I already signed it in the past. Anyway, I just signed it again. |
|
I was mostly referring to the links supplied which seem to be pointing to OpenJDK code. |
Well, those links have just the purpose to show the jdk code running on BSD platforms, no code has been brought into this PR. |
| @@ -31,53 +31,27 @@ object TestMain { | |||
| final val iPv4Loopback = "127.0.0.1" | |||
| final val iPv6Loopback = "::1" | |||
|
|
|||
| private def getFreeBSDLoopbackAddr(): String = { | |||
| private def setFreeBSDWorkaround(): Unit = { | |||
There was a problem hiding this comment.
The "not allowing IPv4 mapped IPv6 addresses," does not match my memory of what was working for me circa 2023-10-15. When I take a run, hopefully soon, at fixing
unit-tests for FreeBSD, I will have to refresh my memory.
I believe I was running most unit-tests with an unmodified
TestMain on IPv6. If that was and continues to be true, this section may need to be re-worked in another PR.
I believe this edit can proceed because my only evidence is two month old memory.
There was a problem hiding this comment.
The fact that you had to introduce iPv6Loopback and use it on FreeBSD (even if openjdk8 on FreeBSD has dual mode sockets) was exactly because IPv4 mapped IPv6 addresses are forbidden, so using 127.0.0.1 on an AF_INET6 socket wasn't working.
If you enable them, then on Java8 there isn't anything else to do, but on java 11+ the server will continue to listen only on tcp4, unless java.net.preferIPv4Stack is explicitely set to false on sbt. Manually adjusting both settings seems much more complex than automatically push SN to use an IPv4 socket.
test-interface/src/main/scala/scala/scalanative/testinterface/TestMain.scala
Outdated
Show resolved
Hide resolved
test-interface/src/main/scala/scala/scalanative/testinterface/TestMain.scala
Outdated
Show resolved
Hide resolved
test-interface/src/main/scala/scala/scalanative/testinterface/TestMain.scala
Outdated
Show resolved
Hide resolved
|
|
||
| *Note 3:* Using the boehm GC with multi-threaded binaries doesn't work | ||
| out-of-the-box yet. | ||
|
|
There was a problem hiding this comment.
Given that a number of Tests, primarily unit-tests, are known to not execute, it may be worthwhile to add a "Note 4" which says something like:
"
A number of Tests, primarily unit-tests, are known to fail
on FreeBSD. It is believed that the code-under-test is correct and that the defect is in the Test itself.
Work is underway to fix these tests. See PR 3625 for a list of tests known to fail at that time."
I/we could create an Issue, with checkboxes, which lists
the known failing tests (Java 8) and substitute that URL into the text above.
That way, we are saying what we are doing and doing what
we say.
The last PR in the series which fixes these tests would delete this Note. I usually dislike creating work items which have to be deleted later, but this work is in flux.
Your thoughts?
|
I've pushed some other improvements, I think I'm done. |
|
Thank you for the info about FreeBSD dual stack. Later: I read the referenced draft RFC from the turn of the century. I then read For reference, the Java 20 (Networking Properties)[https://docs.oracle.com/en/java/javase/20/docs/api/java.base/java/net/doc-files/net-properties.html] says: Java 20 was the latest I found with a 20 second google search for Java 21. The quoted text is substantially FreeBSD (and other BSD?) may be setting some OS properties outside of the JDK to force IPv4. |
|
I think this PR is almost done. The work you have put into evolutions shows. I'd like to give Wojciech, who will probably be the one doing the merge, To move it towards conclusion and capture unresolved concerns, I suggest creating at least two formal
Your thoughts? |
|
Why do you think the IPv4/dual-stack issue is still unresolved? What would be the purpose of the new github issue? I've contacted the boehm-gc maintainer today and I hope he'll change the package to install the threaded libraries with the standard name, as done by other OS, so no change will be needed by SN. I'm waiting for his feedback. |
The idea is to convince the person doing the merge is that "due diligence" has been done. If there is an OS specific hack necessary, I would like to understand it. Not the least because such will I have read the reports of dual-stack dragon encounters and believe them. Forewarned is forearmed. Having an Issue, from a person other than me, would help keep the issue from falling off the bottom of my
Thank you for doing that. Please keep posted. It would be nice to not have to do anything here. |
| @@ -3,7 +3,7 @@ | |||
| set -e | |||
|
|
|||
| HERE="`dirname $0`" | |||
| VERSION=$(sed -nre "s#version[^0-9]+([0-9.]+)#\1#p" $HERE/../.scalafmt.conf) | |||
| VERSION=$(sed -nre "s#[[:space:]]*version[^0-9]+([0-9.]+)#\1#p" $HERE/../.scalafmt.conf) | |||
There was a problem hiding this comment.
Sorry to repeat myself. Has this been run manually on Windows?
We, you & I, are going to win no friends if we break SN development on
Windows.
CI runs on Ubuntu Linux. Unless we know that this has been successfully manually run on Windows, we should assume it broken.
As mentioned, I can run this on Windows if such would be hard for you.
There was a problem hiding this comment.
I'm not sure what's the build env on windows. I've tried on mingw64 and it worked, I don't know what exact sed implementation uses SN on Windows. To be 100% sure it's better if you try yourself.
There was a problem hiding this comment.
Now that I look at this more closely, I realize that I am a goat, the barnyard critter, not the
"Greatest of all time" kind.
This is obviously a bash script, which will not run on plain Windows to begin with. Arrgh! 🤦
It might be run on WSL (linux on Windows), but any sed there should handle the POSIX
[[:space:]]
Sorry for the false trigger.
|
CI failure summary TL;DR I believe the two failures and the several multi-arch warnings on Windows are unrelated to The following breakage in this also occurs in merged PR #3626. Breakage may have entered 3 Multiarch warnings 2 runtime failures, TestMain loopback connection failing, but only on Scala 2 & Windows. |
|
I believe this PR is ready for review & merge at your convenience. There is a long & complicted conversation trail for this PR. The issues raised and not resolved in A number of When this PR is merged, I will peck away at merging my private unit-test fixes. |
* Fix compilation on FreeBSD. * Explicitly error out on unsupported platforms. * Make the MAP_NORESERVE change specific for FreeBSD. * Fix the sed expression to run on FreeBSD, too. * Make the TestMain to run on all java versions on FreeBSD. * Update the documentation for FreeBSD. * Restore the ability to skip leading whitespaces in a more portable way. * Add a note about failing tests. (cherry picked from commit 963666d)
* Fix compilation on FreeBSD. * Explicitly error out on unsupported platforms. * Make the MAP_NORESERVE change specific for FreeBSD. * Fix the sed expression to run on FreeBSD, too. * Make the TestMain to run on all java versions on FreeBSD. * Update the documentation for FreeBSD. * Restore the ability to skip leading whitespaces in a more portable way. * Add a note about failing tests. (cherry picked from commit 963666d)
This patch allows to successfully run
sandbox3/runon FreeBSD.This is not enough to have a fully working scala-native setup, but many applications will work.
There are at least a couple of additional issues that I've found:
gc-threadedinstead of justgc. I'm not sure how to change that. Moreover, the documentation should be updated unconditionally to require theboehm-gc-threadedpackage instead ofboehm-gc(regardless the runtime use of multithreading)java.net.preferIPv4Stackset totrueby default, and the IPv4-mapped IPv6 addresses are disabled by default. Many tests won't work unless thenet.inet6.ip6.v6onlysysctl is set to zero andjava.net.preferIPv4Stackis set tofalse