Skip to content

Changes to build in case-insensitive filesystems, builds with Ubuntu.Dockerfile in OSX (arm64) and Ubuntu (amd64) #52

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

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

pentatonick
Copy link

Hi @mmbednarek,

Here are the changes to allow the project to build with Ubuntu.Dockerfile in both OSX (arm64), which by default has a case-insensitive file system while still building in Ubuntu (amd64).

The motivation of this change is to eliminate the clashes of header files on #include directives on case insensitive file systems. Example, on #include <cstring>, inside it will try to #include <string.h>, but instead of picking up the stand library's string.h, it will pick up minecpp/util/String.h. This happens because the system includes go last when compiling and because of the following CMake configuration in minecpp/library/util/CMakeLists.txt:

# Public include headers
target_include_directories(minecpp_utils
  PUBLIC ${CMAKE_CURRENT_SOURCE_DIR}/include
  PRIVATE ${CMAKE_CURRENT_SOURCE_DIR}/include/minecpp/util  # <-- this line makes String.h be picked up instead of standard library's string.h
)

This PR updates on meta/cmake-generate-target.sh and all the include directives needed for the project to fully build. Because there are many files, the most important change was to remove the following line on cmake-generate-target.sh:

    echo "target_include_directories($targetname"
    echo "  PUBLIC \${CMAKE_CURRENT_SOURCE_DIR}/include"
    echo "  PRIVATE \${CMAKE_CURRENT_SOURCE_DIR}/include/$include_path" # <-- removing this line
    echo ")"
    echo ""
    echo "# Source subdirectory"

Then run meta/cmake-generate.sh and update the header files accordingly for a successful build.

Now the project can be developed in OSX (arm64) through docker, but I'm still evaluating the effort to perform a native build.

Thanks once again for allowing me to learn with this code base.

@pentatonick
Copy link
Author

pentatonick commented Aug 7, 2024

The project builds successfully with the new structure, but running the schema compiler the command line of the makefile schema_compiler -i ./api/minecpp -s library/api/src -h library/api/include/minecpp -I minecpp reverts the changes I've done manually:

--- a/library/api/src/nbt/block/BlockState.schema.cpp
+++ b/library/api/src/nbt/block/BlockState.schema.cpp
@@ -1,4 +1,4 @@
-#include <minecpp/nbt/block/BlockState.schema.h>
+#include "nbt/block/BlockState.schema.h"

I will look into updating the schema compiler to fit this new structure such that this pull request can be considered complete.

@pentatonick
Copy link
Author

pentatonick commented Aug 8, 2024

The fix for the schema compiler was to ignore root_include_pathof IGenerator.h#L28 on NbtGenerator.cpp#L63 and on NetworkGenerator.cpp#L61, making both go from:

m_component.header_include_local(fmt::format("{}/{}", m_path_info.root_include_path, header));

to

m_component.header_include_local(header);

Had to update the command line arguments of the schema compiler from:

-i ./api/minecpp -s library/api/src -h library/api/include -I minecpp

to

-i ./api -s library/api/src -h library/api/include -I minecpp

removing minecpp from the suffix of the input-path argument.

Had to update library/api/Build.json#L3 from "include_path": "minecpp", to "include_path": "", (empty string). Because if not, running meta/cmake-generate.sh would result on:

Configuring library api
find: library/api/include/minecpp/minecpp: No such file or directory
find: library/api/include/minecpp/minecpp/nbt: No such file or directory
find: library/api/include/minecpp/minecpp/nbt/block: No such file or directory
find: library/api/include/minecpp/minecpp/nbt/chunk: No such file or directory
find: library/api/include/minecpp/minecpp/nbt/common: No such file or directory
find: library/api/include/minecpp/minecpp/nbt/item: No such file or directory
find: library/api/include/minecpp/minecpp/nbt/level: No such file or directory
find: library/api/include/minecpp/minecpp/nbt/player: No such file or directory
find: library/api/include/minecpp/minecpp/nbt/repository: No such file or directory
find: library/api/include/minecpp/minecpp/nbt/trace: No such file or directory
find: library/api/include/minecpp/minecpp/net: No such file or directory
find: library/api/include/minecpp/minecpp/net/engine: No such file or directory
find: library/api/include/minecpp/minecpp/net/login: No such file or directory
find: library/api/include/minecpp/minecpp/net/play: No such file or directory
find: library/api/include/minecpp/minecpp/net/status: No such file or directory
find: library/api/include/minecpp/minecpp/net/storage: No such file or directory
Configuring library chat
Configuring library crypto
Configuring library controller
(...)

while having it as an empty string results in no errors:

Configuring library api
Configuring library chat
Configuring library crypto
Configuring library controller
(...)

This allows the project to fully build without errors. I have not investigated a proper fix for this rather than just making it an empty string.

Supporting building on case-insensitive filesystems turned out to be more complex than expected. I wonder if there are other cases to be treated.

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.

1 participant