Skip to content

Conversation

@kolipakakondal
Copy link
Collaborator

@kolipakakondal kolipakakondal commented Oct 16, 2025

Description

Dynamic discovery of of signtool.exe

Fixes # (IEP-1647)

Type of change

Please delete options that are not relevant.

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • This change requires a documentation update

How has this been tested?

Please describe the tests that you ran to verify your changes. Provide instructions so we can reproduce. Please also list any relevant details for your test configuration

  • Test A
  • Test B

Test Configuration:

  • ESP-IDF Version:
  • OS (Windows,Linux and macOS):

Dependent components impacted by this PR:

  • Component 1
  • Component 2

Checklist

  • PR Self Reviewed
  • Applied Code formatting
  • Added Documentation
  • Added Unit Test
  • Verified on all platforms - Windows,Linux and macOS

Summary by CodeRabbit

  • Chores
    • Improved Windows release signing by dynamically locating the signing tool during builds, enhancing reliability across different environments. No changes to application functionality or installer contents.

@coderabbitai
Copy link

coderabbitai bot commented Oct 16, 2025

Caution

Review failed

The pull request is closed.

Walkthrough

The CI workflow for Windows code signing in .github/workflows/ci_release.yml was updated to dynamically discover signtool.exe under Program Files (x86)\Windows Kits via recursive search, store its path in a variable, and use that variable for both signing and verification commands. Other parameters remain unchanged.

Changes

Cohort / File(s) Summary of Changes
CI workflow — Dynamic signtool discovery
.github/workflows/ci_release.yml
Replaced hardcoded signtool.exe path with a recursive search under C:\Program Files (x86)\Windows Kits\..., assigning the found path to a variable and invoking it for signing and verification; unchanged signing/verification arguments.

Sequence Diagram(s)

sequenceDiagram
    autonumber
    actor Dev as GitHub Actions
    participant Runner as Windows Runner
    participant FS as Filesystem
    participant ST as signtool.exe

    Dev->>Runner: Start windows-sign job
    rect rgb(240,245,255)
      note over Runner: Discover signtool.exe path (dynamic)
      Runner->>FS: Recursively search "C:\Program Files (x86)\Windows Kits\...\signtool.exe"
      FS-->>Runner: Return found path or none
      alt Path found
        Runner->>Runner: SIGNTOOL="...\\signtool.exe"
      else Not found
        Runner-->>Dev: Fail job (missing signtool)
      end
    end

    rect rgb(245,255,245)
      note over Runner,ST: Use discovered path
      Runner->>ST: Sign artifact with existing parameters
      ST-->>Runner: Signing result
      Runner->>ST: Verify signed artifact
      ST-->>Runner: Verification result
    end
Loading

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Possibly related PRs

Suggested reviewers

  • alirana01
  • sigmaaa

Poem

A rabbit hopped through Windows kits at night,
Nose twitch—found signtool by soft moonlight.
No hardcoded trails, just paths that flow,
Sign, then verify—onward we go.
Thump-thump, artifacts sealed tight—
CI burrows done just right. 🐇✨

✨ Finishing touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch IEP-1647

📜 Recent review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 8d8ef63 and e9bd545.

📒 Files selected for processing (1)
  • .github/workflows/ci_release.yml (2 hunks)

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@kolipakakondal kolipakakondal merged commit 0cade9d into master Oct 16, 2025
4 checks passed
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.

2 participants