-
Notifications
You must be signed in to change notification settings - Fork 106
Support mkfs.simplefs on macOS #82
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No issues found across 1 file
| @@ -1,5 +1,14 @@ | |||
| #include <fcntl.h> | |||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Consider to add this build check:
diff --git a/mkfs.c b/mkfs.c
index 59dd967..c2f5626 100644
--- a/mkfs.c
+++ b/mkfs.c
@@ -1,3 +1,8 @@
+#if !defined(__linux__) && !defined(__APPLE__)
+#error \
+ "Do not manage to build this file unless your platform is Linux or macOS."
+#endif
+
#include <fcntl.h>
#include <linux/fs.h>
#include <stdint.h>
|
Once this is fixed, consider to help to enable mounting simplefs on macOS runner at CI [1], [2]. [1] https://github.com/sysprog21/rv32emu/blob/master/.ci/boot-linux-prepare.sh |
|
Can you add CI for the verification? |
mkfs.simplefs depended on Linux-only headers and ioctl calls, so the rv32emu CI could not run simplefs tests on macOS runners. Add macOS-specific endian helpers and detect block device size via DKIOCGETBLOCKCOUNT/DKIOCGETBLOCKSIZE while keeping the Linux path (BLKGETSIZE64) unchanged. Fixes: sysprog21#78 Signed-off-by: hsule <leann9001@gmail.com>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
1 issue found across 3 files
Prompt for AI agents (all issues)
Check if these issues are valid — if so, understand the root cause of each and fix them.
<file name=".ci/test-mkfs-macos.sh">
<violation number="1" location=".ci/test-mkfs-macos.sh:16">
P2: macOS dd does not support the GNU `status=none` operand, so this dd call will fail and abort the macOS test run under `set -e`.</violation>
</file>
Since this is your first cubic review, here's how it works:
- cubic automatically reviews your code and comments on bugs and improvements
- Teach cubic by replying to its comments. cubic learns from your replies and gets better over time
- Ask questions if you need clarification on any suggestion
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
|
@ChinYikMing Thanks for the review! |
|
rv32emu relies on tag |
Add a CI script to build mkfs.simplefs and format a test image on macOS runners. Signed-off-by: hsule <leann9001@gmail.com>
Yes, let's move forward. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No issues found across 3 files
ChinYikMing
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM!
|
Thank @hsule for contributing! |
|
@jserv, since the PR is now merged, could you please help bump the tag to
|
I would like to wait for #79 and prepare new tag next week. |
Fixes issue#78 with enabling
mkfs.simplefsto be compiled and executed on macOS. Currently, the utility depends on Linux-specific headers andioctloperations, which prevents therv32emuCI from runningsimplefstest cases on macOS runners.Summary by cubic
Enable mkfs.simplefs to build and run on macOS by adding platform-specific headers, endian helpers, and block device size detection. This unblocks running simplefs tests on macOS runners while keeping Linux behavior unchanged.
Written for commit 1889a19. Summary will update on new commits.