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

zlibstat built with DLL runtime in some configurations #860

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

Conversation

xecrets
Copy link

@xecrets xecrets commented Sep 26, 2023

This fixes zlibstat built with DLL runtime in some configurations #859

@AraHaan
Copy link
Contributor

AraHaan commented Sep 27, 2023

Oh sorry, did I overlook that?

@AraHaan
Copy link
Contributor

AraHaan commented Sep 27, 2023

Sorry, I am used to configs legit being named exactly the following: MT, MTd, MD, MDd so my bad.

@xecrets
Copy link
Author

xecrets commented Sep 27, 2023

Sorry, I am used to configs legit being named exactly the following: MT, MTd, MD, MDd so my bad.

It's been there a loooong time in successive versions of the visual studio solution files. I even think I might have tried to submit a pull request before, but hopefully it'll make it in there this time so I can ditch my special zlib build instructions to fix it before building https://github.com/xecrets/xecrets-file-classic .

@Neustradamus
Copy link

@madler: Can you look this PR?

@Neustradamus
Copy link

@madler: Can you look this PR?

@nmoinvaz
Copy link
Contributor

I think Visual studio project files should just be removed from the source tree. CMake can be used to generate them.

@Mr-Clam
Copy link

Mr-Clam commented Jan 23, 2024

I think Visual studio project files should just be removed from the source tree. CMake can be used to generate them.

I'd second that. This looks like tedious, manual, error prone maintenance work. I'm curious to know the reasons people prefer these projects over generating them via CMake? With CMake, you'll never need to create a new project file again when the next version of VS is released, and if you build on multiple platforms, you might be able to build the same way on all of them.

The other thing with this PR is that it only addresses the project file in the vc17 directory. There are five other VC version specific folders - don't all of these have the same problem? Surely they should all be fixed at the same time rather than just doing it for one? Do the CMake files also have this problem?

@madler
Copy link
Owner

madler commented Jan 24, 2024

I think Visual studio project files should just be removed from the source tree. CMake can be used to generate them.

I rather like this idea. Pinging others with apparent interest in Visual Studio for their comment.

@0ric1 , @arixmkii , @DanKonigsbach , @fredgan , @GerKev-SI , @irwir , @jeking3 , @KalleOlaviNiemitalo , @kiyolee , @kolomenkin , @lukasf , @Mr-Clam , @noahdav , @praiskup , @Sav0966 , @sergeycheban , @tbeu , @Tricky1975 , @yyc12345

@irwir
Copy link

irwir commented Jan 24, 2024

To build a library first build its builder, and this could be made recursive.
So far, I even do not have CMake tools installed.

Some software libraries support a simple and verified alternative.
.vcxproj file format remains stable for quite a while.
It is sufficient to maintain one version of the project (VS 2010 will do).
The only thing that might need changes would be the Platform Toolset, and Visual Studio helps with this on the first load of the project.

@arixmkii
Copy link

The only thing that might need changes would be the Platform Toolset, and Visual Studio helps with this on the first load of the project.

Single set of VS projects will not work for direct use in CI or straight with MSBuild, but it is probably not a big deal, one can prepare patch in advance (it will be needed anyway to adjust the compiler/linker settings in many usage scenarios)

From my perspective CMake is fine. With CMake it will be a bit more weird - first scaffold the Visual Studio files, then apply patches and then build the project. But whatever looks more relevant for the project and is easier to support.

@KalleOlaviNiemitalo
Copy link

Deleting the Visual Studio files is OK with me; at work, we don't currently need those. If the situation changes, then we can maintain them internally.

@kiyolee
Copy link

kiyolee commented Jan 24, 2024

Well, I have been maintaining a repo https://github.com/kiyolee/zlib-win-build offering solutions/projects for VS 2008|2010|2013|2015|2017|2019|2022 for quite some time now. I have no problem VS solutions/projects are removed from official zlib.
I do think CMake should be the main stream solution for most people. VS solutions/projects are relatively niche and have their own purposes.

@xecrets
Copy link
Author

xecrets commented Jan 24, 2024

Happy that there's some interest for this PR finally!

How about... Just take the PR as it is and then create a new issue about improving the build process and/or fixing the older versions of the .vcxproj files? It certainly won't make anything worse to correct some minor errors in the current version of the .vcxproj files that are there. It's a one-click operation... Deleting them will at least initially make things worse for those that are using them now. Like myself ;-) .

As for using CMake, the problem I have with that is that it introduces yet another dependency that needs to be downloaded, installed and kept updated. When building for Windows only, Visual Studio and MSBuild are the tools of choice and both need the .vcxproj files, and I personally think it's quite nice not to have to maintain yet another dependency in order to build zlib for just Windows.

If the CMake files that are already included actually support generating the equivalent solutions and project files that are in the contrib directory, perhaps the best way would be to include that in the creation of the distribution maybe via github actions so that the files are part of it, but built via CMake, thus voiding the need for the consumer to have the tool installed for building just the Windows version.

@kiyolee
Copy link

kiyolee commented Jan 24, 2024

Note that since VS2017, VS ships a usable copy of cmake and that may not be considered another dependency.

@xecrets
Copy link
Author

xecrets commented Jan 24, 2024

Note that since VS2017, VS ships a usable copy of cmake and that may not be considered another dependency.

Ok, fair enough, wasn't aware of that. Thanks for the info.

@irwir
Copy link

irwir commented Jan 25, 2024

Single set of VS projects will not work for direct use in CI or straight with MSBuild

Do library maintainers had to apply black magic if they are using CI and other automation?

@Mr-Clam
Copy link

Mr-Clam commented Jan 25, 2024

Single set of VS projects will not work for direct use in CI or straight with MSBuild

Do library maintainers had to apply black magic if they are using CI and other automation?

In our CI system, we build zlib using CMake and Ninja the same way on every platform from a single build script. We have to pass some different compiler defines for each platform, but that's fairly simple and localised. The biggest pain is Windows (of course) because we have to patch the CMakeLists.txt so it compiles the static lib with /Z7 to avoid warnings about PDBs when linking in to downstream products

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.

10 participants