Skip to content

Attempt to fix Windows build #3635

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.

Sign up for GitHub

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

Jump to bottom
Merged
rcurtin merged 28 commits into from
Apr 22, 2024
Merged

Attempt to fix Windows build #3635

rcurtin merged 28 commits into from
Apr 22, 2024

Conversation

@rcurtin
Copy link
Member

@rcurtin rcurtin commented Feb 23, 2024

Not an uncommon occurrence, we're seeing C1060 (out of RAM) errors again. So... the strategy as per usual is to screw around with the options until it works again. Let's see what happens...

rcurtin added 24 commits February 26, 2024 11:46
@rcurtin
Try using the newest version of cereal instead.
64215b1
@rcurtin
Fix curl syntax.
40fbbc2
@rcurtin
Enclose include directory in quotes.
d09d296
@rcurtin
What happens if we build in Debug mode instead?
d75e38c
@rcurtin
Switch everything to debug to see what happens.
05d7493
@rcurtin
Try disabling cotire to see what happens.
ea3c098
@rcurtin
This ultimately won't work, but will it compile?
1487a36
@rcurtin
Revert "This ultimately won't work, but will it compile?"
38df41b 
This reverts commit 1487a36.
@rcurtin
Revert "Try disabling cotire to see what happens."
9eac8e0 
This reverts commit ea3c098.
@rcurtin
Build in Release configuration.
a7561a7
@rcurtin
Are these the problem files?
dd7e3d2
@rcurtin
Try commenting out more files to see if this fixes compilation.
dec455d
@rcurtin
One more test to comment out.
7386398
@rcurtin
Try to use CMake features for precompiled headers directly.
bca8cdc
@rcurtin
Fix ambiguous symbol error.
507139b
@rcurtin
Try to precompile Armadillo and ensmallen headers.
b8768ac
@rcurtin
I don't understand why this would cause a problem.
8a0087d
@rcurtin
Merge branch 'master' into fix-windows-build-the-usual
c0d4c85
@rcurtin
Can we start re-adding tests?
3fbf405
@rcurtin
Carefully add a few more test sources...
4c94001
@rcurtin
Add all the tests back.
eccf99c
@rcurtin
Adapt DET tests to no longer use awful private/public trick.
30dc7f3
@rcurtin
Remove final bits of cotire and adapt for compatibility with other se…
3cd4299 
…tups.
@rcurtin
Actually include the compatibility fixes (oops).
1e5c090
@rcurtin
Copy link
Member Author

rcurtin commented Apr 11, 2024

@mlpack-jenkins test this please

2 similar comments
@rcurtin
Copy link
Member Author

rcurtin commented Apr 11, 2024

@mlpack-jenkins test this please

@rcurtin
Copy link
Member Author

rcurtin commented Apr 11, 2024

@mlpack-jenkins test this please

@rcurtin
Copy link
Member Author

rcurtin commented Apr 11, 2024

@mlpack-jenkins test this please

2 similar comments
@rcurtin
Copy link
Member Author

rcurtin commented Apr 15, 2024

@mlpack-jenkins test this please

@rcurtin
Copy link
Member Author

rcurtin commented Apr 15, 2024

@mlpack-jenkins test this please

@rcurtin
Copy link
Member Author

rcurtin commented Apr 16, 2024

I can't believe it! The build succeeded! I thought the day would never come. A wrap-up of what I had to do here:

  • Use precompiled headers for all of the mlpack_test build.
  • To do that, the det_test.cpp file needed to be adapted to not use the heinous #define private public that was being used to test internal members. I can't believe I did that years ago.
  • Disabled precompiled headers for Linux CI builds because they take up too much RAM/disk and cause build failures on Azure Pipelines.
  • Remove cotire, since that support is now in stock CMake.
  • Fix the ambiguous name QuadraticFunction.

shrit
shrit approved these changes Apr 22, 2024
View reviewed changes
.ci/macos-steps.yaml

# Configure mlpack (CMake)
- script: |
unset BOOST_ROOT
Copy link
Member

@shrit shrit Apr 22, 2024

Choose a reason for hiding this comment

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

still here, after all these years

@rcurtin rcurtin merged commit 3607ac8 into mlpack:master Apr 22, 2024
@rcurtin rcurtin deleted the fix-windows-build-the-usual branch April 22, 2024 19:25
This was referenced May 26, 2024
Closed
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@zoq zoq zoq approved these changes

@shrit shrit shrit approved these changes

Assignees

No one assigned

Labels

c: build system t: bugfix

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@rcurtin @zoq @shrit