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

Fix Visual Studio 2022 projects (create zconf.h) #1039

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

Conversation

irwir
Copy link

@irwir irwir commented Feb 2, 2025

This is a minimalistic fix for Visual Studio 2022 only.
Successfully builds for all Intel configurations.
It could be enhanced - for example, .filters files added to make project trees look better in IDE.

Please see if anything needs to be changed.

Additional remarks - separate issues might be created.

  1. Other IDEs such as Delphi and Borland also would need to be fixed.
  2. Projects for earlier versions of Visual Studio can be fixed similarly. Though most configurations, if not all, could be reduced - maybe to just one. In this case, Windows SDK Version and Platform Toolset might be requested to be changed or updated on the first opening of the solution/project.
  3. Over time, project files collected a bit of garbage and it should be cleaned. At least, ReleaseWithoutAsm configuration; currently there is no single *.asm file in the source code.
  4. When building a solution, Visual Studio may use several threads to compile multiple projects simultaneously. There is a non-zero probability of copy operation to fail (simply repeat the build, because at least one thread had created zconf.h). But, this is rare, and usually only the library project would be needed, not the provided solution.

Corrections in the related readme.txt
@Vollstrecker
Copy link

You're sure you want to keep all these projects? With the working cmake I would consider this all deprecated.

@irwir
Copy link
Author

irwir commented Feb 2, 2025

With the working cmake I would consider this all deprecated.

For those who still use ed for writing code, cmake could be the ultimate software.
Others may prefer the convenience of IDE.

@madler
Copy link
Owner

madler commented Feb 2, 2025

You're sure you want to keep all these projects? With the working cmake I would consider this all deprecated.

I would indeed like to deep six all of the VS projects if cmake is a viable alternative.

Does VS come with cmake? Or does it need to be separately installed? Does or can cmake generate a VS project?

@witte
Copy link

witte commented Feb 2, 2025

yes to both 👍

Does VS come with cmake?
"C++ CMake tools for Windows is installed as part of the Desktop development with C++ and Linux Development with C++ workloads." (from https://learn.microsoft.com/en-us/cpp/build/cmake-projects-in-visual-studio?view=msvc-170)

can cmake generate a VS project?
https://cmake.org/cmake/help/v3.12/manual/cmake-generators.7.html#visual-studio-generators

@madler
Copy link
Owner

madler commented Feb 3, 2025

Ok, then @irwir , why should I not delete the VS projects, if they can be generated using cmake?

@irwir
Copy link
Author

irwir commented Feb 3, 2025

yes to both 👍

Very brave statement.
Is it correct?

Does VS come with cmake?

This library has vc9 (2008) to vc17 (2022) configurations. Would you kindly provide a similar quote for VS2012 or earlier?

Even in VS2022 CMake is optional, as clearly written on the screenshot; neither IDE nor compiler depend on it. It safely could be unchecked and thus not installed (if user knows what and why he is doing).

can cmake generate a VS project?

https://cmake.org/cmake/help/latest/generator/Visual%20Studio%2012%202013.html
Is this a bit more complex than just opening a solution?

Dear CMake fanboys! Please use Discussions instead of hijacking every topic related to VS projects.

@irwir
Copy link
Author

irwir commented Feb 3, 2025

why should I not delete the VS projects, if they can be generated using cmake?

For the same reasons why many repositories provide VS projects, maybe?
It is native Visual Studio format.
Or, delete and add generated projects to release archives. See if it would be easier to maintains scripts than projects.
VS 2008 used .vcproj files, VS 2010 and later versions, most probably, could share the project - so it would be two instead of 6.

Alternatively, why you just do not return zconf.h? Avoiding the troubles in point 1 of the opening message, and this PR could be scrapped.

@madler
Copy link
Owner

madler commented Feb 3, 2025

Alternatively, why you just do not return zconf.h? Avoiding the troubles in point 1 of the opening message, and this PR could be scrapped.

I would like to do that. Though it is my understanding that that would screw up cmake. Is there a way to restore zconf.h in the distribution that is compatible with cmake?

@madler
Copy link
Owner

madler commented Feb 3, 2025

Or, delete and add generated projects to release archives. See if it would be easier to maintains scripts than projects. VS 2008 used .vcproj files, VS 2010 and later versions, most probably, could share the project - so it would be two instead of 6.

I can't quite parse that. Can you elaborate on your proposal?

@Vollstrecker
Copy link

Bringing zconf.h back from cmake POV is not that big problem. If in doubt it could go into a subdir that get's included in VS-projects and is ignored everywhere else. Setting ZPREFIX would mean to edit a file which could be avoided, but everything else is doable if stuff like unistd.h can be assumed to be always there.

The important question is: How old is a Visual Studio that's not supported by cmake? A recent version of VS is free for private and open source usage on recent versions of Win. So why support WinXP instead of just telling people to upgrade their systems that are not only unsupported by cmake, they are unsupported by MS and shouldn't be used at all anymore. Like it or not, it's just like it is. And yes devs of commercial software need to pay for newer VS, but that's no excuse to keep dead plattforms alive.

@Neustradamus
Copy link

Maybe it can be separated in several repositories?

@irwir
Copy link
Author

irwir commented Feb 3, 2025

I would like to do that. Though it is my understanding that that would screw up cmake. Is there a way to restore zconf.h in the distribution that is compatible with cmake?

This is a case for CMake gurus to shine; there should be a way.
Searched the net (as a dummy in CMake):
https://developernote.com/2024/09/copy-a-file-with-cmake/
https://stackoverflow.com/questions/34799916/copy-file-from-source-directory-to-binary-directory-using-cmake

I can't quite parse that. Can you elaborate on your proposal?

As an example: a repository has no .vcxproj files; but it has project generator scripts instead.
The releaser runs the scripts and adds projects into zlib132.zip - to be published as the release.
Then library user is releaved from necessity to figure out how to create the projects and do multiple preliminary steps before starting IDE.

So why support WinXP instead of just telling people to upgrade their systems

People habitually try to teach other how to live.
Spoiler: teachers commonly get replies with unpleasunt personal advices. :)

@madler
Copy link
Owner

madler commented Feb 4, 2025

Bringing zconf.h back from cmake POV is not that big problem. If in doubt it could go into a subdir that get's included in VS-projects and is ignored everywhere else.

I would like to bring back zconf.h to the base directory, independent of whether there are or are not VS projects. Can that be done?

@Vollstrecker
Copy link

Fir bringing it back, I guess configure also needs adjustments. Suggestion: A Branch where you put zconf.h in place with reasonable defaults and adjusted configure and I adjust to cmake to overwrite the defaults that don't fit. I'm home tomorrow and the day after, should be plenty of time.

So why support WinXP instead of just telling people to upgrade their systems

People habitually try to teach other how to live. Spoiler: teachers commonly get replies with unpleasunt personal advices. :)

I just asked but I guess youi say "I could tell you a good reason but actually it's a dumb one so I answer that way".

@irwir
Copy link
Author

irwir commented Feb 4, 2025

Though it is my understanding that that would screw up cmake.

Possibly a silly question.
Could a directive file(REMOVE zconf.h) be used in the beginning of CMakeLists.txt in order to unscrew the issue?

@Vollstrecker
Copy link

It could, if there would be an issue. By now cmake parses zconf.h.in and adds the needed stuff to generate zconf.h, so I don't really care what file to parse, the only thing needed is to make sure that the generated one (or better it's path) appears before the other one in the compile command, and that include statements in the code respect that.

Removing the file would pollute the git tree, the whole cmake-part is designed for complete out-of-tree builds. There's nothing more annoying than have changed file in git just because you've built the source.

@irwir
Copy link
Author

irwir commented Feb 5, 2025

Until very recently, zconf.h never needed "generation", it was static; VS and make would nicely build the library.
Additional macros could be sent to compiler as command line parameters.
The point of keeping git tree untouched is understandable, but the idea of mangling the sources because of a build tool is simply bad.

@Vollstrecker
Copy link

Parsing is done basically to set Z_PREFIX from configure or cmake options. Plus there are checks for stadrg.h and unistd.h presence that need to be set in zconf.h as HAVE_STDARG_H and HAVE_UNISTD_H.

I guess these checks are there for a reason. I didn't check all the code, if they are needed and missing them would break everything, we could remove the checks for them in the code completely and let configure and cmake fail if they are not found, if they are really optional there's no real option other than either generating this file, or removing the statements and give the flags as compiler switch.

@Mr-Clam
Copy link

Mr-Clam commented Feb 5, 2025

The ./configure script for makefile builds also modifies zconf.h (and Makefile). The make command really pollutes the source folder. i.e. it's not unique to CMake, although obviously could be better. I'd rather zconf.h was renamed to something like zconf.h.in and zconf.h is generated from it (in a separate build folder in the case of Cmake), or something like that.

This PR looks incomplete to me: it only adjusts the VS2022 project file. Or, are all the other VS project files correct and that's why they haven't been adjusted?

And to add my twopence: I'd get rid of the VS files from here because they're redundant and easily out-of-date due to maintenance issues. Cmake can generate native project files for VS 2008-2022 plus nmake projects, which are more likely to be correct and without the maintenance overhead. It can also generate Xcode projects and other project types too for both IDEs and command line builds. I'd suggest getting rid the Makefiles and configure script and just leave CMake, but I'm sure Mark will point out some esoteric platform with no CMake binaries is in use somewhere ;).

@madler
Copy link
Owner

madler commented Feb 5, 2025

I'd rather zconf.h was renamed to something like zconf.h.in and zconf.h is generated from it (in a separate build folder in the case of Cmake), or something like that.

There is a zconf.h.in from which configure writes zconf.h. That's always been the case, and still is in the current develop branch. So for configure and make, there does not need to be a zconf.h. The change in the current develop branch is now zconf.h is gone.

I would like for zconf.h to return to the top directory of the distribution, so that it is possible to compile zlib without make or cmake or bazel or vstudio or xcode or anything. Just compile all the source files in the top directory and link them. Done.

The make command really pollutes the source folder. i.e. it's not unique to CMake, although obviously could be better.

I would like to understand this pollution better. Why can't a zconf.h in the top directory co-exist with cmake?

@madler
Copy link
Owner

madler commented Feb 5, 2025

Cmake can generate native project files for VS 2008-2022 plus nmake projects, which are more likely to be correct and without the maintenance overhead.

Maintenance is the reason I would like to remove all of the VS project files from the distribution. Can someone provide instructions for generating them using cmake, so I that I can include that in the distribution in lieu of the project files?

@irwir
Copy link
Author

irwir commented Feb 5, 2025

I would like to understand this pollution better.

If making process changes contents of zconf.h, git would try to push this file.

Maintenance is the reason I would like to remove all of the VS project files from the distribution.

It would be nice to keep the projects in the distibution.
Only 2 versions are required 2005-2008 and 2010.

@Vollstrecker
Copy link

I would like to understand this pollution better. Why can't a zconf.h in the top directory co-exist with cmake?

Just into a clean checkout of the repo and run configure followed by git status, there are changed and unversioned files. If you're using an IDE these also get listed in the part for git and you always need to be carefull to not commit them with your changes.

Can someone provide instructions for generating them using cmake

Just run cmake -h and you'll get list of available generators for you current system. Choose one and append -G "" to your cmake call. You can the this in the actions. If you use cmake-gui and press configure for the first time, you'll get a dropdown-list with the available one and choose from there.

I just checked, on my Win10 with VS17 I get offered VS9 to 17, except 13. All are list with year: Visual Studio 9 (2008) and all are 2 years apart, so I guess 13 never made it to the public. So with this everything in the contrib/VS-dir seems covered. And yes, I didn't check if you get cmake-binaries for a system designed in 2008 but I'm pretty sure if anyone wants to compile zlib there it's not to much to compile cmake there.

@madler
Copy link
Owner

madler commented Feb 5, 2025

Just into a clean checkout of the repo and run configure followed by git status, there are changed and unversioned files. If you're using an IDE these also get listed in the part for git and you always need to be carefull to not commit them with your changes.

I am thinking of someone getting the distribution and only using cmake, never configure and make. What if zconf.h is there, but you have not and will not run configure? Is there any issue?

@Vollstrecker
Copy link

Just for fun I created zconf.h with just #error in it. No problem here.

@Vollstrecker
Copy link

A bit too fast, as I was in the wrong dir. I also needed to change #include "zconf.h" to #include <zconf.h> in zlib.h to make it work.

@irwir
Copy link
Author

irwir commented Feb 5, 2025

I also needed to change #include "zconf.h" to #include <zconf.h> in zlib.h to make it work.

Strange. Standards pdf tells, if "..." form fails, it is repeated in <...> form.

@madler
Copy link
Owner

madler commented Feb 5, 2025

A bit too fast, as I was in the wrong dir. I also needed to change #include "zconf.h" to #include <zconf.h> in zlib.h to make it work.

I'm not sure exactly what you made work with what when changing what, but anyway, are you saying that having a zconf.h in the top directory is not a problem?

@Vollstrecker
Copy link

Not a problem.

Strange. Standards pdf tells, if "..." form fails, it is repeated in <...> form.

Would be new to me. They are defined with different behaviour and if it's there by intent there can be various problems if that is altered. Is this a compiler pdf or some optional feature to turn on?

@irwir
Copy link
Author

irwir commented Feb 5, 2025

Is this a compiler pdf or some optional feature to turn on?

C language standards; therefore - any conforming compiler. This behaviour is from C11; C90 made no such distinction.
Never encountered any peculiarity with either form of include directive in different compilers.

@madler
Before closing this PR, please use readme.txt file changes.
Apparently, there never were any real issues with CMake itself.

@madler
Copy link
Owner

madler commented Feb 6, 2025

@Vollstrecker Well, I tried putting zconf.h back, and now there are failures in the workflows. E.g. https://github.com/madler/zlib/actions/runs/13169400570

@madler
Copy link
Owner

madler commented Feb 6, 2025

@madler Before closing this PR, please use readme.txt file changes. Apparently, there never were any real issues with CMake itself.

@irwir I don't understand either comment. Please elaborate.

Vollstrecker pushed a commit to cmake-remake/zlib that referenced this pull request Feb 6, 2025
Vollstrecker pushed a commit to cmake-remake/zlib that referenced this pull request Feb 6, 2025
@irwir
Copy link
Author

irwir commented Feb 6, 2025

Please elaborate.

This PR would not be merged "as is".
In addition to project files, the PR also includes related readme.txt and these changes are valid.
Do you need a separate PR for readme.txt?

As for CMake, it can do well with zconf.h in distirbution.
Decision to remove the file and add it to .gitignore was made for reasons, unrelated to making and compilation.

@Mr-Clam
Copy link

Mr-Clam commented Feb 6, 2025

A comment about the VS readme.txt file: the link on this line is dead, even if I change it to HTTPS.

@madler
Copy link
Owner

madler commented Feb 7, 2025

@Vollstrecker I tried your change to use zconf.h instead of zconf.h.in for cmake, but the actions still result in lots of errors.

@Vollstrecker
Copy link

Seen that, investigating that, running out of time for this week.

On my system mostly the find_package tests fail, but I guess I missed some cornercase somewhere else.

@madler
Copy link
Owner

madler commented Feb 7, 2025

Seen that, investigating that, running out of time for this week.

Thanks! No rush.

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.

6 participants