Skip to content

Create temp dir instead of letting unzip do it#106

Merged
zonble merged 1 commit intomasterfrom
dev/unzip-behavior-change
Mar 2, 2025
Merged

Create temp dir instead of letting unzip do it#106
zonble merged 1 commit intomasterfrom
dev/unzip-behavior-change

Conversation

@lukhnos
Copy link
Copy Markdown
Collaborator

@lukhnos lukhnos commented Mar 2, 2025 •
edited by github-actions bot
Loading

User description

This fix #105 as a defensive strategy against any potential unzip -d behavior changes in macOS.

This has the same code as openvanilla/McBopomofo#621 and was also tested using a release side build to make sure the Installer works even if unzip -d no longer behaves the same.

The code also replaces two uses of assert with guard so that the checks are always evaluated no matter which kind of build (debug/release) it is.

PR Type

Bug fix, Enhancement

Description

  • Fixes unzip behavior change in macOS by creating directories manually.

  • Replaces assert with guard for better error handling.

  • Adds logging for directory creation and unzip failures.

  • Ensures compatibility with macOS 15.4 Beta (24E5206s).

Changes walkthrough 📝

Relevant files
Bug fix
ArchiveUtil.swift
Handle unzip behavior changes and improve error handling 

Source/Mac/Installer/ArchiveUtil.swift

  • Added manual directory creation to handle unzip behavior changes.
  • Replaced assert with guard for runtime error checks.
  • Added logging for failures in directory creation and unzipping.
  • Improved compatibility with macOS 15.4 Beta.
    		• +20/-2   
    Need help?
  • Type /help how to ... in the comments thread for any questions about PR-Agent usage.
  • Check out the documentation for more information.
    •

    @lukhnos
    Create temp dir instead of letting unzip do it
    709b49d 
    This fix #105 as a defensive strategy against any potential unzip -d
behavior changes in macOS.
    gemini-code-assist[bot]
    gemini-code-assist bot reviewed Mar 2, 2025
    View reviewed changes
    Copy link
    Copy Markdown

    @gemini-code-assist gemini-code-assist bot left a comment

    Choose a reason for hiding this comment

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

    Hello @lukhnos, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed!

    Summary of Changes

    This pull request addresses issue #105 by implementing a defensive strategy against potential behavior changes in the unzip -d command on macOS. Specifically, it ensures that the target directory for unzipping is created explicitly before calling the unzip command. Additionally, the code replaces assert statements with guard statements to ensure that checks are always evaluated, regardless of the build type (debug/release).

    Highlights

    • Unzip Directory Creation: The code now explicitly creates the target directory for unzipping to prevent issues caused by potential changes in the unzip -d command's behavior.
    • Assertion Replacement: The code replaces two instances of assert with guard to ensure checks are always evaluated, even in release builds.

    Changelog

    • Source/Mac/Installer/ArchiveUtil.swift
      • Added code to create the temporary directory before calling unzip to handle potential unzip -d behavior changes (lines 91-99).
      • Replaced assert with guard to check the unzipTask.terminationStatus and handle errors (lines 109-112).
      • Replaced assert with guard to check if the app bundle exists after unzipping and handle errors (lines 115-118).
    Using Gemini Code Assist

    The full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips.

    Invoking Gemini

    You can request assistance from Gemini at any point in your pull request via creating an issue comment (i.e. comment on the pull request page) using either /gemini <command> or @gemini-code-assist <command>. Below is a summary of the supported commands.

    Feature Command Description
    Code Review /gemini review Performs a code review for the current pull request in its current state.
    Pull Request Summary /gemini summary Provides a summary of the current pull request in its current state.
    Comment @gemini-code-assist Responds in comments when explicitly tagged, both in issue comments and review comments.
    Help /gemini help Displays a list of available commands.

    Customization

    To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a .gemini/ folder in the base of the repository. Detailed instructions can be found here.

    Limitations & Feedback

    Gemini Code Assist is currently in preview and may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments to provide feedback.

    You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension.

    Trivia time!

    What is the default compression algorithm used by the zip file format?

    Click here for the answer
    The original and most common compression algorithm used in zip files is DEFLATE.

    Footnotes

    1. Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configure Gemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution. ↩

    gemini-code-assist[bot]
    gemini-code-assist bot reviewed Mar 2, 2025
    View reviewed changes
    Copy link
    Copy Markdown

    @gemini-code-assist gemini-code-assist bot left a comment

    Choose a reason for hiding this comment

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

    Code Review

    The pull request addresses a potential issue with the unzip -d command on macOS by creating the target directory manually before running the command. This is a good defensive strategy. The code also replaces assert statements with guard statements, which ensures that the checks are always evaluated, regardless of the build type. Overall, the changes seem well-reasoned and implemented.

    Summary of Findings

    • Error Logging: The error logging could be improved to provide more context and potentially aid in debugging.

    Merge Readiness

    The pull request appears to be in good shape. The changes are well-explained and address a specific issue. I am unable to directly approve the pull request, and users should have others review and approve this code before merging. I would recommend addressing the error logging suggestion before merging, as it could be helpful for debugging in the future.

    @github-actions
    Copy link
    Copy Markdown

    github-actions bot commented Mar 2, 2025

    PR Reviewer Guide 🔍

    Here are some key observations to aid the review process:

    🎫 Ticket compliance analysis 🔶

    105 - Partially compliant

    Compliant requirements:

    • Fix the issue where installation fails on macOS 15.4 Beta (24E5206s) with the error "Cannot copy the file to the destination."
    • Ensure compatibility with macOS 15.4 Beta.
    • Address potential behavior changes in unzip -d.

    Non-compliant requirements:

    []

    Requires further human verification:

    []

    ⏱️ Estimated effort to review: 2 🔵🔵⚪⚪⚪
    🧪 No relevant tests
    🔒 No security concerns identified
    ⚡ Recommended focus areas for review

    Error Handling

    Ensure that the error handling for directory creation and unzip failures is robust and logs sufficient information for debugging.

     
    // applefeedback://FB16686738 unzip -d behavior change breaks scripts
// and apps depending on its creating non-existent target directories;
// we now create the directory ourselves.
do {
    try FileManager.default.createDirectory(atPath: tempFilePath, withIntermediateDirectories: true)
} catch {
    NSLog("fatal: could not create directory \(tempFilePath)")
    return nil
}

let arguments: [String] = [notarizedArchive, "-d", tempFilePath]
let unzipTask = Process()
unzipTask.launchPath = "/usr/bin/unzip"
unzipTask.currentDirectoryPath = resourcePath
unzipTask.arguments = arguments
unzipTask.launch()
unzipTask.waitUntilExit()

guard unzipTask.terminationStatus == 0 else {
    NSLog("fatal: could not unzip \(notarizedArchive) to \(tempFilePath)")
    return nil
}

let result = (tempFilePath as NSString).appendingPathComponent(targetAppBundleName)
guard FileManager.default.fileExists(atPath: result) else {
    NSLog("fatal: \(result) does not exist")
    return nil

    @github-actions
    Copy link
    Copy Markdown

    github-actions bot commented Mar 2, 2025

    PR Code Suggestions ✨

    @zonble zonble merged commit 21f70c0 into master Mar 2, 2025
    2 checks passed
    @lukhnos lukhnos deleted the dev/unzip-behavior-change branch March 2, 2025 15:23
    Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

    Reviewers

    @tianjianjiang tianjianjiang tianjianjiang approved these changes

    +1 more reviewer

    @gemini-code-assist gemini-code-assist[bot] gemini-code-assist[bot] left review comments

    Reviewers whose approvals may not affect merge requirements

    Assignees

    No one assigned

    Labels

    Review effort 2/5

    Projects

    None yet

    Milestone

    No milestone

    Development

    Successfully merging this pull request may close these issues.

    在MacOS 15.4 Beta (24E5206s) 無法安裝

    3 participants

    @lukhnos @tianjianjiang @zonble