Add support for custom temporary directories in image creation scripts#26
Add support for custom temporary directories in image creation scripts#26ali-ghanati wants to merge 2 commits into
Conversation
- Updated `make_image.bat`, `mkexfat_macos.sh`, and `mkexfat.sh` to accept a temporary directory argument for improved flexibility. - Enhanced error handling for missing or invalid temporary directories. - Modified `New-OsfExfatImage.ps1` to utilize a custom temp directory for logs and temporary files. - Updated usage instructions in all scripts to reflect new options.
|
Caution Review failedThe pull request is closed. ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (4)
📝 WalkthroughWalkthroughAdds an optional temp-directory option across PowerShell, Batch, Linux, and macOS scripts to control where temporary files and mount points are created during exFAT image building; includes CLI parsing, validation, dynamic mounts, and a PowerShell temp-file helper. ChangesTemporary Directory Control Across Platforms
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@make_image.bat`:
- Around line 75-77: Replace the insecure hardcoded "-ExecutionPolicy Bypass"
invocations for launching "%SCRIPT%" with a safer approach: sign
New-OsfExfatImage.ps1 using a code-signing certificate and change the PowerShell
launches in the batch file (the two powershell.exe calls that pass -File
"%SCRIPT%") to use "-ExecutionPolicy RemoteSigned"; if signing cannot be done,
add clear documentation near the "%SCRIPT%" invocation explaining the security
implications of using Bypass and require an explicit opt-in approval step
instead of silently bypassing execution policy.
In `@mkexfat_macos.sh`:
- Around line 76-79: Add an explicit validation step before normalizing OUTPUT:
check the parent directory of OUTPUT (derived via dirname "$OUTPUT") exists and
is writable; if it does not, print a clear error message referencing OUTPUT and
exit non-zero instead of letting the subsequent cd fail. Implement this
validation right before the OUTPUT="$(cd ...)" normalization and keep the OUTPUT
variable name and the dirname/"basename" approach so the check targets the same
path used for normalization.
In `@mkexfat.sh`:
- Around line 84-87: The current check only verifies TMP_BASE exists; change it
to validate that TMP_BASE is both writable and searchable/executable so mktemp
and child processes can create files there: update the directory check around
TMP_BASE to assert -d, -w and -x (or attempt a safe mktemp creation) and, on
failure, emit a clear error mentioning missing write/execute permissions for
TMP_BASE and exit 1; reference the TMP_BASE variable and the mktemp usage in the
script when implementing the check.
In `@New-OsfExfatImage.ps1`:
- Around line 140-149: The Initialize-ScriptTempDir function currently only
checks existence; update it to fail fast on non-writable directories by
attempting a safe write/delete in $resolved (e.g., create and remove a small
temp marker file) and throw a clear error if that operation fails; keep the
existing Resolve-Path, $script:ScriptTempDir assignment and env:TEMP/TMP updates
but perform the write-check before setting those and before continuing, and
ensure the thrown message mentions the directory and that it is not writable to
aid debugging.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: c0061dd0-8ac7-44cf-9c95-cd3cc12002a5
📒 Files selected for processing (4)
New-OsfExfatImage.ps1make_image.batmkexfat.shmkexfat_macos.sh
note: because of my poor english, I've wrote description using ai, and did some refactor in code using ai, I just wanted to help and improve this awesome project from my own point of view, thank you everyone who helped this project
Summary
Adds an optional temp/work directory flag across all exFAT image creation paths so large builds can use a fast or spacious disk (e.g.
/mnt/hdd,D:\temp) instead of default system temp or fixed mount paths.mkexfat.sh):-t/--tmp-dir/--temp-dirmay appear anywhere in the command line. Creates a unique mount dir under the chosen base (mkexfat.XXXXXX). Mount prefers directexfat-fusefile access and falls back to-o loopwhen needed (fixes loop setup failures on some removable exFAT targets).mkexfat_macos.sh): Same flag parsing; when omitted, behavior is unchanged (mktempdefaults). When set, sizing scratch file and mount staging dir are created under the given path.New-OsfExfatImage.ps1,make_image.bat):-TempDir/-t(PowerShell) and-t/--tmp-dir/--temp-dir(batch). Sets%TEMP%/%TMP%for format redirect files and related scratch work.Motivation
Large game/app images (~80GB+) can fail or run slowly when temp/mount work uses root-owned paths, small system partitions, or loop mounts on USB exFAT volumes. Letting users point temp work at another drive avoids those issues without changing default behavior for existing scripts.
Behavior
-t/--tmp-dir/tmp/mkexfat.*<dir>/mkexfat.*mktemp<dir>/mkexfat.*and<dir>/mkexfat_sizes.*%TEMP%Output image path is always the positional/output argument you pass; only intermediate work moves to the temp directory.
Linux —
mkexfat.shRequires:
exfatprogs,exfat-fuse,fuse3,rsync(often run with
sudofor mount operations)