Skip to content

Bump Sparkle to 2.9.1 and fix mac build JDK selection#132

Open
rz-openai wants to merge 1 commit intomainfrom
dev/rz/update-dependencies-and-build
Open

Bump Sparkle to 2.9.1 and fix mac build JDK selection#132
rz-openai wants to merge 1 commit intomainfrom
dev/rz/update-dependencies-and-build

Conversation

@rz-openai
Copy link
Copy Markdown
Collaborator

Summary

  • Update the mac app’s Sparkle dependency from 2.7.1 to 2.9.1
  • Mark the update-check view and view model as @MainActor to match Sparkle’s newer concurrency annotations
  • Adjust the helper build script to prefer a Java 17 install that works with Compose packaging, even when JAVA_HOME is set to a non-17 JDK

Testing

  • Verified xcodebuild -project Snap-O.xcodeproj -scheme Snap-O CODE_SIGNING_ALLOWED=NO CODE_SIGNING_REQUIRED=NO build completes successfully

Comment thread snapo-app-mac/Snap-O.xcodeproj/project.pbxproj
Copy link
Copy Markdown

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: c682895b25

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

runOnlyForDeploymentPostprocessing = 0;
shellPath = /bin/sh;
shellScript = "set -e\n\nCOMPOSE_DIR=\"${PROJECT_DIR}/../snapo-desktop-compose\"\n\nif [ ! -d \"${COMPOSE_DIR}\" ]; then\n echo \"error: snapo-desktop-compose not found at ${COMPOSE_DIR}\" >&2\n exit 1\nfi\n\nJAVA_HOME_17=\"$(/usr/libexec/java_home -v 17 2>/dev/null || true)\"\nif [ -z \"${JAVA_HOME_17}\" ]; then\n echo \"error: JDK 17 not found. Install Temurin 17 (or set JAVA_HOME) to build snapo-desktop-compose.\" >&2\n exit 1\nfi\n\nexport JAVA_HOME=\"${JAVA_HOME_17}\"\nexport PATH=\"${JAVA_HOME}/bin:${PATH}\"\n\nTASK=\"createDistributable\"\nif [ \"${CONFIGURATION}\" = \"Release\" ]; then\n TASK=\"createReleaseDistributable\"\nfi\n\n( cd \"${COMPOSE_DIR}\" && ./gradlew \"${TASK}\" )\n";
shellScript = "set -e\n\nCOMPOSE_DIR=\"${PROJECT_DIR}/../snapo-desktop-compose\"\n\nif [ ! -d \"${COMPOSE_DIR}\" ]; then\n echo \"error: snapo-desktop-compose not found at ${COMPOSE_DIR}\" >&2\n exit 1\nfi\n\nJAVA_HOME_17=\"${JAVA_HOME:-}\"\nif [ -n \"${JAVA_HOME_17}\" ] && [ -x \"${JAVA_HOME_17}/bin/java\" ]; then\n case \"$(\"${JAVA_HOME_17}/bin/java\" -version 2>&1 | /usr/bin/head -n 1)\" in\n *\\\"17.*\\\"*) ;;\n *) JAVA_HOME_17=\"\" ;;\n esac\nfi\nTEMURIN_17_HOME=\"/Library/Java/JavaVirtualMachines/temurin-17.jdk/Contents/Home\"\nif [ -z \"${JAVA_HOME_17}\" ] && [ -x \"${TEMURIN_17_HOME}/bin/java\" ]; then\n JAVA_HOME_17=\"${TEMURIN_17_HOME}\"\nfi\nif [ -z \"${JAVA_HOME_17}\" ]; then\n JAVA_HOME_17=\"$(/usr/libexec/java_home -v 17 2>/dev/null || true)\"\nfi\nif [ -z \"${JAVA_HOME_17}\" ]; then\n echo \"error: JDK 17 not found. Install Temurin 17 (or set JAVA_HOME) to build snapo-desktop-compose.\" >&2\n exit 1\nfi\n\nexport JAVA_HOME=\"${JAVA_HOME_17}\"\nexport PATH=\"${JAVA_HOME}/bin:${PATH}\"\n\nTASK=\"createDistributable\"\nif [ \"${CONFIGURATION}\" = \"Release\" ]; then\n TASK=\"createReleaseDistributable\"\nfi\n\n( cd \"${COMPOSE_DIR}\" && ./gradlew \"${TASK}\" )\n";
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P1 Badge Reset invalid JAVA_HOME before JDK fallback

The build script only clears JAVA_HOME_17 when ${JAVA_HOME}/bin/java exists and is not Java 17. If JAVA_HOME is set but points to a stale/nonexistent path, JAVA_HOME_17 remains non-empty, so Temurin and java_home -v 17 fallbacks are skipped. The script then exports an invalid JAVA_HOME, causing Gradle packaging to fail.

Useful? React with 👍 / 👎.

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.

1 participant