Fix #10952 #10953 - Fixup tcl-tk issue on mac + add ubuntu arm64 runners #10954

jmarrec · 2025-02-25T15:01:55Z

Pull request overview

Fixes TCL Issue on GHA #10952
Fixes Add Ubuntu arm64 workflows #10953

Description of the purpose of this PR

I updated by jmarrec/python-versions repo
- rebased onto main
- adding runners for the new ubuntu-2x.04-arm GHA hosted runners that are now in public preview
- couple of tweaks to make the mac python be built specifically against brew --prefix tcl-tk@8 (important the @8 here),
  - On mac, homebrew uses tcl-tk 9 now, which is NOT backward compat
  - The old python binaries were linking to brew --prefix tcl-tk, which used to be tcl tk 8.6, but now is 9. So it goes awfully.
  - The GHA team "fixed" it by adding a force symlink from brew --prefix tcl-tk@8 to /usr/local/lib/libtcl8.6.dylib. On mac arm64, brew --prefix is NOT /usr/local, but /opt/homebrew, so it can't work for us
    - macos-13 Python can't find Tcl/Tk 8.6 shared libraries actions/runner-images#11074
- https://github.com/jmarrec/python-versions/releases/tag/3.12.2-13521204118
- Force build macos rebase - Feb 25 jmarrec/python-versions#4
In jmarrec/setup-python (the action), I created a tag v5.4.0 that points to that new release of python 3.12.2. I might update v5 tag to point to that v5.4.0, but haven't done it yet
Added ubuntu 22.04 and 24.04 arm64 runners
I had to update my jmarrec/setup-qtifw action to handle the arm64 lin,ux dependencies that were missing
- https://github.com/jmarrec/setup-qtifw/releases/tag/v1.3.0
Reverted some of the shenanigans @Myoldmopar had to do to release IOFreeze

Test release on my fork at https://github.com/jmarrec/EnergyPlus/releases/tag/v25.1.0-IOFreeze-Ubuntu-arm64-macfixup

Pull Request Author

Reviewer

…dds linux arm64 packages

I kept the "any_missing" stuff in there though. Not sure if that was wise.

…lain if something is missing when it isn't a `@path` and should be an actual file

Myoldmopar · 2025-02-25T15:06:44Z

Reverted some of the shenanigans @Myoldmopar had to do to release IOFreeze

Shenanigans!??? Don't you mean incredibly brave and clever workarounds?

jmarrec · 2025-02-25T15:02:43Z

.github/workflows/release_mac.yml

+    inputs:
+      BUILD_DOCS:
+        description: 'Whether to build docs or not as it takes 15 minutes to install MacTex'
+        required: true
+        type: boolean
+        default: false


Bonus: on workflow_dispatch, ask if BUILD_DOCS should be enabled or not. Avoids having to tweak the workflow file to turn it off.

Beautiful. Should we do this on all of them? I could imagine setting up a very generic build workflow where you provide a bunch of switches or CMake args to configure basically the whole build like a normal local setup. That way we could quickly set a specific configuration with caching/python linking/etc. Anyway, this is great.

jmarrec · 2025-02-25T15:03:13Z

.github/workflows/release_mac.yml

+        macos_dev_target: [12.1, 13.0]
        include:
-#        - macos_dev_target: 12.1
-#          os: macos-12
-#          allow_failure: false
-#          arch: x86_64
-#          python-arch: x64
-#          pretty: "Mac x64"
+        - macos_dev_target: 12.1
+          os: macos-13
+          allow_failure: false
+          arch: x86_64
+          python-arch: x64
+          pretty: "Mac x64"


Revert the commenting out from @Myoldmopar . hopefully I caught everything that needed reverting...

I'm sure it's fine.

jmarrec · 2025-02-25T15:03:24Z

.github/workflows/release_mac.yml

    - name: Set up Python ${{ env.Python_REQUIRED_VERSION }}
      id: setup-python
-      uses: jmarrec/setup-python@v5
+      uses: jmarrec/setup-python@v5.4.0


Using my new python binaries

You are a great hero in properly building out the Python versions we need. It's annoying that we do need them, but I appreciate you handling that.

jmarrec · 2025-02-25T15:03:43Z

.github/workflows/release_mac.yml

+        if [[ "${{ github.event_name }}" == "workflow_dispatch" ]]; then
+          BUILD_DOCS="${{ inputs.BUILD_DOCS }}"
+          echo "BUILD_DOCS=$BUILD_DOCS" >> $GITHUB_ENV
+        fi


Overwrite env.BUILD_DOCS when workflow_dispatch

jmarrec · 2025-02-25T15:04:20Z

.github/workflows/release_linux.yml

  push:
    tags:
      - '*'
+  workflow_dispatch:


Added workflow_dispatch events for linux and windows too. Useful when testing instead of having to tag a release.

That's really great, but how does that work with the upload-release-action? That's why I hadn't done this in the past, I assumed it would require some logic down in the workflow to handle whether it was a release or just an artifact.

jmarrec · 2025-02-25T15:07:12Z

.github/workflows/release_linux.yml

      with:
        name: energyplus-${{ matrix.os }}
-        path: build/EnergyPlus-*-x86_64.tar.gz
+        path: build/EnergyPlus-*-${{ matrix.arch }}.tar.gz


jmarrec · 2025-02-25T15:07:22Z

.github/workflows/release_linux.yml

+          - os: ubuntu-22.04-arm
+            test_key: ubuntu2204-arm64
+          - os: ubuntu-24.04-arm
+            test_key: ubuntu2404-arm64


Test the arm ones too

jmarrec · 2025-02-25T15:08:16Z

.github/workflows/release_mac.yml

-#          python-arch: x64
-#          pretty: "Mac x64"
+        - macos_dev_target: 12.1
+          os: macos-13


macos-12 is bye bye. macos-13 is the last x86_64 machine available for macOS

👍 And what's the EOL for macos-13 I wonder. Will they keep it around a while to support an x86_64 Mac?

Anyway, good stuff.

jmarrec · 2025-02-25T15:08:38Z

cmake/PythonFixUpTclTk.py

-    # as a very quick test, I wonder what happens if we just let it continue without this dep...I'll force fail the workflow as a reminder
-    sys.exit(0)
-


jmarrec · 2025-02-25T15:08:54Z

scripts/package_tests/runner.py

+    'ubuntu2204-arm64': {
+        'os': OS.Linux, 'bitness': 'arm64', 'asset_pattern': 'Linux-Ubuntu22.04-arm64.tar.gz', 'os_version': '22.04'
+    },
+    'ubuntu2404-arm64': {
+        'os': OS.Linux, 'bitness': 'arm64', 'asset_pattern': 'Linux-Ubuntu24.04-arm64.tar.gz', 'os_version': '24.04'
+    },


New test keys.

jmarrec · 2025-02-25T15:15:49Z

Before fix: with v25.1.0-IOFreeze

aria2c https://github.com/NREL/EnergyPlus/releases/download/v25.1.0-IOFreeze/EnergyPlus-25.1.0-46864d6807-Darwin-macOS13-arm64.tar.gz
sudo xattr -d com.apple.quarantine EnergyPlus-25.1.0-46864d6807-Darwin-macOS13-arm64.tar.gz
tar xfz EnergyPlus-25.1.0-46864d6807-Darwin-macOS13-arm64.tar.gz

./EnergyPlus-25.1.0-46864d6807-Darwin-macOS13-arm64/energyplus auxiliary eplaunch
Traceback (most recent call last):
  File "<string>", line 8, in <module>
  File "/Users/julien/Downloads/temp/python/EnergyPlus-25.1.0-46864d6807-Darwin-macOS13-arm64/python_lib/eplaunch/tk_runner.py", line 1, in <module>
    from eplaunch.interface.frame_main import EPLaunchWindow
  File "/Users/julien/Downloads/temp/python/EnergyPlus-25.1.0-46864d6807-Darwin-macOS13-arm64/python_lib/eplaunch/interface/__init__.py", line 4, in <module>
    from tkinter import PhotoImage, Tk, Toplevel
  File "/Users/julien/Downloads/temp/python/EnergyPlus-25.1.0-46864d6807-Darwin-macOS13-arm64/python_lib/tkinter/__init__.py", line 38, in <module>
    import _tkinter # If this fails your Python may not be configured for Tk
    ^^^^^^^^^^^^^^^
ImportError: dlopen(/Users/julien/Downloads/temp/python/EnergyPlus-25.1.0-46864d6807-Darwin-macOS13-arm64/python_lib/lib-dynload/_tkinter.cpython-312-darwin.so, 0x0002): Library not loaded: /opt/homebrew/opt/tcl-tk/lib/libtcl8.6.dylib
  Referenced from: <C66EF5E9-817E-3F86-B278-F1BA42A0A80D> /Users/julien/Downloads/temp/python/EnergyPlus-25.1.0-46864d6807-Darwin-macOS13-arm64/python_lib/lib-dynload/_tkinter.cpython-312-darwin.so
  Reason: tried: '/opt/homebrew/opt/tcl-tk/lib/libtcl8.6.dylib' (no such file), '/System/Volumes/Preboot/Cryptexes/OS/opt/homebrew/opt/tcl-tk/lib/libtcl8.6.dylib' (no such file), '/opt/homebrew/opt/tcl-tk/lib/libtcl8.6.dylib' (no such file), '/opt/homebrew/Cellar/tcl-tk/9.0.1/lib/libtcl8.6.dylib' (no such file), '/System/Volumes/Preboot/Cryptexes/OS/opt/homebrew/Cellar/tcl-tk/9.0.1/lib/libtcl8.6.dylib' (no such file), '/opt/homebrew/Cellar/tcl-tk/9.0.1/lib/libtcl8.6.dylib' (no such file)
libc++abi: terminating due to uncaught exception of type std::runtime_error: Error executing Python code
Abort trap: 6

After fix: on my release

aria2c https://github.com/jmarrec/EnergyPlus/releases/download/v25.1.0-IOFreeze-Ubuntu-arm64-macfixup/EnergyPlus-25.1.0-ab8eee336c-Darwin-macOS13-arm64.tar.gz
sudo xattr -d com.apple.quarantine EnergyPlus-25.1.0-ab8eee336c-Darwin-macOS13-arm64.tar.gz

./EnergyPlus-25.1.0-ab8eee336c-Darwin-macOS13-arm64/energyplus auxiliary eplaunch

Myoldmopar

This is awesome. Thank you @jmarrec!

Myoldmopar · 2025-02-26T14:45:01Z

.github/workflows/release_linux.yml

  push:
    tags:
      - '*'
+  workflow_dispatch:


That's really great, but how does that work with the upload-release-action? That's why I hadn't done this in the past, I assumed it would require some logic down in the workflow to handle whether it was a release or just an artifact.

Myoldmopar · 2025-02-26T15:19:57Z

.github/workflows/release_linux.yml

+        - os: ubuntu-22.04
+          pretty: "Ubuntu 22.04"
+          arch: x86_64
+          python-arch: x64
+        - os: ubuntu-24.04
+          pretty: "Ubuntu 24.04"
+          arch: x86_64
+          python-arch: x64
+        - os: ubuntu-22.04-arm
+          pretty: "Ubuntu 22.04 arm64"
+          arch: arm64
+          python-arch: arm64
+        - os: ubuntu-24.04-arm
+          pretty: "Ubuntu 24.04 arm64"
+          arch: arm64
+          python-arch: arm64


Got it. This is good stuff. I was kinda surprised to see 22.04 added in here, but hey it's fine for a while.

Myoldmopar · 2025-02-26T15:20:24Z

.github/workflows/release_linux.yml

    - name: Set up Python ${{ env.Python_REQUIRED_VERSION }}
      id: setup-python
-      uses: jmarrec/setup-python@v5
+      uses: jmarrec/setup-python@v5.4.0


Yes, very good. Very good.

Myoldmopar · 2025-02-26T15:21:25Z

.github/workflows/release_linux.yml

      uses: jmarrec/setup-qtifw@v1
      with:
-        qtifw-version: '4.6.1'
+        qtifw-version: '4.8.1'


Got it. And we are using your fork because they didn't take your PR to pass arguments down into binarycreator, right? Or something like that? Over time it would be really great to get back to mainline actions so we don't have to rely on custom built ones, but I do appreciate your patches to them.

Myoldmopar · 2025-02-26T15:27:39Z

.github/workflows/release_mac.yml

+    inputs:
+      BUILD_DOCS:
+        description: 'Whether to build docs or not as it takes 15 minutes to install MacTex'
+        required: true
+        type: boolean
+        default: false


Beautiful. Should we do this on all of them? I could imagine setting up a very generic build workflow where you provide a bunch of switches or CMake args to configure basically the whole build like a normal local setup. That way we could quickly set a specific configuration with caching/python linking/etc. Anyway, this is great.

Myoldmopar · 2025-02-26T15:29:41Z

.github/workflows/release_mac.yml

+        macos_dev_target: [12.1, 13.0]
        include:
-#        - macos_dev_target: 12.1
-#          os: macos-12
-#          allow_failure: false
-#          arch: x86_64
-#          python-arch: x64
-#          pretty: "Mac x64"
+        - macos_dev_target: 12.1
+          os: macos-13
+          allow_failure: false
+          arch: x86_64
+          python-arch: x64
+          pretty: "Mac x64"


I'm sure it's fine.

Myoldmopar · 2025-02-26T15:30:26Z

.github/workflows/release_mac.yml

    - name: Set up Python ${{ env.Python_REQUIRED_VERSION }}
      id: setup-python
-      uses: jmarrec/setup-python@v5
+      uses: jmarrec/setup-python@v5.4.0


You are a great hero in properly building out the Python versions we need. It's annoying that we do need them, but I appreciate you handling that.

Myoldmopar · 2025-02-26T15:33:16Z

.github/workflows/release_windows.yml

      uses: jmarrec/setup-qtifw@v1
      with:
-        qtifw-version: '4.6.1'
+        qtifw-version: '4.8.1'


Interesting. Looks like it's in the April-June roadmap on GitHub Action's public roadmap. (github/roadmap#1098). So we shouldn't be waiting long.

Myoldmopar · 2025-02-26T15:33:49Z

cmake/PythonFixUpTclTk.py

-    sos = list(_python_dir.glob("lib-dynload/_tkinter*.so"))
+def locate_tk_so(python_dir: Path) -> Path:
+    print(f"Searching for _tkinter so in {python_dir}")
+    sos = list(python_dir.glob("lib-dynload/_tkinter*.so"))


My renaming here was just to avoid variable shadowing from the outer scope. But that's fine, it can go back to shadowing.

Myoldmopar · 2025-02-26T15:34:00Z

scripts/package_tests/runner.py

+    'ubuntu2204-arm64': {
+        'os': OS.Linux, 'bitness': 'arm64', 'asset_pattern': 'Linux-Ubuntu22.04-arm64.tar.gz', 'os_version': '22.04'
+    },
+    'ubuntu2404-arm64': {
+        'os': OS.Linux, 'bitness': 'arm64', 'asset_pattern': 'Linux-Ubuntu24.04-arm64.tar.gz', 'os_version': '24.04'
+    },


Myoldmopar · 2025-02-26T15:34:52Z

No reason to wait on this, thanks for fixing this all up @jmarrec. Merging.

jmarrec added 7 commits February 25, 2025 13:59

Add ubuntu arm64 runners

f132bc2

Update to my new setup-python action that fixes up macos tcl-tk and a…

b81f5a5

…dds linux arm64 packages

Revert @Myoldmopar shenanigans during release

f6d2c62

I kept the "any_missing" stuff in there though. Not sure if that was wise.

I think the shenanigans weren't correct, attempt to fix it: only comp…

67d7839

…lain if something is missing when it isn't a `@path` and should be an actual file

Add a workflow dispatch for linux and windows ones

b7697bb

on mac workflow_dispatch, ask if BUILD_DOCS

3896da7

Bump qtifw to 4.8.1, so there's a linux arm installer

ab8eee3

jmarrec added Defect Includes code to repair a defect in EnergyPlus AuxiliaryTool Related to an auxiliary tool, not EnergyPlus itself (readvars, preprocessor, ep-launch, etc.) Developer Issue Related to cmake, packaging, installers, or developer tooling (CI, etc) labels Feb 25, 2025

jmarrec self-assigned this Feb 25, 2025

jmarrec commented Feb 25, 2025

View reviewed changes

jmarrec requested a review from Myoldmopar February 25, 2025 18:56

Myoldmopar approved these changes Feb 26, 2025

View reviewed changes

Myoldmopar merged commit a25ffdb into NREL:develop Feb 26, 2025
12 checks passed

		# as a very quick test, I wonder what happens if we just let it continue without this dep...I'll force fail the workflow as a reminder
		sys.exit(0)

Fix #10952 #10953 - Fixup tcl-tk issue on mac + add ubuntu arm64 runners #10954

Fix #10952 #10953 - Fixup tcl-tk issue on mac + add ubuntu arm64 runners #10954

Uh oh!

Conversation

jmarrec commented Feb 25, 2025

Pull request overview

Description of the purpose of this PR

Pull Request Author

Reviewer

Uh oh!

Myoldmopar commented Feb 25, 2025

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!

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!

jmarrec commented Feb 25, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Before fix: with v25.1.0-IOFreeze

After fix: on my release

Uh oh!

Myoldmopar left a comment

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!

Choose a reason for hiding this comment

Uh oh!

Myoldmopar commented Feb 26, 2025

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

jmarrec commented Feb 25, 2025 •

edited

Loading