-
Notifications
You must be signed in to change notification settings - Fork 56
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
Add CI framework for Windows 64 #229
Add CI framework for Windows 64 #229
Conversation
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.
overall, looks good to me, but I'll caveat that by saying that I know very little about windows scripting, package managers, etc. and have never used windows on Docker. so, my feedback may not be the most useful...
given that ACCP won't yet build on windows, is there anything we can do to test these changes to ensure at least the docker images are building properly and the expected commands are being invoked? perhaps we can show the output of the failed ACCP gradle build when :build_and_test
is run?
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've verified the Windows docker image building portion works, along with the CI set up. I've attached a file which shows the docker image building output, along with a screenshot of the images living in the appropriate ECR. Along with that, the CI changes are tested in this local fork PR: samuel40791765#15
Running a test command with the script in the docker image outputs a gradle error during linking MSVC with AWS-LC. This would sound like one of the tasks to fix for ACCP Windows support, so I've stopped here.
C:\codebuild\tmp\output\src862177151\src\github.com\samuel40791765\amazon-corretto-crypto-provider>gradlew.bat release "-DTEST_JAVA_MAJOR_VERSION=17" \|\| goto error
--
135 | Downloading https://services.gradle.org/distributions/gradle-7.2-bin.zip
136 | ..........10%...........20%...........30%...........40%...........50%...........60%...........70%...........80%...........90%...........100%
137 |
138 | Welcome to Gradle 7.2!
139 |
140 | Here are the highlights of this release:
141 | - Toolchain support for Scala
142 | - More cache hits when Java source files have platform-specific line endings
143 | - More resilient remote HTTP build cache behavior
144 |
145 | For more details see https://docs.gradle.org/7.2/release-notes.html
146 |
147 | Starting a Gradle Daemon (subsequent builds will be faster)
148 | > Task :assemble UP-TO-DATE
149 |
150 | > Task :buildAwsLc
151 | -- Building for: Visual Studio 15 2017
152 | CMake Error at CMakeLists.txt:4 (project):
153 | Failed to run MSBuild command:
154 | -- Configuring incomplete, errors occurred!
155 |
156 | See also "C:/codebuild/tmp/output/src862177151/src/github.com/samuel40791765/amazon-corretto-crypto-provider/build/awslc/build/CMakeFiles/CMakeOutput.log".
157 | C:/Program Files (x86)/Microsoft Visual Studio/2017/BuildTools/MSBuild/15.0/Bin/MSBuild.exe
158 |
159 | to get the value of VCTargetsPath:
160 |
161 | Microsoft (R) Build Engine version 15.9.21+g9802d43bc3 for .NET Framework
162 | Copyright (C) Microsoft Corporation. All rights reserved.
163 |
164 | Build started 7/9/2022 4:10:51 AM.
165 | Project "C:\codebuild\tmp\output\src862177151\src\github.com\samuel40791765\amazon-corretto-crypto-provider\build\awslc\build\CMakeFiles\3.15.4\VCTargetsPath.vcxproj" on node 1 (default targets).
166 | C:\codebuild\tmp\output\src862177151\src\github.com\samuel40791765\amazon-corretto-crypto-provider\build\awslc\build\CMakeFiles\3.15.4\VCTargetsPath.vcxproj(15,2): error MSB4019: The imported project "C:\Program Files (x86)\Microsoft Visual Studio\2017\BuildTools\Common7\IDE\VC\VCTargets\Microsoft.Cpp.Default.props" was not found. Confirm that the path in the <Import> declaration is correct, and that the file exists on disk.
167 | Done Building Project "C:\codebuild\tmp\output\src862177151\src\github.com\samuel40791765\amazon-corretto-crypto-provider\build\awslc\build\CMakeFiles\3.15.4\VCTargetsPath.vcxproj" (default targets) -- FAILED.
168 |
169 | Build FAILED.
170 |
171 | "C:\codebuild\tmp\output\src862177151\src\github.com\samuel40791765\amazon-corretto-crypto-provider\build\awslc\build\CMakeFiles\3.15.4\VCTargetsPath.vcxproj" (default target) (1) ->
172 | C:\codebuild\tmp\output\src862177151\src\github.com\samuel40791765\amazon-corretto-crypto-provider\build\awslc\build\CMakeFiles\3.15.4\VCTargetsPath.vcxproj(15,2): error MSB4019: The imported project "C:\Program Files (x86)\Microsoft Visual Studio\2017\BuildTools\Common7\IDE\VC\VCTargets\Microsoft.Cpp.Default.props" was not found. Confirm that the path in the <Import> declaration is correct, and that the file exists on disk.
173 |
174 | 0 Warning(s)
175 | 1 Error(s)
176 |
177 | Time Elapsed 00:00:00.40
178 |
179 |
180 | Exit code: 1
181 |
182 |
183 |
184 |
185 | > Task :buildAwsLc FAILED
186 |
187 | FAILURE: Build failed with an exception.
188 |
189 | * Where:
190 | Build file 'C:\codebuild\tmp\output\src862177151\src\github.com\samuel40791765\amazon-corretto-crypto-provider\build.gradle' line: 75
191 |
192 | * What went wrong:
193 | Execution failed for task ':buildAwsLc'.
194 | > Process 'command 'cmake'' finished with non-zero exit value 1
Docker image building: nohup.txt
ECR:
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.
looks great, thanks sam!
Though official support for Windows (64-bit) is blocked, we can still add CI support for it. Once the Windows CI framework is added to ACCP, we only have to reconfigure the `tests/ci/run_windows_tests.bat` test script for ACCP windows testing, then enable the CI support.
Though official support for Windows (64-bit) is blocked, we can still add CI support for it. Once the Windows CI framework is added to ACCP, we only have to reconfigure the `tests/ci/run_windows_tests.bat` test script for ACCP windows testing, then enable the CI support.
Issue #, if available:
CryptoAlg-1104
Description of changes:
Though official support for Windows (64-bit) is blocked on #48, we can still CI support for it.
Once the Windows CI framework is added to ACCP, we only have to reconfigure the
tests/ci/run_windows_tests.bat
test script for ACCP windows testing, then enable the CI support.By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.