-
Notifications
You must be signed in to change notification settings - Fork 287
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
build: fix packages aged out of mirrors breaking MSVC build #5910
Conversation
Update package list pin on vcpkg to the latest release version Signed-off-by: David Li <[email protected]>
f13f1eb
to
65d3b11
Compare
Both the nightly release and manual-release builds use `msbuild`, while the CI job for PRs use `cmake`. This leads to a weird situation where both the release builds run without issue, but the CI job fails. Signed-off-by: David Li <[email protected]>
65d3b11
to
946edc4
Compare
can't we keep using cmake? we should make releases use cmake not the other way around in order to unify all toolchains with cmake |
Getting cmake to work with I think in terms of getting things to build again, we should standardize on |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The way I look at it, we want MSVC now for Windows users. Unifying technology is good when the game builds at all. After trying to make a release on my fork I see this is why it doesn't work.
We want to have builds first before we worry about improving them. Negatively impacting the playerbase is a losing proposition in the long run.
I'll wait for others to review, but I want this merged before tonight's nightly release
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I crave
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I am very conflicted as to my opinion about removal of CMake. I could very easily see "later time" turning into "several years later" given the Linux v.s. Windows split of people who know how to build at all, much less how to work with the github releases and builds, and we really should bring all the builds over to CMake... but I also recognize that we'd generally prefer MSVC builds to be available, and migrating the rest of the builds to CMake is out-of-scope.
In summary, stuck between a rock and a hard place
Also, it's worth noting that the CMake + MSVC build on PRs has been working just fine even while the MSVC releases haven't been working, so I'd wonder if it wouldn't be better to see if we can't instead base releases off of that? Or has that already been considered? |
I agree it'd be great if we can rip out msbuild and change the release builds to use CMake. I don't have a Windows dev environment to look into that, but given there's no urgent need to land this PR right now there's time if someone wants to take a crack at it.
I don't know if that's 100% true. I might be misinterpreting the logs, but I think the cases of recent successfully builds have been pulling the packages from the CI workflow cache - i.e. if the cache were to expire and they were to try and pull the packages from upstream mirrors, they would fail. i.e. https://github.com/cataclysmbnteam/Cataclysm-BN/actions/runs/12699138333/job/35399125011 didn't download anything from upstream mirrors. |
Well I went to sleep and we lost MSVC last night so I guess now it's not urgent. |
The annoying thing is, having looked at the failed PR build, it's literally one package/library that can't be found by SDL2_mixer: libxmp |
Looking further into it: |
I just fixed CMake + MSVC on my own branch! The solution was, funnily enough, just pinning SDL2-mixer to a version before they introduced the libxmp stuff that borked it all up. 2.6.3#1 was what we were using before, and thus it's what I pinned it to. |
Superseded by #5934 |
Checklist
Required
main
so it won't cause conflict when updatingmain
branch later.Purpose of change
MSVC builds are breaking because vcpkg is pinned to a 1y+ old version, and some of the packages
in its package list just aged out of retention on package mirrors.
Updating
vcpkg
reveals a secondary issue where both nightly and manual release pipelines work but the CI for PRs break. This is because both the first two trigger the build viamsbuild
while the PR one usescmake
, which seems to cause dependency resolution problems.Describe the solution
Newer versions of the
lukka/run-vcpkg
workflow step can read the pin from thevcpkg.json
so we can avoid repeating ourselves across many files.msvc-full-features-cmake
build (used for PRs) to usemsbuild
, to mirror the behavior of the actual release pipelines. We also build theCataclysm-test-vcpkg-static
target, which is skipped in the release builds.Note: We're building in Release mode as the hacking way we're pulling in some SDL and ttf libraries doesn't seem to support Debug builds.
Describe alternatives you've considered
It looks like a specific version is necessary, so we'll need to bump this every year or so.