Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Adding FreeBSD build support. #460

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

LadySerenaKitty
Copy link

Making all changes required to build on FreeBSD. Some CMake entries for Linux may have changed, but their functionality remains unchanged.

BuildTools/FormatValidation/validate_format_freebsd.sh Outdated Show resolved Hide resolved
BuildTools/CMake/BuildUtils.cmake Outdated Show resolved Hide resolved
CMakeLists.txt Outdated Show resolved Hide resolved
Platforms/Linux/src/LinuxPlatformMisc.cpp Show resolved Hide resolved
@@ -342,7 +342,9 @@
#define __stdcall
#define __vectorcall
#define __thiscall
#ifndef PLATFORM_FREEBSD
#define __fastcall
Copy link
Contributor

Choose a reason for hiding this comment

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

Is __fastcall a recognized keyword when compiling on FreeBSD?

Copy link
Author

Choose a reason for hiding this comment

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

On this one, the compiler was complaining about fastcall being redefined, as FreeBSD's headers already define fastcall.

Copy link
Contributor

Choose a reason for hiding this comment

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

Which compiler are you using?

Copy link
Author

Choose a reason for hiding this comment

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

Believe it or not, the entirety of DiligentEngine was built using the compiler provided by the base system: clang 16.0.6.
image

Copy link
Contributor

Choose a reason for hiding this comment

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

It is actually strange why it only complains about __fastcall. It also recognized all other attributes
https://clang.llvm.org/docs/AttributeReference.html#target-version

Can you post the error that the compiler produces?

Copy link
Author

Choose a reason for hiding this comment

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

This is what happens without skipping __fastcall:

[ 35%] Building CXX object DiligentCore/Graphics/ShaderTools/CMakeFiles/Diligent-ShaderTools.dir/src/DXCompiler.cpp.o
In file included from /usr/home/jlhawkwell/Projects/DiligentEngine/DiligentCore/Graphics/ShaderTools/src/DXCompiler.cpp:42:
In file included from /usr/home/jlhawkwell/Projects/DiligentEngine/DiligentCore/Graphics/ShaderTools/include/DXCompilerBaseLinux.hpp:31:
In file included from /usr/home/jlhawkwell/Projects/DiligentEngine/DiligentCore/Graphics/ShaderTools/../../ThirdParty/DirectXShaderCompiler/dxc/dxcapi.h:37:
/usr/home/jlhawkwell/Projects/DiligentEngine/DiligentCore/Graphics/ShaderTools/../../ThirdParty/DirectXShaderCompiler/dxc/WinAdapter.h:345:9: error: '__fastcall' macro redefined [-Werror,-Wmacro-redefined]
#define __fastcall
        ^
/usr/include/sys/cdefs.h:365:9: note: previous definition is here
#define __fastcall      __attribute__((__fastcall__))
        ^
1 error generated.
gmake[2]: *** [DiligentCore/Graphics/ShaderTools/CMakeFiles/Diligent-ShaderTools.dir/build.make:122: DiligentCore/Graphics/ShaderTools/CMakeFiles/Diligent-ShaderTools.dir/src/DXCompiler.cpp.o] Error 1
gmake[1]: *** [CMakeFiles/Makefile2:3521: DiligentCore/Graphics/ShaderTools/CMakeFiles/Diligent-ShaderTools.dir/all] Error 2
gmake: *** [Makefile:156: all] Error 2

Copy link
Contributor

Choose a reason for hiding this comment

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

I updated DXC headers, so this should work now - please try the latest version

Copy link
Contributor

Choose a reason for hiding this comment

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

A better way to fix this is to check

#ifndef __fastcall

Though I do really not understand why it only complains about __fastcall. All other macros are defined in the same way.

Copy link
Author

Choose a reason for hiding this comment

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

The __fastcall macro is already defined in /usr/include/sys/cdefs.h on line 365.

Copy link
Contributor

Choose a reason for hiding this comment

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

What I suggested is to use the #ifndef __fastcall guard instead of #ifndef PLATFORM_FREEBSD.
Also note that this file is a third-party header so with the next update, it may break again.

CMakeLists.txt Show resolved Hide resolved
BuildTools/CMakeLists.txt Outdated Show resolved Hide resolved
CMakeLists.txt Show resolved Hide resolved
CMakeLists.txt Outdated Show resolved Hide resolved
Graphics/Archiver/CMakeLists.txt Outdated Show resolved Hide resolved
@@ -342,7 +342,9 @@
#define __stdcall
#define __vectorcall
#define __thiscall
#ifndef PLATFORM_FREEBSD
#define __fastcall
Copy link
Contributor

Choose a reason for hiding this comment

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

Which compiler are you using?

Copy link
Contributor

@TheMostDiligent TheMostDiligent left a comment

Choose a reason for hiding this comment

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

I updated core module, so that format validation will be disabled on platforms that don't have the script.
Can you update to the latest version and try it?

@LadySerenaKitty
Copy link
Author

I updated core module, so that format validation will be disabled on platforms that don't have the script. Can you update to the latest version and try it?

Format validation is successfully skipped (tested after deleting the scripts I added). Would you like a new push with the scripts removed?

@TheMostDiligent
Copy link
Contributor

Would you like a new push with the scripts removed?

You can force-push to the same branch - the PR will be updated.

Graphics/Archiver/CMakeLists.txt Outdated Show resolved Hide resolved
Graphics/GraphicsEngineOpenGL/CMakeLists.txt Outdated Show resolved Hide resolved
@@ -342,7 +342,9 @@
#define __stdcall
#define __vectorcall
#define __thiscall
#ifndef PLATFORM_FREEBSD
#define __fastcall
Copy link
Contributor

Choose a reason for hiding this comment

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

I updated DXC headers, so this should work now - please try the latest version

CMakeLists.txt Outdated
@@ -513,6 +522,9 @@ else()
set(DILIGENT_INSTALL_PDB OFF)
endif()

if(PLATFORM_FREEBSD AND EXISTS /usr/local/include)
target_include_directories(Diligent-BuildSettings SYSTEM AFTER INTERFACE /usr/local/include)
Copy link
Contributor

Choose a reason for hiding this comment

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

What errors do you get without this include?
This change does not seem right

Copy link
Author

Choose a reason for hiding this comment

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

It is quite correct. Without this, anything that isn't part of base will not be found.

Copy link
Contributor

Choose a reason for hiding this comment

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

What base?

Copy link
Author

Choose a reason for hiding this comment

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

FreeBSD! "base" is short for "base operating system" and is the stuff that's made by the FreeBSD Project - kernel and world. This includes everything outside of /usr/home and /usr/local, which isn't much.

A mirror of the source repo is available here, so you can see what all is part of FreeBSD's base: https://github.com/freebsd/freebsd-src

@@ -520,6 +528,10 @@ else()
set(DILIGENT_INSTALL_PDB OFF)
endif()

if(PLATFORM_FREEBSD AND EXISTS /usr/local/include)
target_include_directories(Diligent-BuildSettings SYSTEM AFTER INTERFACE /usr/local/include)
Copy link
Contributor

Choose a reason for hiding this comment

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

I tried to find some info about this problem, and I think that the right way to fix this is to set CMAKE_REQUIRED_INCLUDES at your project's CMake level:

dotnet/runtime#4073

Copy link
Contributor

Choose a reason for hiding this comment

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

Did you try setting the CMAKE_REQUIRED_INCLUDES in your project CMake?

Copy link
Contributor

@TheMostDiligent TheMostDiligent left a comment

Choose a reason for hiding this comment

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

Sorry for a long delay with response.

Platforms/Linux/src/LinuxPlatformMisc.cpp Show resolved Hide resolved
@@ -342,7 +342,9 @@
#define __stdcall
#define __vectorcall
#define __thiscall
#ifndef PLATFORM_FREEBSD
#define __fastcall
Copy link
Contributor

Choose a reason for hiding this comment

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

A better way to fix this is to check

#ifndef __fastcall

Though I do really not understand why it only complains about __fastcall. All other macros are defined in the same way.

@@ -342,7 +342,9 @@
#define __stdcall
#define __vectorcall
#define __thiscall
#ifndef PLATFORM_FREEBSD
#define __fastcall
Copy link
Contributor

Choose a reason for hiding this comment

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

What I suggested is to use the #ifndef __fastcall guard instead of #ifndef PLATFORM_FREEBSD.
Also note that this file is a third-party header so with the next update, it may break again.

@@ -520,6 +528,10 @@ else()
set(DILIGENT_INSTALL_PDB OFF)
endif()

if(PLATFORM_FREEBSD AND EXISTS /usr/local/include)
target_include_directories(Diligent-BuildSettings SYSTEM AFTER INTERFACE /usr/local/include)
Copy link
Contributor

Choose a reason for hiding this comment

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

Did you try setting the CMAKE_REQUIRED_INCLUDES in your project CMake?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants