Skip to content

Conversation

jasonpugsley
Copy link
Contributor

Add the necessary CMake settings to detect the CPU and
reference the correct libraries when the build host is FreeBSD.

These changes allow me to build the mono subset on FreeBSD 12.2
There shouldn't be any impact when building on other hosts. I also did a cross-build
of FreeBSD on Linux and that completed successfully (it already builds correctly there).

#14537 (comment)

@ghost ghost added community-contribution Indicates that the PR has been added by a community member area-Build-mono labels Aug 7, 2021
@Thefrank
Copy link
Contributor

Thefrank commented Aug 7, 2021

set(HAVE_ALLOCA_H 0)
Does it think we have this when we don't?

@jasonpugsley
Copy link
Contributor Author

jasonpugsley commented Aug 8, 2021

set(HAVE_ALLOCA_H 0)
Does it think we have this when we don't?

I get a build error without the explicit setting:

  mono/mini/../../mono/eglib/eglib-config.h:36:6: error: invalid token at start of a preprocessor expression
  #if  == 1
       ^

From here artifacts/obj/mono/FreeBSD.x64.Debug/mono/eglib/eglib-config.h

#if  == 1
#define G_HAVE_ALLOCA_H
#endif

@Thefrank
Copy link
Contributor

Thefrank commented Aug 8, 2021

ah!
There is a note in src/mono/cmake/configure.cmake about detection not working :(

Speaking of... would it be better/cleaner to move:

  set(HAVE_SYS_ICU 1)
  set(HAVE_ALLOCA_H 0)
  set(HAVE_SYS_SYSCTL_H 1)
  set(HAVE_SYS_USER_H 1)

into its own if(HOST_FREEBSD) block at the end in src/mono/cmake/configure.cmake? The fix for Solaris that was recently committed put those set fixes in that spot.

the add_link_options(-lpthread -lm) would go into src/mono/mono/mini/CMakeLists.txt as

elseif(HOST_FREEBSD)
  set(OS_LIBS pthread m ${CMAKE_DL_LIBS})

after the SOLARIS elseif if following the same pattern. (might not need the CMAKE_DL_LIBS)

--subset mono builds either way.

@jasonpugsley
Copy link
Contributor Author

jasonpugsley commented Aug 8, 2021

OK this needs a little more discussion. It turns out all but one use of HAVE_ALLOCA_H are #ifdefs. So setting it to be 0 doesn't help there - any value satisfies an #ifdef. The only other use case is to set up G_HAVE_ALLOCA_H for just a single reference in src/mono/mono/eglib/glib.h
Ideally we would modify glib.h to change G_HAVE_ALLOCA_H to HAVE_ALLOCA_H.
It is currently

#ifdef G_HAVE_ALLOCA_H
#include <alloca.h>
#endif

If that's not desirable, then can we change src/mono/cmake/eglib-config.h.cmake.in
So from

#if @HAVE_ALLOCA_H@ == 1
#define G_HAVE_ALLOCA_H
#endif

to

#ifdef HAVE_ALLOCA_H
#define G_HAVE_ALLOCA_H
#endif

Does #cmakedefine in the .in file skip defining the var if it is set to 0 in the cmake file?

@jasonpugsley
Copy link
Contributor Author

Following up on my last question, if the variable is set to 0 in the cmake file set(HAVE_ALLOCA_H 0) then it is not defined in the .h.in -> .h generated file. But its value can be referenced which is why the %HAVE_ALLOCA_H% == 1 requires it to be set, but fails to work if it's not defined.
I guess FreeBSD must be the only host platform that doesn't have alloca.h
On a side note, when cross compiling on Linux, the alloca.h file is found. This would mean the host root files are also searched instead of only the cross root files.

@Thefrank
Copy link
Contributor

The way that #cmakedefine works is the variable will either be set or an empty string :(

This is why FreeBSD gets that strange #if == 1 error
Something like #cmakedefine01 found in newer versions of CMake still ends with an empty string during my testing
(https://cmake.org/cmake/help/latest/command/configure_file.html)

@jasonpugsley
Copy link
Contributor Author

In my testing I found this works fine on Linux (native), on FreeBSD (native) and on Linux (cross-building FreeBSD) for src/mono/cmake/eglib-config.h.cmake.in

#ifdef HAVE_ALLOCA_H
#define G_HAVE_ALLOCA_H
#endif

@Thefrank
Copy link
Contributor

At this point ill take what I can get. The CI system will let us know if anything explodes [it shouldn't].

@jasonpugsley
Copy link
Contributor Author

I have refined the changes to be more complete. I can remove the changes to find-native-compiler.sh and init-compiler.sh if they shouldn't be done here - I have been using clang-12 on Linux and FreeBSD without issue.

I have made the change to src/mono/cmake/eglib-config.h.cmake.in as per the previous comments but please review this for consistency.

@jasonpugsley jasonpugsley requested a review from vargaz August 16, 2021 06:05
@jasonpugsley
Copy link
Contributor Author

The cross-build in CI failed because the container is missing or can't find lld. We'll need to update some of the dependencies.

Copy link
Member

@josteink josteink left a comment

Choose a reason for hiding this comment

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

LGTM. Hope someone can get that container fixed.

@jasonpugsley
Copy link
Contributor Author

This container
ubuntu-18.04-cross-freebsd-12-20210813173449-f13d79e
has an updated crossfootfs. It's FreeBSD12.1
I've updated eng/pipelines/common/platform-matrix.yml
Hopefully that works.

@Thefrank
Copy link
Contributor

It does appear that the freebsd12 container is still on 12.1.
It looks like the changes from dotnet/arcade#7588 haven't made their way into the containers yet.

@jasonpugsley
Copy link
Contributor Author

jasonpugsley commented Aug 16, 2021

The 3 FreeBSD cross-builds in the checks succeeded but FreeBSD/mono isn't actually included in runtime.yml yet.

That can go in a separate change set as it affects all build-runs in the CI system and there might be additional devops decisions around it.

Copy link
Member

@wfurt wfurt left a comment

Choose a reason for hiding this comment

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

LGTM

Cross-build on Linux for FreeBSD x64 now uses LLVM provided lld
linker to create valid FreeBSD binaries.

find-native-compiler.sh will now look for clang versions 10, 11
and 12. init-compiler.sh will also look for version 12.

The cmake test for HAVE_ALLOC_H in eglib-config.h.cmake.in
would fail if it wasn't defined. The fix was to convert to
an normal C preprocessor #ifdef

Update the docker container to use the most recent version of
ubuntu-18.04-cross-freebsd-12
Adjusted CMake settings to search the crossrootfs path
for libinotify.so
@jasonpugsley
Copy link
Contributor Author

I noticed libinotify wasn't being found on the cross builds so I've fixed that now.

@Thefrank
Copy link
Contributor

If/when this gets merged, it will need a backport to 6.0 as it looks like main is for 7.0 right now

@wfurt wfurt merged commit 3e6d492 into dotnet:main Aug 27, 2021
@wfurt
Copy link
Member

wfurt commented Aug 27, 2021

any thoughts on 6.0 @danmoseley? Risk for other supported platforms should be really small IMHO and it would allow community to but 6.0 on their own.

@danmoseley
Copy link
Member

@jkotas you are more involved with efforts to bring up/maintain other platforms. does this seem a reasonable change to take at the moment? I see no problem if there is negligible risk to anything else.

@jasonpugsley jasonpugsley deleted the build-freebsd-mono branch August 28, 2021 03:38
@jkotas
Copy link
Member

jkotas commented Aug 30, 2021

This is in the same category as #58085 (comment) . There are likely more changes needed to make FreeBSD work well. It is unlikely we will be able to port them all to the release branch.

After thinking about it a bit, I would recommend to keep the policy simple and only backport changes for officially supported OSes and architectures to the release branches. If somebody wants to build OSes that are not officially supported by the .NET team, they can do it either from main or release branch w/ local patch. Tizen OS has been on this plan too.

@danmoseley
Copy link
Member

I assume that includes changes to RIDs?

@Thefrank
Copy link
Contributor

Where does that leave FreeBSD (and also illumos) for net6?

Currently under net5: runtime crossbuilds Linux->FreeBSD with no patches. Both aspnetcore and installer need a freebsd-x64 RID manually added in via sed/patch/diff but will also build without patches.

I was recently able to make a usable net6-preview7 but it required 3 freebsd-based patches, or 5 total patches as two of the three freebsd ones require earlier illumos-based patches (cherry-pick did not work otherwise). While this is annoying, having the 3(5?) patches backported into net6 will make it much easier for the freebsd community to build/help support net6 during its lifetime. I understand if a backport will not be suitable here, but I can list all of the needed patches if it will help.

Finally, I would like to thank the dotnet team for allowing unofficial platforms/arches to be added into dotnet :)

@ghost ghost locked as resolved and limited conversation to collaborators Sep 29, 2021
@wfurt wfurt added the os-freebsd FreeBSD OS label Sep 30, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
area-Build-mono community-contribution Indicates that the PR has been added by a community member os-freebsd FreeBSD OS
Projects
None yet
Development

Successfully merging this pull request may close these issues.

8 participants