Skip to content
This repository has been archived by the owner on Jan 23, 2023. It is now read-only.

small fixes to build CoreCLR on FreeBSD again #18072

Merged
merged 5 commits into from
May 22, 2018
Merged

small fixes to build CoreCLR on FreeBSD again #18072

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
96417ea
small fixes to build CoreCLR on FreeBSD again
May 21, 2018
40467af
feedback from review
May 21, 2018
4e59667
update genLttngProvider.py as jan suggested
May 21, 2018
407729f
remove extra comment
May 21, 2018
3dba951
add missing extra space
May 21, 2018
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions src/pal/src/CMakeLists.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,11 @@ if(CMAKE_SYSTEM_NAME STREQUAL Darwin)
set(CLR_CMAKE_USE_SYSTEM_LIBUNWIND 1)
endif(CMAKE_SYSTEM_NAME STREQUAL Darwin)

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)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since this is identical to the Darwin block above, could these be combined with an OR condition on the name?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

+1

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I decided to go with OR. This can be revisited later if we ever have more non-linux platforms.


include_directories(SYSTEM /usr/local/include)

add_compile_options(-fPIC)
Expand Down
1 change: 1 addition & 0 deletions src/pal/src/configure.cmake
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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")
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

elseif(CMAKE_SYSTEM_NAME STREQUAL NetBSD)
set(DEADLOCK_WHEN_THREAD_IS_SUSPENDED_WHILE_BLOCKED_ON_MUTEX 0)
set(PAL_PTRACE "ptrace((cmd), (pid), (void*)(addr), (data))")
Expand Down