small fixes to build CoreCLR on FreeBSD again #18072
Conversation
src/pal/src/CMakeLists.txt
Outdated
| if(CMAKE_SYSTEM_NAME STREQUAL FreeBSD) | ||
| # On FreeBSD , we use the libunwind that's part of the OS | ||
| set(CLR_CMAKE_USE_SYSTEM_LIBUNWIND 1) | ||
| endif(CMAKE_SYSTEM_NAME STREQUAL FreeBSD) |
There was a problem hiding this comment.
Since this is identical to the Darwin block above, could these be combined with an OR condition on the name?
There was a problem hiding this comment.
I was thinking about it @stephentoub but I felt this may be easier to read.
One way would be to change it to if(NOT CMAKE_SYSTEM_NAME STREQUAL Linux)
e.g. use our copy only for Linux and use system version for OSX and all BSD variants.
There was a problem hiding this comment.
OR is used a bunch of times elsewhere in the file; same for AND. If if(NOT CMAKE_SYSTEM_NAME STREQUAL Linux) makes more sense logically, then I'm fine with it. I just don't want us having unnecessary duplication.
There was a problem hiding this comment.
I decided to go with OR. This can be revisited later if we ever have more non-linux platforms.
src/pal/src/configure.cmake
Outdated
| @@ -1288,6 +1288,7 @@ elseif(CMAKE_SYSTEM_NAME STREQUAL FreeBSD) | |||
| set(HAS_FTRUNCATE_LENGTH_ISSUE 0) | |||
| set(BSD_REGS_STYLE "((reg).r_##rr)") | |||
| set(HAVE_SCHED_OTHER_ASSIGNABLE 1) | |||
| link_directories("/usr/local/lib") | |||
There was a problem hiding this comment.
This doesn't belong here. This script is used for detecting the configuration of the local system only and should not contain settings for the build of the product. It would need to be located in the root CMakeLists.txt instead.
But I wonder how you have hit the issue. When I've tried to build coreclr on FreeBSD 11 now, it doesn't try to build the lttng support at all, but the build succeeds.
There was a problem hiding this comment.
It seems like others run to similar issue.
https://forums.freebsd.org/threads/how-not-to-build-net-coreclr-on-freebsd.64881/
Unfortunately I don't have pristine base image any more.
I have lttng package installed. I'm wondering if there is detection and we skip tracing in some cases. Without tracing, cli was crashing when it attempted to trace so getting lltng in would be beneficial. Maybe build should fail of lttng is not found? We list it as pre-requisite on (old) FreeBSD building page.
There was a problem hiding this comment.
Ok, I guess I've just not installed lttng and that's why it worked for me. The build is optional because the thinking was that it is valuable to be able to build without tracing support on platforms where it was not present.
src/pal/src/CMakeLists.txt
Outdated
| @@ -275,6 +275,8 @@ if(CMAKE_SYSTEM_NAME STREQUAL Darwin) | |||
| endif(CMAKE_SYSTEM_NAME STREQUAL Darwin) | |||
|
|
|||
| if(CMAKE_SYSTEM_NAME STREQUAL FreeBSD) | |||
| # some dependencies from port collection are installed under /usr/local | |||
| link_directories("/usr/local/lib") | |||
There was a problem hiding this comment.
I was thinking about it a bit and I would actually prefer a different solution (I've verified that it works). Instead of adding the /usr/local/lib to the library directories, could you please modify the src/scripts/genLttngProvider.py as follows? We should be using the find_library for all non-system libraries.
diff --git a/src/scripts/genLttngProvider.py b/src/scripts/genLttngProvider.py
index de537d0f1a..296042be5b 100644
--- a/src/scripts/genLttngProvider.py
+++ b/src/scripts/genLttngProvider.py
@@ -555,8 +555,10 @@ def generateLttngFiles(etwmanifest,eventprovider_directory):
tracepointprovider_Cmake.write(""" )
+ find_library(LTTNG NAMES lttng-ust)
+
target_link_libraries(coreclrtraceptprovider
- -llttng-ust
+ ${LTTNG}
)
# Install the static coreclrtraceptprovider library
There was a problem hiding this comment.
I updated code as you suggested @janvorli (and verified it works for me)
It is interesting that we already add /usr/local/include to include path but not lib/lib64 for libraries.
src/scripts/genLttngProvider.py
Outdated
| target_link_libraries(coreclrtraceptprovider | ||
| -llttng-ust | ||
| ${LTTNG} |
There was a problem hiding this comment.
A micro-nit - there is one space missing in front of the ${LTTNG} so that it is aligned with the coreclrtraceptprovider string above.
Use system libunwind - as we do on OSX. The version we checked in does not build on FreeBSD.
Add /usr/local/lib - standard location where FreebSD puts libraries from additional packages - lttnng in this case.
cc: @mateusrodrigues