Skip to content

Disable Windows tests using legacy build systems if they are not supported by the tested branch #219

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

Merged
merged 5 commits into from
Sep 16, 2025
Merged

Disable Windows tests using legacy build systems if they are not supported by the tested branch #219

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
37a3f76
Track supported VS versions in BranchInfo
bensze01 Sep 11, 2025
bd68ee8
Detect support for legacy shipped build systems
bensze01 Sep 11, 2025
06b4367
Return empty job list instead of asserting
bensze01 Sep 12, 2025
5342e0f
Avoid unnecessary Map copies when generating jobs
bensze01 Sep 12, 2025
f9af8e2
Detect support for shipped VS solution files
bensze01 Sep 15, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 8 additions & 0 deletions src/org/mbed/tls/jenkins/BranchInfo.groovy
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,12 @@ class BranchInfo {
* done. */
public String python_requirements_override_file

/** All of the Visual Studio versions supported by this branch */
public List<String> supported_vs_versions

/** Whether this branch supports builds using legacy, non-CMake build systems */
boolean supports_legacy_build_systems

/** Keep track of builds that fail */
final Set<String> failed_builds
final Set<String> outcome_stashes
Expand All @@ -66,6 +72,8 @@ class BranchInfo {
this.has_min_requirements = false
this.python_requirements_override_content = ''
this.python_requirements_override_file = ''
this.supported_vs_versions = []
this.supports_legacy_build_systems = false
this.failed_builds = []
this.outcome_stashes = []
this.coverage_details = 'Code coverage job did not run'
Expand Down
16 changes: 7 additions & 9 deletions vars/common.groovy
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -271,6 +271,13 @@ List<BranchInfo> get_branch_information(Collection<String> tls_branches, Collect
info.python_requirements_override_file = 'override.requirements.txt'
}
}

/* At the time of writing, all supported branches (3.6, development)
* advertise support for Visual Studio 2017 (VS 15.0) and above. */
info.supported_vs_versions = ['2017']

// Detect support for legacy build systems (< Mbed TLS 4.0)
info.supports_legacy_build_systems = fileExists('Makefile')
Copy link
Contributor

Choose a reason for hiding this comment

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

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.

}

String platform = linux_platforms[0]
Expand Down Expand Up @@ -381,15 +388,6 @@ void check_every_all_sh_component_will_be_run(Collection<BranchInfo> infos) {
}
}

def get_supported_windows_builds() {
def vs_builds = []
/* At the time of writing, all supported branches (3.6, development)
* advertise support for Visual Studio 2017 (VS 15.0) and above. */
vs_builds = ['2017']
echo "vs_builds = ${vs_builds}"
return ['mingw'] + vs_builds
}

/* In the PR job (recognized because we set the BRANCH_NAME environment
* variable), report an additional context to GitHub.
* Do nothing from a job that isn't triggered from GitHub.
Expand Down
8 changes: 6 additions & 2 deletions vars/gen_jobs.groovy
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -321,14 +321,15 @@ def gen_windows_testing_job(BranchInfo info, String toolchain) {
def prefix = "${info.prefix}Windows-${toolchain}"
def build_configs, arches, build_systems, retargeted
if (toolchain == 'mingw') {
assert info.supports_legacy_build_systems
Copy link
Contributor

Choose a reason for hiding this comment

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

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?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I can just return an empty map, if you'd prefer that instead of asserting on a "disallowed" case.

Copy link
Contributor Author

@bensze01 bensze01 Sep 11, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

It would also remove the need for the conditionals in gen_windows_jobs()

Copy link
Contributor

Choose a reason for hiding this comment

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

Yea that could be easier to maintain

build_configs = ['mingw']
arches = ['x64']
build_systems = ['shipped']
retargeted = [false]
} else {
build_configs = ['Release', 'Debug']
arches = ['Win32', 'x64']
build_systems = ['shipped', 'cmake']
build_systems = info.supports_legacy_build_systems ? ['shipped', 'cmake'] : ['cmake']
retargeted = [false, true]
}

Expand Down Expand Up @@ -446,9 +447,12 @@ def gen_windows_jobs(BranchInfo info) {
info, info.prefix + 'win32-msvc15_64',
preamble + scripts.win32_msvc15_64_test_bat
)
for (build in common.get_supported_windows_builds()) {
for (build in info.supported_vs_versions) {
jobs = jobs + gen_windows_testing_job(info, build)
}
if (info.supports_legacy_build_systems) {
jobs = jobs + gen_windows_testing_job(info, 'mingw')
}
return jobs
}

Expand Down