-
Notifications
You must be signed in to change notification settings - Fork 4.1k
Refresh msvc2017 Dockerfile and add windows workflow #3233
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
base: master
Are you sure you want to change the base?
Conversation
77ec2b9 to
a7e5230
Compare
|
FYI: I am also looking at a Windows Action but taking a bit of a different approach: CJCombrink#7 |
Cool! Your workflow runs much faster :D. The part I like about using docker is that it publishes a thrift-build image that can help folks get started with a ready-to-use environment. Maybe I can rename the workflow to something like msvc.yml. |
3e3c644 to
d22c693
Compare
|
does that include net10? |
I don't think so, I can try to add the support. |
d22c693 to
b74e055
Compare
I'm not so sure if this is really needed, sorry. Not sure what I had in mind when I asked that question. |
|
workflow failed as expected, I think the failed python test is being fixed in #3231 |
Sure it does have some benefit and it builds on top of what is already existing. Although I am not sure who is brave enough to do local docker based development on Windows :P Just regarding the approach: I opted to use conan as a package manager instead of building dependencies manually. I think conan is more inclined to keep dependencies building and up to data compared to trying to maintain basically the same logic here. Conan also provides some pre-build packages that can speed up the build. |
| ADD https://boost.teeks99.com/bin/1.69.0/boost_1_69_0-msvc-14.1-64.exe C:\TEMP\boost.exe | ||
| RUN C:\TEMP\boost.exe /DIR="C:\Libraries\boost_1_69_0" /SILENT && ` | ||
| DEL C:\TEMP\boost.exe | ||
| ADD https://boost.teeks99.com/bin/1.88.0/boost_1_88_0-msvc-14.1-64.exe C:\TEMP\boost.exe |
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 can only recommend adding --checksum checking either to the ADD or do it manually to add some layer of protection and verification.
I totally agree using Conan or vcpkg would be much better. I can switch the build script to use Conan after your change got merged. Just curious, what is wrong about local docker based development on Windows? 🤣 |
Refresh msvc2017 Dockerfile and add windows workflow
[skip ci]anywhere in the commit message to free up build resources.