Disable Windows tests using legacy build systems if they are not supported by the tested branch #219
Conversation
Signed-off-by: Bence Szépkúti <bence.szepkuti@arm.com>
Disable windows test using the legacy build systems if they are not supported. Signed-off-by: Bence Szépkúti <bence.szepkuti@arm.com>
github-project-automation
bot
moved this to In Development
in Roadmap pull requests (new board)
bensze01
added
needs: reviewer
size-xs
vars/gen_jobs.groovy
Outdated
| def prefix = "${info.prefix}Windows-${toolchain}" | ||
| def build_configs, arches, build_systems, retargeted | ||
| if (toolchain == 'mingw') { | ||
| assert info.supports_legacy_build_systems |
There was a problem hiding this comment.
Any reason to explicitly break the flow of execution? Or is that because we need to return something from gen_windows_testing_job and cannot easily fail early wihtout adding the job?
There was a problem hiding this comment.
I can just return an empty map, if you'd prefer that instead of asserting on a "disallowed" case.
There was a problem hiding this comment.
It would also remove the need for the conditionals in gen_windows_jobs()
There was a problem hiding this comment.
Yea that could be easier to maintain
This allows us to only check whether we support legacy build systems / shipped build scripts in one place. Signed-off-by: Bence Szépkúti <bence.szepkuti@arm.com>
Use the "<<" operator (== Map.putAll()) instead of "+". Signed-off-by: Bence Szépkúti <bence.szepkuti@arm.com>
vars/common.groovy
Outdated
| info.supported_vs_versions = ['2017'] | ||
|
|
||
| // Detect support for legacy build systems (< Mbed TLS 4.0) | ||
| info.supports_legacy_build_systems = fileExists('Makefile') |
There was a problem hiding this comment.
In this PR we are removing the windows jobs that uses the makefile and MS Visual Studio files to build the library and the tests. Thus we need to remove the generation of MS Visual Studio files in mbedtls. This is done now in Mbed-TLS/mbedtls#10382. I think it would then be better to check here for the presence of the directory visualc/VS2017 directory to check if we run or not the jobs using the MS Visual Studio files.
This separates their detection from shipped Makefiles. Signed-off-by: Bence Szépkúti <bence.szepkuti@arm.com>
There was a problem hiding this comment.
LGTM. Before to merge it as 5342e0f affects more than just the Windows jobs we may want to run all tests. Wdyt?
github-project-automation
bot
moved this from In Development
to Has Approval
in Roadmap pull requests (new board)
|
5342e0f is a uniform change, and the windows test paths do touch code that is changed the same way as the non-windows parts, so I'm comfortable with merging this change without further non-windows testing. |
github-project-automation
bot
moved this from Has Approval
to Done
in Roadmap pull requests (new board)
Fixes Mbed-TLS/mbedtls#10144
This PR avoids a larger refactor of
gen_windows_testing_job()to make reviewing easier.I've also prepared a branch where this PR is rebased on-top of #218, and used this PR for the new-ci tests.
The
developmentruns below detect that it supports the legacy build systems. This is indended, as the support is dropped in Mbed-TLS/mbedtls#10382, which depends on this PR.CI runs: