Skip to content

Conversation

@redsun82
Copy link
Contributor

@redsun82 redsun82 commented Jun 2, 2025

These are the result of this run, and after I understood there was no way for a reviewer to check that, I changed the bazel wrapping to directly use the zip archives from that run. This means that now the SHA256 printed by the GitHub UI in the workflow summary can be directly compared with the SHA256 in the LFS pointers.

In this case:

image

compares to

image

Copilot AI review requested due to automatic review settings June 2, 2025 16:10
@redsun82 redsun82 requested a review from a team as a code owner June 2, 2025 16:10
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR upgrades ripunzip to version 2.0.2 by switching from direct binary LFS pointers to ZIP archives in Bazel builds and updating BUILD rules accordingly.

  • Replaced per-OS binary LFS pointers with ZIP-based LFS pointers.
  • Introduced a native_binary rule in BUILD.ripunzip.bazel and updated the alias in BUILD.bazel.
  • Swapped out lfs_files for lfs_archive in MODULE.bazel to consume the new ZIP archives.

Reviewed Changes

Copilot reviewed 9 out of 9 changed files in this pull request and generated 1 comment.

Show a summary per file
File Description
misc/ripunzip/ripunzip-windows.exe Removed LFS pointer lines, leaving an empty file
misc/ripunzip/ripunzip-macos Removed LFS pointer lines, leaving an empty file
misc/ripunzip/ripunzip-linux Removed LFS pointer lines, leaving an empty file
misc/ripunzip/ripunzip-macOS.zip Added LFS pointer for macOS ZIP archive
misc/ripunzip/ripunzip-Windows.zip Added LFS pointer for Windows ZIP archive
misc/ripunzip/ripunzip-Linux.zip Added LFS pointer for Linux ZIP archive
misc/ripunzip/BUILD.ripunzip.bazel Added a native_binary target wrapping the unzipped artifact
misc/ripunzip/BUILD.bazel Updated alias ripunzip to point to new per-OS repos
MODULE.bazel Replaced lfs_files rules with lfs_archive rules
Comments suppressed due to low confidence (2)

misc/ripunzip/ripunzip-windows.exe:1

  • The binary file ripunzip-windows.exe has been emptied instead of being deleted or replaced, resulting in a zero-byte tracked file. Please remove this file from the repository or restore the correct LFS pointer content.
version https://git-lfs.github.com/spec/v1

misc/ripunzip/BUILD.bazel:4

  • The select on actual covers only Linux, Windows, and macOS without a //conditions:default fallback. Adding a default case will prevent build failures on other platforms.
actual = select({"@platforms//os:" + os: "@ripunzip-%s//:ripunzip" % os for os in ("linux", "windows", "macos")} ),

@redsun82 redsun82 marked this pull request as draft June 2, 2025 16:16
@redsun82 redsun82 marked this pull request as ready for review June 2, 2025 16:25
@redsun82
Copy link
Contributor Author

redsun82 commented Jun 2, 2025

(we are not making the workflow directly update the LFS files to avoid spamming the LFS quota, but that could be rediscussed)

@redsun82 redsun82 merged commit f48012a into main Jun 3, 2025
24 checks passed
@redsun82 redsun82 deleted the redsun82/update-ripunzip branch June 3, 2025 11:51
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