clean between darwin archs #28866
clean between darwin archs #28866
Conversation
Co-authored-by: Mike Maxim <mike.maxim@zoom.us>
There was a problem hiding this comment.
Pull request overview
This pull request adds git repository cleaning between darwin architecture builds to ensure a clean state before building the arm64 version. The PR addresses a potential issue where build artifacts from the amd64 build could interfere with the subsequent arm64 build.
Changes:
- Added
git reset --hardandgit clean -fdcommands between the amd64 and arm64 darwin builds - Added a comment explaining why cleaning is needed between architecture builds
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| ARCH="amd64" PLATFORM="darwin" "$client_dir/packaging/prerelease/pull_build.sh" | ||
|
|
||
| # Clean repo between arch builds | ||
| cd "$client_dir" && git reset --hard && git clean -fd |
There was a problem hiding this comment.
The commands on this line are chained together with && operators, which can make error tracking difficult and is inconsistent with the coding style used in other packaging scripts. Consider splitting this into separate lines for better readability and error handling:
cd "$client_dir"
git reset --hard
git clean -fd
This also helps identify which specific operation failed if there's an issue.
| cd "$client_dir" && git reset --hard && git clean -fd | |
| cd "$client_dir" | |
| git reset --hard | |
| git clean -fd |
| "$client_dir/packaging/slack/send.sh" "Starting darwin build" | ||
| ARCH="amd64" PLATFORM="darwin" "$client_dir/packaging/prerelease/pull_build.sh" | ||
|
|
||
| # Clean repo between arch builds |
There was a problem hiding this comment.
The 'git clean -fd' command will forcefully delete all untracked files and directories. While this is likely the intended behavior to ensure a clean state between architecture builds, it could potentially delete important untracked files if the script is run in an unexpected context.
Consider adding a comment explaining why these destructive operations are safe in this context (e.g., "Safe to force clean since check_status_and_pull.sh verifies repo is clean before first build, and we only need to remove build artifacts"). This helps prevent future maintainers from accidentally misusing this pattern in contexts where it might not be safe.
| # Clean repo between arch builds | |
| # Clean repo between arch builds. Safe to force reset/clean here because this | |
| # script runs in the dedicated client repo and is only expected to remove build | |
| # artifacts between the amd64 and arm64 builds (no user work should be untracked). |
| cd "$client_dir" && git reset --hard && git clean -fd | ||
|
|
||
| # NOTE: We build the arm64 version second to get a later timestamp, so it will | ||
| # be presented as a later version to your updater. This allows the one-time | ||
| # upgrading from the x86 build to the arm64 one. |
There was a problem hiding this comment.
After the git reset and clean, the second architecture build will call check_status_and_pull.sh which performs a 'git pull --ff-only'. This means if new commits are pushed to the remote between the amd64 and arm64 builds, the two architectures will be built from different commits.
Consider setting NOPULL=1 for the second build to ensure both architectures are built from the same commit:
ARCH="arm64" PLATFORM="darwin-arm64" NOPULL=1 "$client_dir/packaging/prerelease/pull_build.sh"
This would make the builds more deterministic and ensure version consistency across architectures.
2 participants
No description provided.