Skip to content

Conversation

@chrchr-github
Copy link
Collaborator

No description provided.

@firewave
Copy link
Collaborator

firewave commented Jan 4, 2022

We really need to get the actual msbuild back into the regular CI. It was removed in 8dc1fa7

@danmar danmar merged commit 184ef4e into danmar:main Jan 4, 2022
@pfultz2
Copy link
Contributor

pfultz2 commented Jan 4, 2022

We really need to get the actual msbuild back into the regular CI.

We need to either update dmake or add another script to generate the vs studio project files, if we want to add these builds to the CI. Otherwise, it makes it difficult for contributors to add new source files as they need to figure out all the different build systems, and msbuild is not easily available(it only runs on one OS) so contributors cannot test or debug their changes.

It was removed in 8dc1fa7

Yea it was because right now we only have cmake to generate these project files. It seems like it isn't used anymore. In my opinion, I think it would be better to remove it and just use cmake to create the project files(they will also be more complete and include running all of our tests).

@danmar
Copy link
Owner

danmar commented Jan 4, 2022

I think it would be better to remove it and just use cmake to create the project files(they will also be more complete and include running all of our tests).

To me it's ok to remove the visual studio project files. I believe we kept those before because there was some optimisation option that was not set by cmake.

I am even starting to feel that we can remove dmake.

@pfultz2
Copy link
Contributor

pfultz2 commented Jan 4, 2022

I am even starting to feel that we can remove dmake.

But how would we generate the Makefile? The Makefile seems a little more complicated as we need to add the header dependencies. Are you suggesting we simplify the Makefile to automatically glob the files and the header dependencies? Or should we drop the Makefile and just use cmake instead since it can generate Makefiles?

@danmar
Copy link
Owner

danmar commented Jan 4, 2022

Or should we drop the Makefile and just use cmake instead since it can generate Makefiles?

Yes I think we can use cmake to generate Makefile. I don't have a strong opinion about dropping dmake but I am starting to accept the idea.

@firewave
Copy link
Collaborator

firewave commented Jan 4, 2022

We need to either update dmake or add another script to generate the vs studio project files, if we want to add these builds to the CI. Otherwise, it makes it difficult for contributors to add new source files as they need to figure out all the different build systems, and msbuild is not easily available(it only runs on one OS) so contributors cannot test or debug their changes.

Even if it is done via CMake only people might need to do MSVC specific things in it which they do cannot test. That's why we need proper coverage in the CI. But that is also why we do PRs.

Yea it was because right now we only have cmake to generate these project files. It seems like it isn't used anymore. In my opinion, I think it would be better to remove it and just use cmake to create the project files(they will also be more complete and include running all of our tests).

It was stated that the structure of those files is horrible - which I agree with. But given the sheer amount of different approaches in total and even the multiple ways to build using MSVC it's definitely the one thing we should get rid of since it is the most inconvenient way as well as additional one work.

To me it's ok to remove the visual studio project files. I believe we kept those before because there was some optimisation option that was not set by cmake.

CMake is not aligned with all the Visual Studio stuff yet. There still too much other things coming up which distract as well as my condition which seriously limits the time I can spent on this (to be honest I should not be involved with anything at all).

I am even starting to feel that we can remove dmake.

Yes I think we can use cmake to generate Makefile. I don't have a strong opinion about dropping dmake but I am starting to accept the idea.

I do see the advantage of having just support for make. But CMake is the most complete of the build systems but it still lacks things from all the other ones. (Hopefully nobody comes along and suggests/submits to just do it properly from scratch with something more modern/better like meson 😱)
That's why I started #3596 so we can decide on what the actually supported moving parts are and what we can/want to remove so it can be all be added to the CI and properly consolidated. But we have not even put all the things from Travis or Appveyor into the GitHub actions yet. And there's a few more pressing things we need to address in the CI first (build time, tooling coverage, tests, merging in changes from obsolete CI, etc. - I will try to file tickets for it).

I will try to integrate this into the other PR as well as file and reference tickets for the various tasks. Please give me some time for this. This has been coming a long time and it's good it is discussed and we seem to be making decisions so we can make progress. So I don't think we have to rush this but it would be great if have this sorted out until the end of the year 😀🤞🧹

BTW it seems GitHub allows you to have actual discussions in your repo but that seems to be disabled at the moment (there should be a tab). I would like to see this enabled this there's a few things beside which should be discussed and/or agreed on by the whole "team" and that seems like a more appropriate place than the bug tracker or a PR.

@chrchr-github
Copy link
Collaborator Author

For reference, the difference between CMake-generated and native project files: #3500 (comment)

@pfultz2
Copy link
Contributor

pfultz2 commented Jan 5, 2022

That's why we need proper coverage in the CI.

We shouldn't block PRs for tools that are not easily available. For different toolchains we could provide dockerfiles for the dev environment. GH actions can use dockerfiles(even with docker-layer caching) for CI. This makes it easier for a contributor to get the environment for a specific toolchain. However, this still wont help with MSVC.

I do see the advantage of having just support for make

Yea, I dont see a need to have all these build systems. It makes testing harder for us as we need to test more combinations. Furthermore, without some scripts to autogenerate these build files, contributors need to navigate all these build systems as well.

But CMake is the most complete of the build systems

CMake does have a lot of advantages: it can generate files for many different buildsystems and IDEs, has a simple way to tell where dependencies have been installed(meson does not have this), and can run the tests much faster.

@eli-schwartz
Copy link

has a simple way to tell where dependencies have been installed(meson does not have this),

I think that generally has a lot more to do with what kind of dependency you're looking for. e.g. if you're looking for pkg-config dependencies, you must set the PKG_CONFIG_PATH variable, both in meson and in cmake

CMake config dependencies searched for via cmake find_package() support various arcane path lookups in order to find that *-config.cmake file, and those lookup methods include command line methods to tell where the dependency was installed (I hesitate to call this "simple") which are always respected whenever looking up cmake config dependencies, because only cmake understands cmake config dependencies.

But, those methods are all respected in meson too (using meson's "run cmake and see if cmake can find the dependency" mode, which is the only way to parse a *-config.cmake at all).

Moreover, meson does something cmake doesn't: it lets you look up dependency('foo') and look up "foo" with both pkg-config and cmake foo-config.cmake (and meson's collection of builtin override lookups, and framework dependencies on macOS, and external projects included as a submodule). All these are just considered dependency satisfiers, you don't need to use a particular dependency satisfier by explicitly using either find_package(foobar), pkg_check_modules(FOOBAR foobar), or add_subdirectory(thirdparty/foobar).

Finding dependencies using raw library searches in meson supports the compiler builtin method: https://mesonbuild.com/Reference-manual_returned_compiler.html#returned-by

In cmake there are 4 different listed methods (and another 3 you don't set as a user), either set one of 3 different CMAKE_* variables in one of 3 different locations (env, cmakecache, toolchain), or for option 5 use the compiler builtin method of setting the LIB variable just like in meson".

...

I would say the bigger advantage of CMake is that a CMakeLists.txt already exists.

@pfultz2
Copy link
Contributor

pfultz2 commented Jan 5, 2022

e.g. if you're looking for pkg-config dependencies, you must set the PKG_CONFIG_PATH variable, both in meson and in cmake

No you dont. In cmake, you only need to set CMAKE_PREFIX_PATH as pkg_check_modules will use CMAKE_PREFIX_PATH to set PKG_CONFIG_PATH. All cmake's find_* commands use CMAKE_PREFIX_PATH to find dependencies as well. This makes it simple for users to tell cmake where it installed dependencies without needing to learn each package-specific custom variable. It also makes it much simpler for source-based package managers.

In meson, if all dependencies are not found using pkg-config, then you need to lookup meson's custom variables for that dependency such as BOOST_ROOT, CMAKE_PREFIX_PATH(which is only specific to cmake dependencies) or figure out the compiler-specific custom variables.

@chrchr-github chrchr-github deleted the chr_FixMSVC branch April 10, 2022 21:15
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.

5 participants