CMake: improve Windows build#14959
Conversation
FranciscoPombal
added
OS: Windows
FranciscoPombal
force-pushed
the
cmake_dynamic_build_install
branch
2 times, most recently
from
fed4dc5 to
9e42428
Compare
|
Perhaps you should add installation step to windows builds on github actions. |
Copying all the dependencies to build directory doesn't seem like valid building workflow, IMO. It is absolutely unnecessary in correctly set development environment (I don't mean just IDE). If you want all to be copied into some target directory just install it there. |
glassez
changed the title
Only for the static builds, right? OK. CI for the dynamic builds will warrant a bit more disicussion because I don't think there is any more space for caching dependency artifacts, hence this comment in the OP:
What do you mean? The standard way to do the "debug" part of the "edit/compile/debug" cycle (IDE or not) is to just point the debugger at the executable generated in the build directory, and it should "just work". If some required runtime DLLs are not there, it will not work. Such is life on Windows. I'm sure it can also be done in some other roundabout way by configuring debugger search paths, etc, but this is simpler and more closely mimics the final setup that will be deployed to the user.
Regardless of matters related to development setup, copying the remaining runtime DLLs to the build directory is a prerequisite to make installation trivial. Maybe you would prefer if the full "runtime tree" were generated to its own sub directory of the build folder, so that it is not at top-level? For example, copying |
It "just works" in correctly set development environment that I mentioned above (i.e. when all runtime dependencies are reachable via environment variables etc.). |
That's in the Linux world, which I prefer as well - where every library, header file, etc is neatly stored in standard, well-known paths, all properly configured and ready to go, and automatically managed by a package manager. But on Windows, where there is no such thing (beyond core system libs) and the common practice is to bundle everything separately between programs, it is much more practical to do it this way, rather than fiddling with See for yourself, many people face this problem: https://www.google.com/search?q=visual+studio+debug+missing+dll. Of course those needing to ask these questions are mostly novices. But those who don't are experts, whose precious time is wasted configuring such things. [1] And it will become even smarter from 3.21 onward, thanks to https://gitlab.kitware.com/cmake/cmake/-/merge_requests/5837. |
I don't talk about Linux/Windows world. I talk about "correctly set development environment" that is configured by developer to maintain all needed workflows (building, testing etc.). It isn't dependent of any OS. Yes, from time to time I use Linux for development purposes and I use several non-default libraries/tools so they are located outside well-known system paths, but my "development environment" is still "correctly set" so I don't need apply such contr-productive actions like copying all dependencies in build dir. |
|
If we are talking about installing on Windows (at least in passing), then wouldn't it be better to make it possible for CMake build script to produce a real installer (e.g. NSIS) package for all types of builds (static and dynamic)? |
You are missing the point. It's not about complexity, it's about comfort and ease of use (basically, improving developer UX). And it's not just newcomers I care about (and I do care about them, not just "supposedly"). Everyone on Windows expects it to work this way in the best case, and when there are not many special requirements (which is the case for qBittorrent, it's a very run-of-the-mill GUI application). If it doesn't work this way, it's not the end of the world of course, but the newcomers will struggle, and the experts will waste their time unnecessarily once more.
Lowering barriers of entry and simplifying workflows and setup is important. That's how you get more people into the project, both experts and newcomers alike. This is not a cathedral, it's a bazaar. It's for the community, by the community; the purpose of this project is not to stroke some developer's ego because they managed to setup and maintain an unnecessarily complex development setup, as opposed to some filthy gamer who could never achieve such a thing (I have no strong opinion about this comparison to video games, I simply don't understand it - this was my best interpretation of what it could mean).
I did not say you never needed to use "non-default libraries/tools" on Linux - in fact, I usually install the latest version of Boost to Do you think stuff like this: https://github.com/qbittorrent/qBittorrent/wiki/Compilation:-Windows-(MSVC-2019,-64-bit,-static-linkage) is the best we can do? When I set out to improve the build system and the CI, the goal was to improve the project by making certain tasks easier and more accessible. I'm certain you agree this goal has been met. Whereas previously, when a user wanted to test an ongoing PR, you'd have to tell them "oh hey, you just have to follow this horrendously long procedure to compile from source" or "wait for some contributor to have the time to build it for you", now they can simply download the test build and provide feedback. Many users have been taking advantage of this and providing feedback more quickly; some of them who provide valuable feedback would not have otherwise been able to do so, either due to lack of time to compile themselves or expertise. In fact, I would bet you and other contributors have made use of the artifact downloads to quickly test things yourself without having to pull the PR and wait on the build (I sometimes do this, especially for Windows stuff). Through simplification and improving accessibility and developer UX, efficiency was improved - more people can do more things done quicker and with less effort. This is yet another step in that direction. Additionally, the maintainers' time was freed up from having to maintain dependency bundles for the CI on the website; now everything is handled automatically and quickly (due to the caching). This in turn means we can be quicker in updating dependencies for CI. I should also reiterate that these improvements are not solely targeted at "making it easier for newcomers", and especially not at the expense of more experienced users. Sometimes things get messy, and improving developer UX also means making it easy for experts to really dive in and dissect everything when needed. And, of course, this change is obviously not at odds with this goal either. Perhaps this does not seem like a big deal to you, because you are biased by how long you've been doing this, but trust me, it is. That's fine, for most people, once something becomes a habit it severely distorts the perception of the areas where the task could be improved/simplified (this is the case for me as well in other matters). Please keep that in mind. |
To reiterate this: can I slog through https://github.com/qbittorrent/qBittorrent/wiki/Compilation:-Windows-(MSVC-2019,-64-bit,-static-linkage)? Yes. Did I do it? Yes, more than one time, unfortunately. Can many other people do it? Of course, this is not rocket science. Do I wish to do it again? No, unless possibly as a means to try to track down and isolate a very specific and evasive bug (was never the case thus far). Are there people who would have otherwise been able to help, but just gave up (not necessarily due to lack of technical capabilities)? You bet. Very quickly I started thinking "there must be a better way", for most cases, and worked towards it. Similarly: can i setup the paths to find the required DLLs to debug the qBittorrent executable in the build folder? Yes. Can many other people also do so? Of course. Do I or others want to do it, given that there is an alternative way that is simpler and sufficient for 99% of cases? No. Are there people who would have otherwise been able to help, but just gave up (not due to lack of technical capabilities), and instead contributed their time and expertise to other FOSS projects that were easier to setup a development environment for? You bet. |
Just to be clear, the point is to test (run through) the install step of cmake script and ensure it is working. |
|
@FranciscoPombal @glassez |
👍
👍 |
|
@FranciscoPombal But as suggested above, if our community wants this "feature", we can provide it as an option. |
Honestly I still see no valid reason to distort meaning of build action instead of just apply install action if you need exe and its dependencies to be installed in some location. You can even set build dir as installation target dir if you want, can't you? |
|
(if you are in a hurry, just read the last 2 paragraphs at the bottom, below the horizontal line/separator)
👍
Seems like a reasonable stop-gap solution for now. Ideally we would of course want to cache the dependencies needed for the dynamic builds as well. I think the best way for now is to make it an entirely separate workflow (e.g. "CI for dynamic builds") Maybe GitHub will increase the available space for cached artifacts in the future, so that this can be simplified.
It seems to me like these statements contradict each other. If there is supposed to be a clear distinction between "install" and "build", why is it suggested that I run the "install" step if I just want to build for debugging? The install step is not meant to be executed as part of the normal development cycle.
No. building step should produce artifacts and do whatever else is necessary in order for the executable to be usable for the build folder. In general, on Linux nothing else needs to be done. On Windows, it means potentially gathering and copying in artifacts to the build folder. The purpose of the install step is to gather all artifacts for deployment for deployment on an "actual" install location, to simulate what will end up on the users' machine. Of course, these need not be the same, in qBittorrent's case it just so happens that the artifacts copied are all the same. But suppose we started including the As I've shown by the link to the google search above, this is a practice that is widely expected. But if you want to argue "those are just noobs", then I refer you to this comment by the maintainer of CMake himself: https://gitlab.kitware.com/cmake/cmake/-/issues/20417#note_707008 (and really, that whole thread and related threads), if you are more receptive to that argument of authority. You should consider that you may be a bit out of touch here.
As mentioned in the previous paragraphs, running the "install" step as part of the normal edit/compile/debug step is in violation of the separation of concerns that you guys champion.
It seems to me you're implying the "Windows way" is at completely at odds with the general and widespread use of the CLI and CLI tools on Windows. This is no longer the case, if you look at what Microsoft has been doing for a while now. In the past years they have made a great effort to provide better command-line tooling. If there's a GUI on top of that to expedite certain common workflows, great, but the CLI is no longer a second-class citizen on Windows. Look at Powershell Core,
Yes, it is possible. You can open the dialogs in Visual Studio or whatever to define the correct settings. Or, with this change, you don't need to do it. It is also possible to define those settings, PATHs, etc. via the command line - again, with this change, you don't need to do it. This is not about GUI vs CLI. It's about having to do less work with both by default.
I said I personally preferred it, and others may as well if they have certain special requirements. I did not say others should do the same. You can also use the installer (the more "Windows" way), and won't be worse off for it.
If one can do math with an abacus, why do we need calculators? Yes, this is an exaggeration, but the point is that in that sentence you're implicitly recognizing that there is an advantage in implementing this change, but you don't want it essentially because in your opinion he current status quo is not that much harder. If it can be improved with no downside, why not do it? Anyway, speaking of "no downside", let's look at this in practical terms: in what way does this new way of doing things impact you negatively? If you can provide objective reasons for why this breaks/worsens your workflow, I can start to work on solutions with you. After all, I don't want to implement something at your- or anyone else's expense whose workflow would be disrupted by this change. But, if it's all the same to you, why prevent implementing and enabling by default a feature that is clearly useful for many other people, and improves their experience?
If this disrupts your existing workflows in some way, this can simply be gated behind a feature option that is |
no one will run this step for a compile, debug cycle, it's would just add extra time. we would like to have "build" as fast as possible to do dev work, you're just adding extra unnecessary steps here.
No, the building is literally mean building, not aggregating dependents that's an install step.
nope it doesn't
you only perform this one time when setting the environment, your solution will run with every compile run. |
Exactly. Hence why copying of dependent DLLs to the build folder should be done as an automatic post-build step. EDIT: also, having to set up PATHs and whatnot when first starting out also prevents you form building as fast as possible at first...
Look. I have provided sourced, real-world examples contradicting this, including one from other big CMake users and developers. Thus, at a minimum, I expect you to provide sources to contradict my claims, "nope it doesn't" isn't good enough here. Also, in case you missed it, a small clarification, in case you missed it: building means that at the end of the process, running the executable from the output directory should "just work". On Windows, there are 2 ways to make it "just work". Either setup paths, or have your build tool copy the necessary dependents to the folder. I should not have to convince you at this point that the latter saves you work and keeps your setup simpler and more self-contained.
Is this supposed to be an argument about the performance of the post-build operation? Because as you can see, my solution uses EDIT: if you want to contribute constructively to the discussion, please address this:
|
|
@FranciscoPombal look I don't think none of the other active devs actually uses CMake, modifying paths is just how it works on windows and I don't have enough motivation for argument. so I would really like you to at least put this under a define, this way you won't have to bump the CMake version as well and will keep your intended audience happy. This feature can be useful for release builds but I really don't see much use of it in debug builds (for my case at least), Also since it will become more "smarter" in cmake 3.21 why don't just wait until then it should be released in like 15 days. |
© Francisco Pombal. All rights reserved. |
|
@FranciscoPombal I'm really trying to find proof that this is a really useful thing (although you may not believe me). Let's move on...
Could it be this? It does "make installation trivial" by simply moving the execution of all non-trivial actions from the installation step to the build step! This is obviously illogical, because the build step is performed much more often than the installation step, and most of the build cases do not imply further installation (if we are talking about developers workflow). In addition, it is not clear why it is impossible to still have this non-trivial logic where it is really required (in installation procedure)?
This does not apply to this topic at all. No need to abuse it. |
glassez
left a comment
glassez
left a comment
There was a problem hiding this comment.
Well, I had to change my development environment to use the CMake installed by Visual Studio Installer (although it doesn't look as convenient as it was with Qt Installer). So now merging this PR won't break my workflow.
glassez
requested review from
Chocobo1 and
jagannatharjun
and removed request for
Chocobo1
Wrong :)
Yes, I substantiate my arguments. That's a bad thing?
You keep insisting on some vague notion of "formal proofs" in contexts where it does not make much sense. This is not a mathematical discussion. This is about conventions and making development experience easier. In some cases (such as this one) substantiating ones position by showing that many others do the same thing is the ultimate form proof that you need. Unless you are doing some really weird/novel thing (which is not the case for qBittorrent, it is a relatively simple GUI download manager), it is a good thing to do certain things the same tried and true way that others do it. See https://en.wikipedia.org/wiki/Convention_over_configuration. Of course "many people do it X way, so we should also do it" is a very dangerous way to think in general, and I'm sure you agree with that. But, in this case and others, it is a good thing to do the "standard" thing and/or the thing that many people request to be done a certain way.
I do care about your opinion, otherwise I wouldn't bother engaging with you. It is valuable to engage with people with different perspectives.
I believe you. The thing is, you seem to think I'm trying to convince you that this is a "really useful thing" for you. That is not what I'm trying to do. This might in fact never be useful for you, and that's completely fine. You might also find in the future that this turns out to be useful for you and that's fine as well. What I'm trying to say is that this is useful for many others (and I believe I have provided enough evidence of this), while not breaking your existing workflow. I'm not trying to convince you that this will change your life. What I have trouble understanding is your opposition to something that can clearly benefit many people while not impacting you at all. From a utilitarian perspective, it is a clear-cut improvement - it has an upside and no downside. And no, I don't thing requirink a simple upgrade to CMake counts as a downside - if you do, I guess we just agree to disagree here. Everyone has to have CMake installed and updated anyway. I probably wouldn't even have considered submitting a PR if the changes I proposed implied telling a bunch of existing contributors/team members "hey, remember how you used to do things? Well from now on you'll have to do it this way". A big selling point of this PR IMO is that no one is forced to do anything any differently.
You keep centering your argument around "my preferred way of doing things is not difficult, so the others are invalid". This is very obviously not about sufficient mental faculties. Of course anyone could do it your way. It's just that clearly many people prefer to do things another way. The reasons for this can be various, and if the implementation does not impact you negatively, why stop it? For example, consider the case of users who are involved in many different projects - they might not wish to deal with an env var jungle in their systems, and might prefer to have each project more self-contained rather than have them leak stuff to the system's environment. Or even just dealing with environment variables at the project level - some just appreciate the convenience of not having to fiddle environment variables, the build system does the tedious work for them.
Now this is an instance of an argument that does require "formal proofs"/hard data to substantiate - extraordinary claims require extraordinary proof. The burden of proof is on you to supply profiling data showing how this adds "a few extra seconds" (a simple average of three
Again, I believe I've shown how the shortcomings you mentioned are nor really there (except for "forcing" you to update CMake, which again, we can agree to disagree on that one). And, more importantly, I've clarified how this does not impact your workflow, while being useful for others and that I'm not trying to convince you (or others who prefer other workflows) to switch to this workflow.
The thing is, CMake is moving towards making it trivial to do this way. Consider that in the new native feature that they are introducing in a future version, the use case I describe is exactly the example use case shown in the documentation: https://gitlab.kitware.com/cmake/cmake/-/merge_requests/5837/diffs#cad53f118ab22a5cf1bd8bea8eb4c9b39cbfa8dd_941_949. This is convenient for many people. As for non-trivial logic: again, once CMake releases the new version with the I wish I had a good example at hand of how this in turn makes installation easier from some experiments I made, but I can't seem to find them right now... Regardless, you'll be able to see it once I submit such a PR... For now, feel free to ignore this argument about "making installation easier", my above points stand on their own just fine and are enough to justify this.. |
I can assure you installing CMake with its own installer (or zip) is even more convenient, and means you're no longer dependent on a 3rd party bundling the most recent CMake version.
It never did in any reasonable way. You were creating your own problems (incompatible CMake version) by refusing to install the most recent version the standard way, instead of through a 3rd-party bundle. |
|
Small note:
This is already the case, I have already agreed to this. This feature is gated behind the optional, disabled by default |
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
|
@sledgehammer999 @Chocobo1 All done, any further comments? |
Note that I would not have approved it so easily if this feature was of my personal interest. Its implementation may raise a number of questions. But let those who intend to actually use it worry about it. |
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
|
Any further comments? This is ready. |
|
@FranciscoPombal |
|
also ref.: microsoft/vcpkg#18955 |
|
BTW, how long is the linking against qbt_webui.lib supposed to be when we follow your (incredible) wiki ? On a 10920x, it's at it for more than an hour now... But no error so I let it run for now.. EDIT : my bad; the wiki is working great, I messed up something... |
Doesn't seem to be correct... |
|
This PR is stale because it has been 60 days with no activity. This PR will be automatically closed within 7 days if there is no further activity. |
|
@glassez @Chocobo1 Should we revise this update plan & perhaps implement 2/3 of these commits in a New PR:
MSVC 2022 which will be GA ~November 8th & is Qt docs seem to suggest we use
Reference: Build Tools
Reference: System RequirementsAll desktop platforms
Reference: |
|
This PR is stale because it has been 60 days with no activity. This PR will be automatically closed within 7 days if there is no further activity. |
|
This PR was closed because it has been stalled for some time with no activity. |
7 participants
CI: Run installation for Windows static builds
CMake: improve Windows build
All necessary runtime components for dynamic builds are now copied to
the build directory as a post-build step, improving the
edit/compile/debug cycle experience for these builds, as no PATH setup
is necessary.
Windows installation was revamped and fixed for both static
and dynamic builds. The default behavior is less surprising: all
necessary files are installed to
C:\Program Files\qBittorrentbydefault.
CMake: clarify variable purpose and expansion
as separate parameters.
_LISTsuffix to all variables that are meant to be lists.CMake: Migrate to CheckSourceCompiles
CMake: bump minimum version to 3.20
This a "Qt5 backport" of the efforts posted in glassez#1. CI for dynamic builds is purposefully not included here, that is to be discussed and maybe implemented in another PR.
The bump to CMake 3.20 is perhaps a bit unfortunate, but
vcpkgalready works with it, GitHub Actions supports it, and we've been dropping support for older OS's in master, so I don't think the fact that its not in many repos yet is a big deal (not that installing the latest version of CMake is hard... https://youtu.be/_yFPO1ofyF0?t=45)