-
Notifications
You must be signed in to change notification settings - Fork 45
New issue
Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? # to your account
Trying to fix all of the builds #352
Conversation
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… :( |
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. |
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. |
If that's not the reason why Windows requires extra include statements, then the reason must be even more illogical and terrifying. |
This reverts commit 48fa461.
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. |
proj/vs2017/vcpkg.json
Outdated
@@ -1,4 +1,5 @@ | |||
{ | |||
"builtin-baseline": "007aaced1a9d3245e28a2ba9395dca88ea890db1", |
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.
What is this…?
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.
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.
I made a redo PR of this, minus the changes that broke it after the one build passed |
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.