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

Trying to fix all of the builds #352

Closed
wants to merge 25 commits into from

Conversation

NQNStudios
Copy link
Collaborator

For now, I can't figure out why, but my fork won't run the CI actions. So, I don't know if this is going to work, but I've tried adding a dependency to install-deps.bat and we'll see what happens.

@CelticMinstrel
Copy link
Member

CelticMinstrel commented May 27, 2024

What was the reason to include boost/filesystem.hpp?

Unrelated, but it looks like the MacOS checks are never going to run because GitHub removed support for the version they want to run on… :(

@NQNStudios
Copy link
Collaborator Author

The reason to include filesystem.hpp (as far as I understand it) is that Visual Studio handles recursive includes differently than gcc does... I think.

I think for gcc, if file 1 includes file 2 which includes file 3, then file 1 can reference names defined in file 3. I think for Visual Studio's compiler, file 1 needs to include file 3 directly to get those names.

I could be wrong about why, but adding the includes definitely fixes the windows build.

@CelticMinstrel
Copy link
Member

That doesn't make sense for reasoning… but adding them alongside operations.hpp may be redundant, I think? That is to say, if we're including filesystem.hpp it is quite likely redundant to also include filesystem/operations.hpp.

@NQNStudios
Copy link
Collaborator Author

If that's not the reason why Windows requires extra include statements, then the reason must be even more illogical and terrifying.

@NQNStudios NQNStudios changed the title Trying to fix the CI Visual Studio builds Trying to fix all of the builds May 28, 2024
@NQNStudios
Copy link
Collaborator Author

Trying to pin the dependencies on Windows and Mac brought only heartache, so I reverted all my commits that were trying to do that (at least for now).

For some reason the visual studio builds failed this time, looks like with a network error when downloading packages? Trying to re-run those builds later should hopefully fix it because I think this commit is identical to the one that worked.

@@ -1,4 +1,5 @@
{
"builtin-baseline": "007aaced1a9d3245e28a2ba9395dca88ea890db1",
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What is this…?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's a horrible thing that vcpkg requires if you want to specify an out-of-date version of a dependency. It's a commit hash for the vcpkg central repo. I put it in correctly and it still didn't work.

@NQNStudios NQNStudios mentioned this pull request May 28, 2024
@NQNStudios
Copy link
Collaborator Author

I made a redo PR of this, minus the changes that broke it after the one build passed

@NQNStudios NQNStudios closed this May 28, 2024
@NQNStudios NQNStudios deleted the fix-vs-build branch September 8, 2024 20:32
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.

2 participants