Skip to content

feat(ci): add macOS CI workflow - #95

Open
wdconinc wants to merge 54 commits into
masterfrom
macos-ci-workflow
Open

feat(ci): add macOS CI workflow#95
wdconinc wants to merge 54 commits into
masterfrom
macos-ci-workflow

Conversation

@wdconinc

@wdconinc wdconinc commented Dec 18, 2025

Copy link
Copy Markdown
Contributor

This PR adds a GitHub Actions workflow to build the ci, tf, and xl environments on macOS.

Features

  • Matrix strategy testing ci, tf, and xl environments
  • Automatic buildcache push/pull for faster rebuilds

Copilot AI lite review requested due to automatic review settings December 18, 2025 22:30
Comment thread .github/workflows/macos-ci.yml Outdated
Comment thread .github/workflows/macos-ci.yml Outdated
Comment thread .github/workflows/macos-ci.yml Outdated
Comment thread .github/workflows/macos-ci.yml Outdated
Comment thread .github/workflows/macos-ci.yml Outdated

This comment was marked as resolved.

This comment was marked as resolved.

wdconinc and others added 2 commits August 9, 2026 12:47
…s-buildcache

This workflow builds the ci environment on macOS using:
- spack/setup-spack@v2.1.1 for Spack installation
- spack/github-actions-buildcache@v2 for build cache management
- Matrix strategy for both gcc and clang compilers
- Automatic buildcache push/pull for faster rebuilds
…96)

* Initial plan

* Align macOS CI with container build: spack versions, cherry-picks, externals, and buildcache

Co-authored-by: wdconinc <4656391+wdconinc@users.noreply.github.com>

* Derive spack configuration from spack.sh and spack-packages.sh at runtime

Co-authored-by: wdconinc <4656391+wdconinc@users.noreply.github.com>

* Fix cherry-pick loops to filter heredoc delimiters and empty lines

Co-authored-by: wdconinc <4656391+wdconinc@users.noreply.github.com>

* Improve cherry-pick filtering and add proper quoting for security

Co-authored-by: wdconinc <4656391+wdconinc@users.noreply.github.com>

---------

Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com>
Co-authored-by: wdconinc <4656391+wdconinc@users.noreply.github.com>
Copilot AI review requested due to automatic review settings August 9, 2026 17:47
@wdconinc
wdconinc force-pushed the macos-ci-workflow branch from 24f3357 to 61b57b4 Compare August 9, 2026 17:47

This comment was marked as resolved.

Copilot AI review requested due to automatic review settings August 9, 2026 18:01
@wdconinc wdconinc changed the title Add macOS CI workflow feat(ci): add macOS CI workflow Aug 9, 2026

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 1 out of 1 changed files in this pull request and generated no new comments.

Suppressed comments (4)

.github/workflows/macos-ci.yml:86

  • Same issue as above: this grep | while pipeline will fail the step when SPACKPACKAGES_CHERRYPICKS is empty because grep exits 1 under bash -e -o pipefail.
          # Apply cherry-picks from spack-packages.sh (match only valid git commit hashes)
          echo "${{ steps.spack-config.outputs.spackpackages-cherrypicks }}" | grep -E '^[a-f0-9]{40}$' | while read -r commit; do
            echo "Cherry-picking ${commit}"
            git cherry-pick "${commit}"
          done

.github/workflows/macos-ci.yml:71

  • The cherry-pick loop will fail the step when there are no matching commits because grep exits 1 and GitHub Actions runs bash with -e -o pipefail by default. This makes the workflow brittle whenever SPACK_CHERRYPICKS is empty (a valid case in this repo).

This issue also appears on line 82 of the same file.

          # Apply cherry-picks from spack.sh (match only valid git commit hashes)
          echo "${{ steps.spack-config.outputs.spack-cherrypicks }}" | grep -E '^[a-f0-9]{40}$' | while read -r commit; do
            echo "Cherry-picking ${commit}"
            git cherry-pick "${commit}"
          done

.github/workflows/macos-ci.yml:20

  • This workflow requests packages: write but none of the steps push images/packages to GHCR. Using write here unnecessarily broadens the GITHUB_TOKEN scope; packages: read (or omitting it) should be sufficient for pulling the public buildcache.
permissions:
  contents: read
  packages: write

.github/workflows/macos-ci.yml:107

  • The GHCR Spack buildcache is treated as unsigned elsewhere in this repo (e.g., mirrors.yaml.in sets signed: false and the Debian Dockerfile adds it with --unsigned). Here the mirror is added without --unsigned, and the workflow then tries to install/trust buildcache keys (which is typically unnecessary for unsigned mirrors and may fail if no keys exist). Align with the repo’s unsigned mirror configuration and drop the keys step.
          spack env activate -d spack-environment/${{matrix.ENV}}
          spack mirror add eic oci://ghcr.io/eic/spack-${{ steps.spack-config.outputs.buildcache-version }}
          spack buildcache keys --install --trust

Updated the Spack setup process by changing the method of cloning the Spack repository and applying cherry-picks.
Copilot AI review requested due to automatic review settings August 9, 2026 18:06

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 1 out of 1 changed files in this pull request and generated 1 comment.

Suppressed comments (2)

.github/workflows/macos-ci.yml:83

  • spack repo add expects a Spack repository root (with a repo.yaml). The spack-packages checkout is typically added via its builtin repo directory (as done in containers/debian/Dockerfile: .../repos/spack_repo/builtin). Adding the top-level var/spack/repos/spack-packages is likely not a valid repo root and can break package resolution.
          cd ${{ github.workspace }}/spack
          spack repo add var/spack/repos/spack-packages

.github/workflows/macos-ci.yml:100

  • This mirror is being used as a binary buildcache source, but it is added without the buildcache flags/scope used elsewhere in this repo (e.g. containers/debian/Dockerfile adds the GHCR buildcache mirror with --scope spack --unsigned). Without --unsigned/scope, Spack may treat this as a source mirror instead of a buildcache mirror, and spack buildcache keys / installs won’t behave as intended.
          spack env activate -d spack-environment/${{matrix.ENV}}
          spack mirror add eic oci://ghcr.io/eic/spack-${{ steps.spack-config.outputs.buildcache-version }}
          spack buildcache keys --install --trust

Comment thread .github/workflows/macos-ci.yml Outdated
Added environment variables for GitHub registry authentication.
Copilot AI review requested due to automatic review settings August 9, 2026 18:16
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 1 out of 1 changed files in this pull request and generated no new comments.

Suppressed comments (2)

.github/workflows/macos-ci.yml:108

  • spack mirror add is configured with --autopush unconditionally. On pull_request events (especially from forks), registry secrets are not available and the install step can fail when Spack tries to push buildcache artifacts. Consider enabling autopush only when credentials are present and the event is not pull_request, while still allowing pulls.
      - name: Configure buildcache
        run: |
          spack env activate -d spack-environment/${{matrix.ENV}}
          spack mirror add eic \
            --autopush --unsigned \
            --oci-username-variable GITHUB_REGISTRY_USER \
            --oci-password-variable GITHUB_REGISTRY_TOKEN \
            oci://ghcr.io/eic/spack-${{ steps.spack-config.outputs.buildcache-version }}

.github/workflows/macos-ci.yml:55

  • This comment says it’s extracting the buildcache URL from mirrors.yaml.in, but the code just reuses SPACKPACKAGES_VERSION. That’s fine, but the comment is misleading and makes the step harder to maintain.
          # Extract buildcache URL from mirrors.yaml.in (simplified - get the version)
          echo "buildcache-version=${SPACKPACKAGES_VERSION}" >> $GITHUB_OUTPUT

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 5 out of 5 changed files in this pull request and generated no new comments.

Suppressed comments (3)

.github/workflows/macos-ci.yml:99

  • spack-packages is checked out with the default shallow fetch depth. Since the next step cherry-picks specific SHAs, those commits may not exist locally in a shallow clone, causing git merge-base / git cherry-pick to fail. Use fetch-depth: 0 (or explicitly fetch the SHAs) like the Spack checkout step does.
        uses: actions/checkout@v7
        with:
          repository: spack/spack-packages
          path: spack/var/spack/repos/spack-packages
          ref: ${{ steps.spack-config.outputs.spackpackages-version }}

.github/workflows/macos-ci.yml:109

  • In the spack-packages cherry-pick step, only develop is fetched. If any cherry-pick SHA is not already reachable from the checked-out ref, git merge-base --is-ancestor will error (and fail the step under bash -e). Fetch the required commits by SHA from the spackpackages-cherrypicks list before running the merge-base / cherry-pick loop.
          # Fetch branches from which we cherry-pick
          git fetch origin refs/heads/develop:refs/heads/develop

.github/workflows/macos-ci.yml:83

  • The workflow hard-codes fetching two specific PR refs before cherry-picking, which is brittle and can break as soon as spack.sh updates to cherry-pick commits from different refs. Also, if a cherry-pick SHA isn’t present locally, git merge-base --is-ancestor will error (and the step will fail under the default bash -e). Fetch the required commit objects by SHA from the spack-cherrypicks list instead of pinning PR numbers.

This issue also appears in the following locations of the same file:

  • line 95
  • line 107
          # Fetch branches from which we cherry-pick
          git fetch origin refs/heads/develop:refs/heads/develop
          git fetch origin pull/51584/head:pr-51584
          git fetch origin pull/52207/head:pr-52207

Removed a cherry-pick reference for a specific commit and updated the comments for clarity.
Updated cherry-pick references in spack-packages.sh.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 5 out of 5 changed files in this pull request and generated no new comments.

Suppressed comments (1)

.github/workflows/macos-ci.yml:76

  • The workflow runs multiple spack commands that write user config (e.g. mirror add, compiler find) but doesn’t create ~/.spack first. This repo already carries a workaround for Spack occasionally failing to create that directory automatically (see containers/debian/Dockerfile), so this workflow may fail intermittently on fresh runners.

Create the directory before invoking spack to make the job more reliable.

          cd ${{ github.workspace }}/spack
          echo "${{ github.workspace }}/spack/bin" >> $GITHUB_PATH
          git config user.name "GitHub Actions"
          git config user.email "actions@github.com"

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 5 out of 5 changed files in this pull request and generated no new comments.

Suppressed comments (3)

.github/workflows/macos-ci.yml:83

  • The cherry-pick logic can fail the job if a listed commit SHA isn’t present in the current checkout. git merge-base --is-ancestor <sha> HEAD exits non-zero (and errors) when the object isn’t available locally, and the workflow currently relies on a couple of hard-coded PR refs that won’t cover future cherry-picks. Fetch the commit by SHA when missing and guard the ancestor check so the step remains robust as spack.sh changes.
          # Fetch branches from which we cherry-pick
          git fetch origin refs/heads/develop:refs/heads/develop
          git fetch origin pull/51584/head:pr-51584
          git fetch origin pull/52207/head:pr-52207

.github/workflows/macos-ci.yml:111

  • Same issue as the Spack cherry-picks: git merge-base --is-ancestor will fail the step if a commit SHA isn’t present locally. Since SPACKPACKAGES_CHERRYPICKS can change over time, it’s safer to fetch missing objects by SHA and guard the ancestor check.
          # Fetch branches from which we cherry-pick
          git fetch origin refs/heads/develop:refs/heads/develop
          git fetch origin pull/3698/head:pr-3698

          # Apply cherry-picks from spack-packages.sh (match only valid git commit hashes)

.github/workflows/macos-ci.yml:151

  • The workflow expects to build specs that require a Fortran compiler (e.g. root +fortran), and spack-environment/packages.yaml requires fortran to be provided by GCC on darwin. spack compiler find won’t discover gfortran unless GCC is installed on the runner, so this job is likely to fail (or force an unexpected toolchain) on a clean macOS runner. Install GCC (for gfortran) before running spack compiler find.

      - name: Add Spack environment
        run: |
          spack -e spack-environment/${{matrix.ENV}} compiler find
          spack -e spack-environment/${{matrix.ENV}} external find llvm

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 5 out of 5 changed files in this pull request and generated 1 comment.

Suppressed comments (2)

.github/workflows/macos-ci.yml:152

  • The macOS workflow runs spack compiler find, but the Spack config now requires a GNU Fortran-capable toolchain on darwin (packages:fortran requires gcc). GitHub macOS runners typically only have Apple Clang by default, so this workflow should explicitly install GCC (for gfortran) before calling spack compiler find to avoid non-reproducible failures when the runner image changes.
      - name: Add Spack environment
        run: |
          spack -e spack-environment/${{matrix.ENV}} compiler find
          spack -e spack-environment/${{matrix.ENV}} external find llvm

.github/workflows/macos-ci.yml:83

  • The Spack/spack-packages cherry-pick logic depends on hard-coded git fetch origin pull/<N>/head refspecs. This will break the next time the cherry-pick list changes to a commit that isn’t on these specific PR refs, and it couples the workflow to specific upstream PR numbers. Consider adopting the same approach used in containers/debian/Dockerfile (full git clone then checkout the desired tag/SHA), or otherwise generating the required fetch refspecs alongside the cherry-pick list so they stay in sync.
          # Fetch branches from which we cherry-pick
          git fetch origin refs/heads/develop:refs/heads/develop
          git fetch origin pull/51584/head:pr-51584
          git fetch origin pull/52207/head:pr-52207

Comment on lines +91 to +95
require:
- spec: gcc
when: platform=linux
- spec: apple-clang
when: platform=darwin

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 5 out of 5 changed files in this pull request and generated no new comments.

Suppressed comments (5)

spack-environment/packages.yaml:84

  • apple-clang is a compiler name, not a Spack package/provider for the c virtual. Setting it under packages:c:prefer/require is unlikely to select Apple Clang and may break provider resolution on macOS. Prefer leaving c unconstrained on darwin (Spack will use the discovered compiler), and keep the gcc constraint for linux only.
    require:
    - spec: gcc
      when: platform=linux
    - spec: apple-clang
      when: platform=darwin

spack-environment/packages.yaml:95

  • Same issue as c: above: apple-clang is not a provider package for the cxx virtual, so this config is unlikely to do what is intended on macOS. Keep the gcc constraint for linux, and avoid specifying apple-clang here.
    require:
    - spec: gcc
      when: platform=linux
    - spec: apple-clang
      when: platform=darwin

.github/workflows/macos-ci.yml:110

  • Same brittleness as the Spack cherry-pick logic: this relies on a specific PR ref being fetched. Fetch each required commit SHA from spack-packages.sh before cherry-picking so the workflow stays valid as the cherry-pick set changes.
          # Fetch branches from which we cherry-pick
          git fetch origin refs/heads/develop:refs/heads/develop
          git fetch origin pull/3698/head:pr-3698

.github/workflows/macos-ci.yml:92

  • Hardcoding specific PR refs for cherry-picks is brittle: any future cherry-pick not on develop or those PR heads will fail because the commit object isn't present. Fetch the needed commits by SHA derived from spack.sh instead of pinning PR numbers.

This issue also appears on line 107 of the same file.

          # Fetch branches from which we cherry-pick
          git fetch origin refs/heads/develop:refs/heads/develop
          git fetch origin pull/51584/head:pr-51584
          git fetch origin pull/52207/head:pr-52207

.github/workflows/macos-ci.yml:61

  • This comment says the workflow extracts the buildcache URL/tag from mirrors.yaml.in, but the code just reuses SPACKPACKAGES_VERSION. Please update the comment so it matches the actual behavior (or implement the extraction).
          # Extract buildcache URL from mirrors.yaml.in (simplified - get the version)

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 5 out of 5 changed files in this pull request and generated no new comments.

Suppressed comments (3)

.github/workflows/macos-ci.yml:98

  • actions/checkout for spack/spack-packages uses the default shallow clone (fetch-depth=1), but the next step cherry-picks specific SHAs. With a shallow checkout those commits may be missing, causing git cherry-pick (or even git merge-base) to fail. Set fetch-depth: 0 for the spack-packages checkout to ensure the required commit objects are available.
      - name: Checkout spack-packages repository
        uses: actions/checkout@v7
        with:
          repository: spack/spack-packages
          path: spack/var/spack/repos/spack-packages

.github/workflows/macos-ci.yml:117

  • Same issue as the Spack cherry-pick loop: if any SHA from SPACKPACKAGES_CHERRYPICKS isn't present in the locally fetched refs, git merge-base will error and the workflow will fail. Fetch missing commit objects by SHA inside the loop before checking ancestry / cherry-picking, so the workflow doesn’t rely on keeping the hard-coded git fetch origin pull/... list in sync with spack-packages.sh.
          while read -r commit; do
            if git merge-base --is-ancestor "${commit}" HEAD; then
              echo "Skipping already applied ${commit}"
              continue
            fi

.github/workflows/macos-ci.yml:92

  • The cherry-pick loop assumes every SHA in SPACK_CHERRYPICKS is already present locally. If a SHA isn't contained in the refs fetched above, git merge-base will error (and the step will fail) before you even reach git cherry-pick. To decouple the workflow from a hard-coded list of fetched PR refs, ensure each commit object is present (fetch by SHA if missing) before calling merge-base/cherry-pick.

This issue also appears in the following locations of the same file:

  • line 94
  • line 113
          while read -r commit; do
            if git merge-base --is-ancestor "${commit}" HEAD; then
              echo "Skipping already applied ${commit}"
              continue
            fi

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 5 out of 5 changed files in this pull request and generated no new comments.

Suppressed comments (3)

.github/workflows/macos-ci.yml:111

  • The workflow hard-codes a small set of refs to git fetch before cherry-picking. Since the cherry-pick SHAs come from spack-packages.sh, this will break as soon as the list changes (missing objects → cherry-pick fails). Fetch the required SHAs directly instead of maintaining a manual ref list.
          # Fetch branches from which we cherry-pick
          git fetch origin refs/heads/develop:refs/heads/develop
          git fetch origin pull/3698/head:pr-3698
          git fetch origin pull/4871/head:pr-4871
          git fetch origin pull/5856/head:pr-5856

.github/workflows/macos-ci.yml:27

  • The workflow matrix includes the tf environment (ENV: [ci, xl, tf]), but the PR description says the macOS workflow builds only the ci and xl environments. Either update the PR description to match the actual matrix, or drop tf from the matrix so CI scope matches what the PR claims.
      fail-fast: false
      matrix:
        ENV: [ci, xl, tf]

.github/workflows/macos-ci.yml:82

  • The workflow hard-codes a small set of refs to git fetch before cherry-picking. Since the cherry-pick SHAs come from spack.sh, this is brittle: if the cherry-pick list changes, the needed objects may not be present locally and git cherry-pick can fail with an unknown revision. Fetch the required SHAs directly instead of maintaining a manual ref list.

This issue also appears on line 107 of the same file.

          # Fetch branches from which we cherry-pick
          git fetch origin refs/heads/develop:refs/heads/develop
          git fetch origin pull/51584/head:pr-51584
          git fetch origin pull/52207/head:pr-52207

Added conditional definitions for TensorFlow based on platform.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 6 out of 6 changed files in this pull request and generated 3 comments.

Suppressed comments (2)

spack-environment/packages.yaml:95

  • Same issue as above for cxx: prefer is using {spec, when} mappings. Keep prefer as a simple ordered list and use conditional require entries for platform selection.
  cxx:
    prefer:
    - spec: gcc
      when: platform=linux
    - spec: apple-clang
      when: platform=darwin
    require:
    - spec: gcc
      when: platform=linux
    - spec: apple-clang
      when: platform=darwin

.github/workflows/macos-ci.yml:27

  • PR description says the macOS workflow builds only the ci and xl environments, but the workflow matrix also runs tf (ENV: [ci, xl, tf]). Either update the PR description to include tf, or remove tf from the macOS workflow matrix to match the stated scope.
      fail-fast: false
      matrix:
        ENV: [ci, xl, tf]

Comment on lines 74 to +84
c:
prefer:
- gcc
require:
- gcc
- spec: gcc
when: platform=linux
- spec: apple-clang
when: platform=darwin
require:
- spec: gcc
when: platform=linux
- spec: apple-clang
when: platform=darwin
Comment on lines 215 to +225
fortran:
prefer:
- gcc
require:
- gcc
- spec: gcc
when: platform=linux
- spec: gcc
when: platform=darwin
require:
- spec: gcc
when: platform=linux
- spec: gcc
when: platform=darwin
Comment thread .github/workflows/macos-ci.yml

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 6 out of 6 changed files in this pull request and generated no new comments.

Suppressed comments (3)

.github/workflows/macos-ci.yml:112

  • Similar to the Spack checkout step, hard-coding PR refs for spack-packages makes this workflow fragile: when spack-packages.sh cherry-picks change, the workflow must be manually updated to fetch the right PR refs or the cherry-pick loop will fail. Fetch each cherry-pick commit by hash inside the loop and remove the PR-specific fetches.
          # Fetch branches from which we cherry-pick
          git fetch origin refs/heads/develop:refs/heads/develop
          git fetch origin pull/3698/head:pr-3698
          git fetch origin pull/4871/head:pr-4871
          git fetch origin pull/5856/head:pr-5856

spack-environment/xl/spack.yaml:145

  • valgrind is included in the XL environment, but upstream Spack’s valgrind package declares conflicts for Apple Silicon targets (e.g. target=m1 / target=m2). Since this PR adds macOS CI on macos-latest (arm64), concretization/install of the xl environment is likely to fail on those runners. Consider making valgrind Linux-only the same way strace was handled (e.g., move it into a platform-conditional definition) so macOS CI can complete.
  - stow
  - valgrind
  - xeyes

.github/workflows/macos-ci.yml:92

  • The workflow hard-codes a few PR refs to make cherry-pick commits available locally. This couples the workflow to a specific set of cherry-picks: if spack.sh is updated with a commit that isn’t reachable from develop or those PR refs, git merge-base / git cherry-pick will fail. Fetch each cherry-pick commit by hash inside the loop (and drop the PR-specific fetches) to make this self-maintaining when the cherry-pick list changes.

This issue also appears on line 107 of the same file.

          # Fetch branches from which we cherry-pick
          git fetch origin refs/heads/develop:refs/heads/develop
          git fetch origin pull/51584/head:pr-51584
          git fetch origin pull/52207/head:pr-52207

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 6 out of 6 changed files in this pull request and generated 2 comments.

Suppressed comments (3)

.github/workflows/macos-ci.yml:18

  • Using long-lived registry credentials via secrets increases blast radius and complicates PR execution. For publishing to GHCR from GitHub Actions, prefer GITHUB_TOKEN with packages: write (and github.actor as the username) where possible; additionally, avoid exporting registry credentials as job-wide env vars when they’re only needed for a single step.
permissions:
  contents: read
  packages: write

.github/workflows/macos-ci.yml:30

  • Using long-lived registry credentials via secrets increases blast radius and complicates PR execution. For publishing to GHCR from GitHub Actions, prefer GITHUB_TOKEN with packages: write (and github.actor as the username) where possible; additionally, avoid exporting registry credentials as job-wide env vars when they’re only needed for a single step.
    env:
      GITHUB_REGISTRY_USER: ${{ secrets.GHCR_REGISTRY_USER }}
      GITHUB_REGISTRY_TOKEN: ${{ secrets.GHCR_REGISTRY_TOKEN }}

spack-environment/packages.yaml:225

  • The fortran compiler config duplicates identical entries for Linux and macOS in both prefer and require. This can be simplified (e.g., dropping prefer if require is authoritative, or consolidating so there’s a single prefer: [gcc] and only conditional require entries where needed) to reduce repetition and future drift.
  fortran:
    prefer:
    - spec: gcc
      when: platform=linux
    - spec: gcc
      when: platform=darwin
    require:
    - spec: gcc
      when: platform=linux
    - spec: gcc
      when: platform=darwin

Comment on lines +28 to +30
env:
GITHUB_REGISTRY_USER: ${{ secrets.GHCR_REGISTRY_USER }}
GITHUB_REGISTRY_TOKEN: ${{ secrets.GHCR_REGISTRY_TOKEN }}
Comment on lines +155 to +161
- name: Configure buildcache
run: |
spack mirror add --scope site eic \
--autopush --unsigned \
--oci-username-variable GITHUB_REGISTRY_USER \
--oci-password-variable GITHUB_REGISTRY_TOKEN \
oci://ghcr.io/eic/spack-${{ steps.spack-config.outputs.buildcache-version }}

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 6 out of 6 changed files in this pull request and generated 4 comments.

Suppressed comments (3)

.github/workflows/macos-ci.yml:153

  • spack external find llvm can fail on macOS runners if there isn’t a discoverable external llvm package installed/configured, which would fail the whole job. Consider either installing llvm explicitly (e.g., via Homebrew) before this step, or making the external discovery non-fatal (and/or conditional) so the workflow remains robust across macOS runner images.
      - name: Add Spack environment
        run: |
          spack compiler find --scope site
          spack external find --scope site llvm

.github/workflows/macos-ci.yml:82

  • The workflow hardcodes a set of upstream PR refs to fetch before cherry-picking. This will require ongoing manual updates as cherry-picks change and can break if those refs are removed/renamed upstream. A more maintainable approach is to derive what to fetch from the configured commit list (e.g., keep a single authoritative list of required refs/branches alongside the cherry-picks, or avoid fetching PR refs unless a cherry-pick actually requires them).
          # Fetch branches from which we cherry-pick
          git fetch origin refs/heads/develop:refs/heads/develop
          git fetch origin pull/51584/head:pr-51584
          git fetch origin pull/52207/head:pr-52207

.github/workflows/macos-ci.yml:111

  • The workflow hardcodes a set of upstream PR refs to fetch before cherry-picking. This will require ongoing manual updates as cherry-picks change and can break if those refs are removed/renamed upstream. A more maintainable approach is to derive what to fetch from the configured commit list (e.g., keep a single authoritative list of required refs/branches alongside the cherry-picks, or avoid fetching PR refs unless a cherry-pick actually requires them).
          # Fetch branches from which we cherry-pick
          git fetch origin refs/heads/develop:refs/heads/develop
          git fetch origin pull/3698/head:pr-3698
          git fetch origin pull/4871/head:pr-4871
          git fetch origin pull/5856/head:pr-5856

Comment on lines +7 to +9
pull_request:
branches:
- master
Comment on lines +16 to +18
permissions:
contents: read
packages: write
Comment on lines +28 to +30
env:
GITHUB_REGISTRY_USER: ${{ secrets.GHCR_REGISTRY_USER }}
GITHUB_REGISTRY_TOKEN: ${{ secrets.GHCR_REGISTRY_TOKEN }}
Comment on lines +155 to +161
- name: Configure buildcache
run: |
spack mirror add --scope site eic \
--autopush --unsigned \
--oci-username-variable GITHUB_REGISTRY_USER \
--oci-password-variable GITHUB_REGISTRY_TOKEN \
oci://ghcr.io/eic/spack-${{ steps.spack-config.outputs.buildcache-version }}

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 6 out of 6 changed files in this pull request and generated no new comments.

Suppressed comments (1)

.github/workflows/macos-ci.yml:100

  • The spack-packages checkout uses the default shallow clone (fetch-depth=1) but the next step fetches extra refs and cherry-picks specific commits. With a shallow clone, git cherry-pick <hash> can fail if parent/history objects aren’t present. Set fetch-depth: 0 (like the Spack checkout step) to make cherry-picks reliable.
      - name: Checkout spack-packages repository
        uses: actions/checkout@v7
        with:
          repository: spack/spack-packages
          path: spack/var/spack/repos/spack-packages
          ref: ${{ steps.spack-config.outputs.spackpackages-version }}

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 6 out of 6 changed files in this pull request and generated 1 comment.

Comment on lines +158 to +161
- name: Add Spack environment
run: |
spack compiler find --scope site
spack external find --scope site llvm

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 7 out of 7 changed files in this pull request and generated no new comments.

Suppressed comments (1)

spack-environment/ci/spack.yaml:22

  • linux_packages is defined for conditional inclusion, but it is never referenced in specs:. As a result, the listed Linux-only packages (imagemagick, nopayloadclient, ollama, py-numpy, py-scipy) will be dropped from the CI environment on Linux as well as macOS. Add $linux_packages to the specs list so the packages remain on Linux while staying empty on macOS.
  specs:
  - acts
  - actsvg

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants