Skip to content

[mono] Use more idiomatic cmake style#82182

Merged
lambdageek merged 14 commits into
dotnet:mainfrom
lambdageek:idiomatic-cmake
Feb 21, 2023
Merged

[mono] Use more idiomatic cmake style#82182
lambdageek merged 14 commits into
dotnet:mainfrom
lambdageek:idiomatic-cmake

Conversation

@lambdageek
Copy link
Copy Markdown
Member

@lambdageek lambdageek commented Feb 15, 2023 •
edited
Loading

Make mono's build use a slightly more idiomatic cmake style:

  • One project() per subdirectory, rather than just in mini
  • toplevel add_subdirectory for each subdirectory
  • specify sources relative to the current project source dir, rather than using addprefix to relativize everything from mini
  • use object libraries for each logical grouping. Set compile definitions, link libraries, etc on the object libraries.
  • Have the final toplevel exes and shared libs (in mini; and components - when components are built to shared libs) pull in the object libraries. When building a static libmonosgen-2.0.a, pull in all the obejct libraries' objects to make a complete archive.

Doesn't untangle src/mono/mono/components from src/mono/moni/mini yet, although things could be grouped better there too.

@lambdageek
make eglib into a cmake object library
263fde2 
The goal is to use the cmake targets instead of _sources and _headers
variables as well as to use the cmake add_subdirectory commands to
bring in parts of the build
@lambdageek
make mono/utils into a cmake object library
f9fa99f
@lambdageek
make sgen into a cmake object library
74531c7
@lambdageek
make metadata into a cmake object library
389cd85
@lambdageek
set MONO_DLL_EXPORT when building the object libraries
04aed9f
@lambdageek
add eglib_api target
837b66a 
unlike eglib_objects, this one can be added by executables and static
or shared libraries without pulling in the eglib sources.  This is
what we want for dynamic runtime components, static interpreter and
icall table libs, etc
@ghost ghost added the area-Build-mono label Feb 15, 2023
@ghost ghost assigned lambdageek Feb 15, 2023
am11
am11 reviewed Feb 15, 2023
View reviewed changes
Comment thread src/mono/mono/eglib/CMakeLists.txt Outdated
@lambdageek
set project languages to C
ac0701e 
except for mini where we also enable CXX under some conditions (LLVM
or ICU, or browser wasm)
@lambdageek
fix darwin framework builds
8ee027c
@vargaz
Copy link
Copy Markdown
Contributor

vargaz commented Feb 15, 2023

The reason we were not doing this is that metadata/ etc. dirs are not buildable by themselves, they are just used to organize source files. Previously in mono/mono, there were actually libmetadata.a files somebody can link against to build monodis,
etc., but now we only have one libmonosgen.a file.

@lambdageek
Copy link
Copy Markdown
Member Author

lambdageek commented Feb 15, 2023

The reason we were not doing this is that metadata/ etc. dirs are not buildable by themselves, they are just used to organize source files. Previously in mono/mono, there were actually libmetadata.a files somebody can link against to build monodis, etc., but now we only have one libmonosgen.a file.

None of that changes. there's no libmetadata.a, etc. It's only changing how the build is organized.

@lambdageek lambdageek marked this pull request as ready for review February 15, 2023 20:43
@lambdageek
fix for older cmake
5af87e5 
can't have sources directly on add_library(target INTERFACE ...)
@lambdageek
Copy link
Copy Markdown
Member Author

lambdageek commented Feb 15, 2023 •
edited
Loading

This also seems to speed up the native runtime builds (by about 2%, but still...)

methodology (osx arm64, xcode 14.2, cmake 3.25.2, ninja 1.11.1):

killall dotnet
rm -rf artifacts
./build.sh --restore -s mono -c Debug
time ./build.sh --build -s mono -c Debug

baseline: 2:06.86 total
this PR: 2:03.99 total

@lambdageek
fix windows build
45969cd
@lambdageek
remove uneeded define
e0e3eea 
it's set later in the cmake file already
@build-analysis build-analysis Bot mentioned this pull request Feb 16, 2023
Closed
akoeplinger
akoeplinger approved these changes Feb 21, 2023
View reviewed changes
Copy link
Copy Markdown
Member

@akoeplinger akoeplinger left a comment

Choose a reason for hiding this comment

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

looks good to me

@lambdageek
Copy link
Copy Markdown
Member Author

lambdageek commented Feb 21, 2023

/azp run runtime-extra-platforms

@azure-pipelines
Copy link
Copy Markdown

azure-pipelines Bot commented Feb 21, 2023

Azure Pipelines successfully started running 1 pipeline(s).

@lambdageek
Copy link
Copy Markdown
Member Author

lambdageek commented Feb 21, 2023

runnning extra-platforms to check whether I broke the mono.mscordbi build

@lambdageek lambdageek mentioned this pull request Feb 21, 2023
Merged
@lambdageek
Copy link
Copy Markdown
Member Author

lambdageek commented Feb 21, 2023

mscordbi links ok. we're good

@lambdageek lambdageek merged commit f7d95da into dotnet:main Feb 21, 2023
@build-analysis build-analysis Bot mentioned this pull request Feb 22, 2023
Closed
@ghost ghost locked as resolved and limited conversation to collaborators Mar 24, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

@akoeplinger akoeplinger akoeplinger approved these changes

@vargaz vargaz Awaiting requested review from vargaz

@BrzVlad BrzVlad Awaiting requested review from BrzVlad BrzVlad is a code owner

@naricc naricc Awaiting requested review from naricc

@SamMonoRT SamMonoRT Awaiting requested review from SamMonoRT

@thaystg thaystg Awaiting requested review from thaystg thaystg is a code owner

@marek-safar marek-safar Awaiting requested review from marek-safar

+1 more reviewer

@am11 am11 am11 left review comments

Reviewers whose approvals may not affect merge requirements

Assignees

@lambdageek lambdageek

Labels

area-Build-mono

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@lambdageek @vargaz @akoeplinger @am11