Fix #11161 - GHA: Windows Server 2019 has been retired. Use 2022 by jmarrec · Pull Request #11162 · NatLabRockies/EnergyPlus

jmarrec · 2025-08-22T10:33:15Z

Pull request overview

Fix Cannot build Windows Release due to removal of windows-2019 from GHA #11161
Updates workflow to use 2022
Updates package_testing to use strongly type Enums and avoid the whole confusion between passing '17' or '2022' for MSVC versions.

Description of the purpose of this PR

Pull Request Author

Title of PR should be user-synopsis style (clearly understandable in a standalone changelog context)
Label the PR with at least one of: Defect, Refactoring, NewFeature, Performance, and/or DoNoPublish

Reviewer

Windows Server 2019 has been retired. The Windows Server 2019 image has been removed as of 2025-06-30. For more details, see actions/runner-images#12045

…fy when 7z.exe isn't found

jmarrec · 2025-08-22T10:35:23Z

scripts/package_tests/runner.py

-        if msvc_version in ['16', '17', '18']:
+        if self.os == OS.Windows:
            self.msvc_version = msvc_version
-        elif self.os == OS.Windows and self.os_version == '2022':
-            self.msvc_version = 17
-        elif self.os == OS.Windows:
-            self.msvc_version = 16
-        self.asset_pattern = this_config['asset_pattern']
-        self.bitness = this_config['bitness']


Here the GHA test step was passing --msvc 2022 for win64 for run_config_key. Given this code block, it was just ignored, and it was checking the CONFIGURATIONS, which listed "os_version" as "10" for "win64", so it'd pick Visual Studio 16 2019, which doesn't exist.

jmarrec · 2025-08-22T10:35:40Z

scripts/package_tests/ep_testing/constants.py

+from enum import IntEnum, StrEnum
+
+
+class OS(StrEnum):
+    """Operating Systems."""
+
+    Windows = "Windows"
+    Linux = "Linux"
+    Mac = "Mac"
+
+
+class MSVC(IntEnum):
+    """Microsoft Visual Studio versions."""
+
+    V16 = 16
+    V17 = 17
+    # V18 = 18  # Future version placeholder
+
+    def generator_name(self) -> str:
+        """Get the CMake generator name for the MSVC version."""
+        return MSVC_GENERATOR_MAPPING[self]
+
+
+# Alias mapping for common version names to MSVC enum, for argparse
+MSVC_ALIAS_MAPPING = {
+    "2017": MSVC.V16,
+    "2022": MSVC.V17,
+}
+
+# Mapping from MSVC enum to CMake generator names
+MSVC_GENERATOR_MAPPING = {
+    MSVC.V16: "Visual Studio 16 2019",
+    MSVC.V17: "Visual Studio 17 2022",
+}
+
+
+class Bitness(StrEnum):
+    X86 = "x86"
+    X64 = "x64"
+    ARM64 = "arm64"


Hello Enums.

jmarrec · 2025-08-22T10:36:49Z

scripts/package_tests/runner.py

+def handle_msvc_arg(arg: str) -> MSVC:
+    # Check for aliases first
+    if arg in MSVC_ALIAS_MAPPING:
+        return MSVC_ALIAS_MAPPING[arg]
+
+    try:
+        return MSVC(int(arg))
+    except ValueError:
+        possible_values = list(map(str, MSVC))
+        aliases = [f"{k} -> {v}" for k, v in MSVC_ALIAS_MAPPING.items()]
+        msg = f"Invalid MSVC version argument: '{arg}' (choose from {possible_values} or aliases {aliases})"
+        raise argparse.ArgumentTypeError(msg)


Turns the --msvc argument (str) into a MSVC enum instance.

I maintained the backward compat use of aliases, such as '2017' or '2022', but they map to one of MSVC.V16 or MSVC.V17

jmarrec · 2025-08-22T10:37:24Z

scripts/package_tests/runner.py

+    if platform.system() == "Windows" and args.msvc is None:
+        parser.error("On Windows, the --msvc argument is required to specify the Visual Studio version.")


I'm enforcing that the msvc argument is actually passed. Makes more sense instead of trying to default it for whatever reason.

jmarrec · 2025-08-22T10:38:18Z

scripts/package_tests/runner.py

            print(" ...Extraction Complete")
        except CalledProcessError as e:
-            raise Exception("Extraction failed with this error: " + str(e))
+            raise Exception(f"Extraction failed with this error: {e}\nCommand: {extract_command}")


My local VM had 7zip installed, by it wasn't on PATH. Wasn't clear what was happening.

jmarrec · 2025-08-22T10:38:47Z

scripts/package_tests/runner.py

+        if self.os == OS.Windows:
            self.msvc_version = msvc_version
-        elif self.os == OS.Windows and self.os_version == '2022':
-            self.msvc_version = 17
-        elif self.os == OS.Windows:
-            self.msvc_version = 16
-        self.asset_pattern = this_config['asset_pattern']
-        self.bitness = this_config['bitness']
+            if not isinstance(self.msvc_version, MSVC):
+                raise ValueError("On Windows, an MSVC (Enum) version must be provided.")


Enforce having msvc_version as an Enum here.

jmarrec · 2025-08-22T10:39:34Z

scripts/package_tests/runner.py

+        TestCAPIAccess(os=self.os, bitness=self.bitness, msvc_version=self.msvc_version).run(
+            install_path,
+            verbose,


The C API tests take their required params into their Ctor now. Instead of weirdly passing them via kwargs on the "run" method and assigning there

jmarrec · 2025-08-22T10:39:52Z

scripts/package_tests/runner.py

+        if self.bitness == Bitness.X86:
            print("Travis does not have a 32-bit Python package readily available, so not testing Python API")


That shoudn't be there nowadays FYI

jmarrec · 2025-08-22T10:40:50Z

scripts/package_tests/ep_testing/tests/api.py

+    def __init__(self, os: OS, bitness: Bitness, msvc_version: Optional[MSVC] = None):
        super().__init__()
-        self.os = None
-        self.bitness = None
-        self.source_file_name = 'func.cpp'
-        self.target_name = 'TestCAPIAccess'
-        self.msvc_version = None
+        self.os = os
+        self.bitness = bitness
+        self.msvc_version = msvc_version
+        self.source_file_name = "func.cpp"
+        self.target_name = "TestCAPIAccess"
+        if self.os == OS.Windows and self.msvc_version is None:
+            raise Exception("Must pass msvc_version if os is Windows")


Like I said, they assigned in Ctor. And will throw if windows and no msvc_version, since it's required

jmarrec · 2025-08-22T10:43:19Z

scripts/package_tests/ep_testing/tests/api.py

+def make_build_dir_and_build(cmake_build_dir: str, verbose: bool, bitness: Bitness, msvc_version: MSVC):
+
+    os.makedirs(cmake_build_dir)
+    my_env = os.environ.copy()
+    if platform.system() == "Darwin":  # my local comp didn't have cmake in path except in interact shells
+        my_env["PATH"] = "/usr/local/bin:" + my_env["PATH"]
+    command_line = ["cmake", ".."]
+    is_windows = platform.system() == "Windows"
+    if is_windows:
+        command_line.extend(["-G", msvc_version.generator_name(), "-A", bitness.value])

+    try:
        my_check_call(verbose, command_line, cwd=cmake_build_dir, env=my_env)
-        command_line = ['cmake', '--build', '.']
-        if platform.system() == 'Windows':
-            command_line.extend(['--config', 'Release'])
+    except Exception as e:
+        print("C API Wrapper Configuration Failed!")
+        raise e


Couple of things happening here:

One: I try / catch only the check_call. I don't want to swallow other exception. And I clarify the print to let you know it's the CONFIGURATION that failed here (the build later)

Two: The cmake Generator options are computed from the enums, less error prone.

Three: I removed Visual Studio 15. It would have forced me to do more complicated things since it doesn't accept a "-A" argument, and it's too old

Myoldmopar

This is all fine. The ' to " changes make it a little difficult to see the functional changes, so I appreciate you calling the changes out. This has been needed for a while. The 2022 change is great, and the package testing code was written way way back, so it definitely needed some love.

Myoldmopar · 2025-08-26T13:18:13Z

@jmarrec have you built a release from this branch? If not, I'll go ahead and kick one off to check out the release testing updates.

jmarrec · 2025-08-26T13:21:24Z

@Myoldmopar https://github.com/jmarrec/EnergyPlus/actions/runs/17152798112

https://github.com/jmarrec/EnergyPlus/releases/tag/v25.2.0-pre-IOFreeze-1

Myoldmopar · 2025-08-26T13:23:44Z

Perfect, thank you! Let's get this merged now.

jmarrec added 6 commits August 21, 2025 21:06

GHA: Windows Server 2019 has been retired. Use 2022

1f01945

Windows Server 2019 has been retired. The Windows Server 2019 image has been removed as of 2025-06-30. For more details, see actions/runner-images#12045

Missed a few places

c769d6d

black & isort -l 120 before mod

0123afc

use Enums for MSVC versions and Bitness + pass arguments in C API ctor

ed2d5f9

Botched assigning msvc_version, require --msvc arg on windows + clari…

099de54

…fy when 7z.exe isn't found

Damn license check.

ecb5dad

jmarrec self-assigned this Aug 22, 2025

jmarrec added Defect Includes code to repair a defect in EnergyPlus NotIDDChange Code does not impact IDD (can be merged after IO freeze) Developer Issue Related to cmake, packaging, installers, or developer tooling (CI, etc) labels Aug 22, 2025

jmarrec commented Aug 22, 2025

View reviewed changes

Myoldmopar approved these changes Aug 26, 2025

View reviewed changes

Myoldmopar merged commit 64a6fac into develop Aug 26, 2025
12 checks passed

Myoldmopar deleted the 11161_windows_release branch August 26, 2025 13:23

		if platform.system() == "Windows" and args.msvc is None:
		parser.error("On Windows, the --msvc argument is required to specify the Visual Studio version.")

		if self.bitness == Bitness.X86:
		print("Travis does not have a 32-bit Python package readily available, so not testing Python API")

Conversation

jmarrec commented Aug 22, 2025

Pull request overview

Description of the purpose of this PR

Pull Request Author

Reviewer

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Myoldmopar left a comment

Choose a reason for hiding this comment

Uh oh!

Myoldmopar commented Aug 26, 2025

Uh oh!

jmarrec commented Aug 26, 2025

Uh oh!

Myoldmopar commented Aug 26, 2025

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants