Skip to content

build: minor fixes to notarize.sh and getversion.sh#1267

Closed
Wpc-121 wants to merge 2 commits intoActivityWatch:masterfrom
Wpc-121:mod_001
Closed

build: minor fixes to notarize.sh and getversion.sh#1267
Wpc-121 wants to merge 2 commits intoActivityWatch:masterfrom
Wpc-121:mod_001

Conversation

@Wpc-121
Copy link
Copy Markdown
Contributor

@Wpc-121 Wpc-121 commented Apr 18, 2026

No description provided.

Wpc-121 added 2 commits April 18, 2026 16:41
修复notarization_method未正确设置的问题,将altool检查逻辑移至else块中
@greptile-apps
Copy link
Copy Markdown
Contributor

greptile-apps Bot commented Apr 18, 2026

Greptile Summary

This PR makes two targeted improvements to the macOS build/notarization scripts. In notarize.sh, it fixes a bash bug where output=xcrun altool >/dev/null 2>&1 never actually ran xcrun altool (it assigned the string "xcrun" to output and attempted to execute altool as a standalone command), meaning altool detection always silently failed; the fix uses a proper if/else structure and calls xcrun altool directly. In build_app_tauri.sh, the hardcoded VERSION="0.1.0" is replaced with a dynamic version from getversion.sh.

Confidence Score: 4/5

Safe to merge once the open dev-build version format concern (from previous thread) is addressed.

The notarize.sh fix is a clear correctness improvement with no new issues. The dynamic version in build_app_tauri.sh is better than a hardcoded placeholder, but the CFBundleShortVersionString/CFBundleVersion validity for dev builds (non-numeric suffix) remains an open P1 flagged in the previous review thread and is not yet resolved in this PR.

scripts/package/build_app_tauri.sh — CFBundleShortVersionString and CFBundleVersion validity on dev builds.

Important Files Changed

Filename Overview
scripts/notarize.sh Fixes a bash bug where output=xcrun altool failed to detect altool, and correctly restructures the tool-detection logic into an if/else so notarytool takes precedence.
scripts/package/build_app_tauri.sh Replaces hardcoded VERSION="0.1.0" with a dynamic version sourced from getversion.sh; on non-tagged commits the version string contains a non-numeric dev suffix that is invalid for Apple's CFBundleShortVersionString and CFBundleVersion plist keys (flagged in previous review thread).

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A[notarize.sh starts] --> B{xcrun notarytool available?}
    B -- Yes --> C[notarization_method = run_notarytool]
    B -- No --> D{xcrun altool available?}
    D -- Yes --> E[notarization_method = run_altool]
    D -- No --> F[notarization_method = exit]
    C --> G{method == exit?}
    E --> G
    F --> G
    G -- Yes --> H[Exit with error]
    G -- No --> I[Notarize .app and .dmg]
    I --> J[run_stapler]
Loading

Reviews (2): Last reviewed commit: "build(tauri): 从getversion.sh获取版本号并移除v前缀" | Re-trigger Greptile

VERSION="0.1.0"
SCRIPT_DIR="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)"
VERSION_WITH_V="$("$SCRIPT_DIR/getversion.sh")"
VERSION="$(echo "$VERSION_WITH_V" | sed -e 's/^v//')"
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.

P2 Dev-build VERSION invalid for CFBundleShortVersionString

When the repo is not on an exact tag, getversion.sh outputs a version like v0.13.0.dev-abc1234, which after stripping v becomes 0.13.0.dev-abc1234. Apple's CFBundleShortVersionString must consist of 1–3 period-separated integers (e.g. 0.13.0). A non-numeric suffix can cause codesign/notarization warnings or failures on dev builds.

@ErikBjare ErikBjare closed this Apr 18, 2026
@ErikBjare ErikBjare reopened this Apr 18, 2026
@ErikBjare ErikBjare changed the title .2059806565729300:699ce3027bf3d778dc1c7e47b3d62d73_69e3499ab13256d09ece8088.69e349a0b13256d09ece808c.69e349a07afba9383018b931:Trae CN.T(2026/4/18 17:06:40) build: minor fixes to notarize.sh and getversion.sh Apr 18, 2026
@ErikBjare
Copy link
Copy Markdown
Member

Changes to notarize merged in #1266, this would need to be rebased.

@ErikBjare
Copy link
Copy Markdown
Member

Clean up your existing PRs, don't spam more. Don't use noisy titles.

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