mold
mold copied to clipboard
rui314
CMake Port
For supporting Windows(#190), we definitely need something more portable than Makefiles. There are currently many Build-systems promising Cross-platform support and CMake is one of them. I would argue, that with mold's third-party modules TBB and mimalloc already using CMake, it is a natural choice. This PR ports mold to CMake. I have implemented every functionality(build, install, testing) from the current Makefiles.
This generally seems good. I agree with you that CMake is probably the build system we should migrate to, as we have already been using it for the third-party packages. I don't want to integrate this until mold 2.0 though, as our Makefile is pretty stable right now and we don't have a need to change it immediately.
Can you put the cd "$(dirname "$0")"/../.. changes in another commit? It floods the diff and makes it almost impossible to look at the actual changes.
- Breaking cross-platform conventions, despite quoting Windows support as the reason for this PR.
@friendlyanon The intention of this PR is not to support Windows. It is just one building block to get there. This PR is more of an initial port of the existing Makefiles.
Rebased onto the latest main. @rui314 Sorry I think I accidentally reverted your rebase.
The intention of this PR is not to support Windows. It is just one building block to get there.
Sure, but why do extra work now that later has to be undone anyway, especially now that you know how to do that.
The point of CMake is that you can write (mostly) declarative code that works on all platforms by default.
FYI @rui314
I have now also run the ./install-build-deps.sh script and all tests, running on the host machine, pass :smile:.
Running ctest -R .*_host$ yields: 100% tests passed, 0 tests failed out of 250.
Now the CMake build system is on feature parity with the Makefiles (I still have to figure out how to run the arch test though)
Also, CTest is even more capable than the current Make testing, parallel testing, test selection, etc. :wink:.
I made a few changes to the makefiles and test files. Can you rebase your patch?
@rui314 I am running a ubuntu docker container for testing, with the same installation as your github actions CI. But most of the arch tests still fail. Here is an example:
3: Test command: /opt/mold/test/elf/absolute-symbols.sh
3: Environment variables:
3: TEST_CC=i686-linux-gnu-gcc
3: TEST_CXX=i686-linux-gnu-g++
3: TEST_GCC=i686-linux-gnu-gcc
3: TEST_GXX=i686-linux-gnu-g++
3: OBJDUMP=i686-linux-gnu-objdump
3: MACHINE=i686
3: QEMU="qemu-i686 -L /usr/i686-linux-gnu"
3: Test timeout computed to be: 1500
3: Testing absolute-symbols ... <stdin>: In function 'handler':
3: <stdin>:9:46: error: 'REG_RIP' undeclared (first use in this function); did you mean 'REG_EIP'?
3: <stdin>:9:46: note: each undeclared identifier is reported only once for each function it appears in
1/1 Test #3: absolute-symbols_arch_i686 .......***Failed 0.01 sec
0% tests passed, 1 tests failed out of 1
I still don't understand this part error: 'REG_RIP' undeclared (first use in this function); did you mean 'REG_EIP'?
What are you doing to use the architecture specific includes?
Wow, setting the environment variables for the tests doesn't work... :sweat_smile:
Figuring out how to set the environment variables and where to put the quotes was a bit tricky, but with all that fixed, all of the tests pass :partying_face:
It looks like this commit is good enough to be included in the main branch, though I don't want to make a switch from Make to CMake immediately. So I'll put CMakeLists.txt under contrib. Any objections, anyone?
When you say "I don't want to make a switch from Make to CMake immediately" are you also considering users in this? As in, you don't want to make it appear as if CMake was the way to build the project? If you want to put the CML in a subdirectory, that will require adjusting paths (prepending ../). The Makefile and the CML can live next to each other in the interim and maybe there could be an AUTHOR_WARNING emitted in the CML exclaiming that it's not yet the primary way to build the project.
@friendlyanon
I meant I would merge this but wouldn't recommend users to use CMake. In order to not confuse users, I'll move CMakeLists.txt under contrib directory to hide it from users.
@clemenswasser Could you add a signed-off-by line to your commit messages as explained in https://github.com/rui314/mold/blob/main/CONTRIBUTING.md?
Such a little check for mingw:
- error:
-- Found OpenSSL: C:/msys64/mingw64/lib/libcrypto.dll.a (found version "1.1.1q") found components: Crypto
CMake Error at CMakeLists.txt:98 (file):
file STRINGS file "C:/msys64/usr/local/pkg_mold/src/mold-1.3.1/.git/HEAD"
cannot be read.
after commenting the line: #file(STRINGS .git/HEAD GIT_HASH)
- error:
-- Configuring done
CMake Error at CMakeLists.txt:60 (add_executable):
Cannot find source file:
third-party/rust-demangle/rust-demangle.c
Tried extensions .c .C .c++ .cc .cpp .cxx .cu .mpp .m .M .mm .ixx .cppm .h
.hh .h++ .hm .hpp .hxx .in .txx .f .F .for .f77 .f90 .f95 .f03 .hip .ispc
after modification:
macho/yaml.cc)
#third-party/rust-demangle/rust-demangle.c)
the target compilation starts as for the Makefiles version, and stop like https://github.com/rui314/mold/issues/190#issuecomment-1025452326
PS. Yes I know: "The intention of this PR is not to support Windows"
@3rav Your 1. error was actually a issue on my side, which I now fixed. Thanks, good catch! The 2. error however is really strange, did you accidentally deleted the third-party folder?
Using make, I can use the -C option to set the working directory.
~~No such option exists for cmake as far as I know.~~
https://github.com/rui314/mold/pull/571#issuecomment-1185305032
@3rav Your 1. error was actually a issue on my side, which I now fixed. Thanks, good catch! The 2. error however is really strange, did you accidentally deleted the third-party folder?
The 2. is my bad, I used 1.3.1 version, now in main version are OK
No such option exists for
cmakeas far as I know.
cmake --build requires a path argument: https://cmake.org/cmake/help/latest/manual/cmake.1.html#build-a-project
I rebased on current main and added the definition of MOLD_VERSION for elf/lto.cc, which it now uses.
I applied these patches locally but I cannot build mold due to the following error:
ruiu@blue:~/mold$ mkdir build
ruiu@blue:~/mold$ cd build
ruiu@blue:~/mold/build$ cmake ..
-- The C compiler identification is Clang 14.0.0
-- The CXX compiler identification is Clang 14.0.0
-- Detecting C compiler ABI info
-- Detecting C compiler ABI info - done
-- Check for working C compiler: /usr/lib/ccache/cc - skipped
-- Detecting C compile features
-- Detecting C compile features - done
-- Detecting CXX compiler ABI info
-- Detecting CXX compiler ABI info - done
-- Check for working CXX compiler: /usr/lib/ccache/c++ - skipped
-- Detecting CXX compile features
-- Detecting CXX compile features - done
CMake Error at /home/ruiu/mimalloc/cmake/mimalloc-config.cmake:1 (include):
include could not find requested file:
/home/ruiu/mimalloc/cmake/mimalloc.cmake
Call Stack (most recent call first):
CMakeLists.txt:21 (find_package)
CMake Error at CMakeLists.txt:22 (target_compile_definitions):
Cannot specify compile definitions for target "mimalloc" which is not built
by this project.
CMake Error at CMakeLists.txt:36 (find_package):
By not providing "FindTBB.cmake" in CMAKE_MODULE_PATH this project has
asked CMake to find a package configuration file provided by "TBB", but
CMake did not find one.
Could not find a package configuration file provided by "TBB" with any of
the following names:
TBBConfig.cmake
tbb-config.cmake
Add the installation prefix of "TBB" to CMAKE_PREFIX_PATH or set "TBB_DIR"
to a directory containing one of the above files. If "TBB" provides a
separate development package or SDK, be sure it has been installed.
-- Configuring incomplete, errors occurred!
See also "/home/ruiu/mold/build/CMakeFiles/CMakeOutput.log".
As discussed in this thread: https://github.com/rui314/mold/pull/571#discussion_r916267809, using the system TBB and mimalloc is now the default. You either have to install them with your system package manager or set -DMOLD_USE_SYSTEM_MIMALLOC=OFF -DMOLD_USE_SYSTEM_TBB=OFF to keep using the vendored dependencies.
You might want to improve the error message if TBB & mimalloc aren't found on the system
I'm against changing the default. At least it shouldn't be done by this commit.
It looks like ninja test runs all tests serially. Do you think you can run them in parallel?
the check target runs the tests in parallel by default and ensures that all targets get build first.
You can do cmake --build <buildDir> -t check.
When you execute ctest itself there it the typical -j <jobs> flag.
Did you know the difference between
ninja testandninja check?
The difference is that the test target is generated by CMake and only runs ctest on the build directory (no parallelism and doesn't build mold, which can result in strange file not found errors). The check target is a custom one by us, which builds and then runs the tests in parallel.
You might want to improve the error message if TBB & mimalloc aren't found on the system
Note that this error message is generated by CMake and not by us. In theory we could check the "*_FOUND" variables and output a different error message, but I have never seen this before...
@rui314 The below script successfully builds mold and its dependencies (with the above fix):
#!/bin/bash
set -euo pipefail
git clone [email protected]:microsoft/mimalloc.git
(
cd mimalloc
cmake -GNinja -DBUILD_SHARED_LIBS=1 -DCMAKE_BUILD_TYPE=Release -B build
cmake --build build
cmake --install build --prefix ../mimalloc-prefix
)
rm -rf mimalloc
git clone [email protected]:oneapi-src/oneTBB.git
(
cd oneTBB
cmake -GNinja -DBUILD_SHARED_LIBS=1 -DCMAKE_BUILD_TYPE=Release -B build -DTBB_TEST=0 -DTBB_STRICT=0 -DTBBMALLOC_BUILD=0 -DTBB_ENABLE_IPO=0
cmake --build build
cmake --install build --prefix ../oneTBB-prefix
)
rm -rf oneTBB
git clone [email protected]:clemenswasser/mold.git -b cmake
cmake -S mold -B mold/build -GNinja -DBUILD_SHARED_LIBS=1 -DCMAKE_BUILD_TYPE=Release \
"-DCMAKE_PREFIX_PATH=$PWD/oneTBB-prefix;$PWD/mimalloc-prefix" \
'-DCMAKE_CXX_FLAGS=-fno-exceptions -fno-unwind-tables -fno-asynchronous-unwind-tables'
cmake --build mold/build
So I'm not sure where your error comes from.
@rui314 and @friendlyanon since you are testing this on Ubuntu: The mimalloc packages on Ubuntu are only available since jammy (22.04 LTS) and don't even work:
- jammy: the cmake module files are not contained in either
libmimalloc2.0orlibmimalloc-dev - kinetic (currently in development): they fixed this by including them in
libmimalloc2.0, but now the include dir of the mimalloc cmake module is wrong and points to/usr/include/mimalloc-2.0
On Fedora 36 (which I am testing this on) their mimalloc package works correctly.
I built mimalloc and oneTBB from source as you can see, so I got the CMake package from them proper.
However, I have sent an email to @alexmyczko (the Debian page calls you Gürkan Myczko btw) about the CMake package being missing in libmimalloc-dev, so hopefully that can be fixed whenever. The fix in Debian will naturally reach downstream Ubuntu as well.
I just checked, for debian sid/unstable it is the same problem as with ubuntu kinetic/devel. They include the cmake files in the libmimalloc2.0 package but the include dir is incorrect as you can see from the build log:
CMake Error in CMakeLists.txt:
Imported target "mimalloc" includes non-existent path
"/usr/include/mimalloc-2.0"
in its INTERFACE_INCLUDE_DIRECTORIES. Possible reasons include:
* The path was deleted, renamed, or moved to another location.
* An install or uninstall procedure did not complete successfully.
* The installation package was faulty and references files it does not
provide.
Oh I see, there is something fishy going on with the Debian packages. libmimalloc2.0 contains the CMake package, but that is missing from libmimalloc-dev. I think this is an actual bug in the way mimalloc is packaged. There is no reason to provide the CMake package in the runtime component, since it's only ever used for developer purposes.
cc @alexmyczko Apologies for the ping again, just want to make sure you see this. Reach out to me via email if you want to talk about this.
@friendlyanon http://sid.ethz.ch/debian/mimalloc/2022b contains the CMake package in -dev instead of the library. can you confirm with 2.0.6+ds-2 if it's fixed?
if yes, i'll upload if no, might need to rename libmimalloc2.0 to libmimalloc2 (needs a round of NEW queue) something else: needs probably a patch
I checked the -dev package and what mimalloc itself builds, and I'm not sure how the build rules on salsa achieve what goes into the .deb.
Expected output from mimalloc itself with -DMI_BUILD_STATIC=0 -DMI_BUILD_OBJECT=0 -DMI_BUILD_TESTS=0 -DCMAKE_BUILD_TYPE=Release:
mimalloc-prefix/
├── include
│ └── mimalloc-2.0
│ ├── mimalloc.h
│ ├── mimalloc-new-delete.h
│ └── mimalloc-override.h
└── lib
├── cmake
│ └── mimalloc-2.0
│ ├── mimalloc.cmake
│ ├── mimalloc-config.cmake
│ ├── mimalloc-config-version.cmake
│ └── mimalloc-release.cmake
├── libmimalloc.so -> libmimalloc.so.2
├── libmimalloc.so.2 -> libmimalloc.so.2.0 # these ought to go in the runtime package
└── libmimalloc.so.2.0 # these ought to go in the runtime package
As you can see, the includes are in a subdirectory and the mimalloc.cmake propagates that same directory in its properties.
Output from your upload(s):
mimalloc-dev/usr/
├── include # where is mimalloc-2.0 ???
│ ├── mimalloc.h
│ ├── mimalloc-new-delete.h
│ └── mimalloc-override.h
├── lib
│ └── x86_64-linux-gnu
│ ├── cmake
│ │ └── mimalloc-2.0
│ │ ├── mimalloc.cmake
│ │ ├── mimalloc-config.cmake
│ │ ├── mimalloc-config-version.cmake
│ │ └── mimalloc-none.cmake
│ └── libmimalloc.so -> libmimalloc.so.2
As you can see, the subdirectory for the headers is not present and the mimalloc.cmake file still contains a path to that.
https://bugs.debian.org/1006249 for reference why the mimalloc subdir in include was removed (from debian/changelog)
please check same url, same version again, the missing part from you was added in that pkg, tell me if that's working for you
While the paths are correct now, your bugreport was requesting mimalloc headers to be directly in the system includes of compilers. mimalloc upstream build file allows this without patches, so their CML isn't entirely unsalvageable.
I recommend applying this patch and distributing like that:
--- debian/rules
+++ debian/rules
@@ -21,6 +21,9 @@
override_dh_auto_configure:
dh_auto_configure -- \
-DMI_BUILD_STATIC=OFF \
+ -DMI_BUILD_OBJECT=OFF \
+ -DMI_BUILD_TESTS=OFF \
+ -DMI_INSTALL_TOPLEVEL=ON \
-DMI_USE_LIBATOMIC=$(LIBATOMIC) \
override_dh_missing:
This also disables tests and an object library, so you don't unnecessarily build things that are just discarded anyway.
Worth noting that mimalloc says that MI_INSTALL_TOPLEVEL is deprecated, however that is the NORMAL way to do things. They don't even require this for any particular reason, since their includes aren't in a subdirectory (i.e. their usage is #include <mimalloc.h> not #include <mimalloc/mimalloc.h>, but then you wouldn't need an option for that either way).
Anyhow, a little offtopic, but the above allows all users to use mimalloc easily.
Also, I'm surprised dh_auto_configure goes against lintian rules and doesn't add -Wdate-time -D_FORTIFY_SOURCE=2 -fdebug-prefix-map=<binary-dir>=. -fstack-protector-strong to the build flags and -Wl,-Bsymbolic-functions -Wl,-z,relro -Wl,-z,now to the link flags.
it has been uploaded and built meanwhile https://buildd.debian.org/status/package.php?p=mimalloc&suite=sid