Skip to content

Conversation

@roynor
Copy link

@roynor roynor commented Nov 5, 2025

Fixed the issue where the compression platform was set to Unix in the Windows environment. Now the platform can be automatically set to Dos or Unix based on the specific operating system.

Windows; need auto-detection or manual configuration
@gemini-code-assist
Copy link
Contributor

Summary of Changes

Hello @roynor, 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!

This pull request resolves a bug where the platform field in generated zip files was incorrectly set to "Unix" even when created on a Windows operating system. The change introduces automatic detection of the operating system during zip file creation, ensuring that the system field is accurately set to "Dos" for Windows environments and "Unix" for Linux environments, improving compatibility and correctness of zip archives.

Highlights

  • Platform Auto-detection: The ZipFileData structure now automatically detects the operating system (Windows, Linux) to set the appropriate compression platform (Dos, Unix) for zip files.
  • Bug Fix: Addresses an issue where zip files created on Windows incorrectly reported their platform as Unix, ensuring correct platform identification.
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 by creating a comment using either /gemini <command> or @gemini-code-assist <command>. Below is a summary of the supported commands on the current page.

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 pull request 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 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. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here.

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.

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.

Copy link
Contributor

@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

This pull request correctly addresses the issue of the zip platform being hardcoded to Unix by introducing OS detection. The platform is now correctly set to DOS on Windows. However, the check for Unix-like systems is too specific, using cfg!(linux), which will incorrectly classify other common platforms like macOS as System::Unknown. I've provided a suggestion to use the more general cfg!(unix) to properly support all Unix-like systems.

@Pr0methean Pr0methean enabled auto-merge November 19, 2025 22:44
@Pr0methean Pr0methean changed the title Fix #470:Default "platform" field in zip files is set to "Unix" on Windows; need auto-detection or manual configuration fix: Default "platform" field in zip files should be set to the local platform, rather than always "Unix" (#470) Nov 19, 2025
};
let mut local_block = ZipFileData {
system: System::Unix,
system: system,

Choose a reason for hiding this comment

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

Simplify:

Suggested change
system: system,
system,

@pzhlkj6612
Copy link

failures:
    read::test::test_cannot_symlink_outside_destination
    write::test::write_mimetype_zip
    write::test::write_non_utf8
    write::test::write_symlink_simple
    write::test::write_symlink_wonky_paths
    write::test::write_zip_dir

test result: FAILED. 82 passed; 6 failed

Well, I have no idea why all test suites in the "tests" directory was not be counted without specifying --lib 1. For now, I encountered failed tests in the test suite "end_to_end" on Windows and I think I know the reason.

In initialize_local_block(), we have no parameter to designate if the current ZipFileData is related to a directory or a file, So the external_attributes field is always permissions << 16 2.

Because of the incorrect external_attributes, the System::Dos branch in unix_mode() always produces ffi::S_IFREG | 0o0664 == 0o0100664 3. This incorrect Unix mode has at least two negative effects on our ZIP file:

  • It may cause an incorrect version_made_by field by making the check mode & S_IFDIR == S_IFDIR in version_needed() always fail 4.
  • It does cover the real Unix mode of every item by a constant 0o664 5 when someone calls the unix_mode() function.

The 2nd point is the reason why the "end_to_end" suite failed.

Footnotes

  1. https://github.com/zip-rs/zip2/actions/runs/19553408314/job/55990236771?pr=471

  2. /src/types.rs#L750

  3. /src/types.rs#L626-L638

  4. /src/types.rs#L669-L677

  5. /tests/end_to_end.rs#L252

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.

3 participants