Skip to content

fix(oci): account for darwin mode stripping - #10681

Merged
jdx merged 1 commit into
jdx:mainfrom
laozc:fix-darwin-nix
Jul 1, 2026
Merged

jdx merged 1 commit into
jdx:mainfrom
laozc:fix-darwin-nix

Conversation

@laozc

@laozc laozc commented Jul 1, 2026 •

Copy link
Copy Markdown
Contributor

Summary

Fix the Darwin Nix build failure by making the OCI layer test assert the mode that the filesystem actually retains after set_permissions.

On this filesystem, the setuid bit is stripped from the test file, so the previous hard-coded 04755 expectation was too strict.

Verification

  • nix build .#mise --print-build-logs
  • Result: test result: ok. 1460 passed; 0 failed; 0 ignored; 0 measured; 1 filtered out

Notes

  • The production layer code was already preserving the metadata that the filesystem exposed.
  • This change keeps the sticky-bit check for the directory case and makes the file-mode expectation reflect the platform behavior.

Summary by CodeRabbit

  • Tests
    • Improved coverage for preserving special permission bits in Unix layer builds by validating the actual helper file permissions instead of relying on a fixed expected value.

@coderabbitai

coderabbitai Bot commented Jul 1, 2026 •

Copy link
Copy Markdown
Contributor

Review Change Stack

📝 Walkthrough

Walkthrough

A unit test in src/oci/layer.rs was updated so that the expected permission mode for bin/helper is computed from the actual on-disk metadata rather than hardcoded, while the sticky-bit assertion for bin remains unchanged.

Changes

Test Permission Mode Fix

Layer / File(s) Summary
Compute expected mode from actual permissions
src/oci/layer.rs
Test now derives the expected mode for bin/helper from symlink_metadata instead of asserting a hardcoded 0o4755 value.

Estimated code review effort

🎯 1 (Trivial) | ⏱️ ~3 minutes

Poem

A rabbit checked a helper's bits,
No more hardcoded fits,
Now the mode's read true and fair,
Straight from metadata's lair,
Hop, hop, tests pass with care! 🐇

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
Title check ✅ Passed The title clearly matches the main change: adjusting OCI layer behavior for Darwin mode stripping.

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@laozc laozc changed the title test(oci): account for darwin mode stripping fix(oci): account for darwin mode stripping Jul 1, 2026
@greptile-apps

greptile-apps Bot commented Jul 1, 2026

Copy link
Copy Markdown
Contributor

Greptile Summary

This PR updates the OCI layer metadata test for platforms that strip file mode bits. The main changes are:

  • Reads the helper file mode after set_permissions.
  • Uses the retained helper mode as the expected layer mode.
  • Keeps the directory sticky-bit assertion unchanged.

Confidence Score: 5/5

This looks safe to merge.

  • No blocking issues found in the changed code.

Important Files Changed

Filename Overview
src/oci/layer.rs Updates the OCI metadata preservation test to compare against the file mode retained by the local filesystem.

Reviews (1): Last reviewed commit: "test(oci): account for darwin mode strip..." | Re-trigger Greptile

@jdx
jdx enabled auto-merge (squash) July 1, 2026 21:08
@jdx
jdx disabled auto-merge July 1, 2026 21:08
@jdx
jdx merged commit 4b7cec3 into jdx:main Jul 1, 2026
31 of 33 checks passed
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.

2 participants