Add local docker-based build support and update documentation#43
Add local docker-based build support and update documentation#43RenanGBarreto wants to merge 4 commits into
Conversation
📝 WalkthroughWalkthroughAdds a Docker-driven build script (build.sh), a "Building from source" README section for direct/Docker/CI builds, .gitignore entries for IDE directories and a local shared-object artifact, and small PFS mount/image-type code adjustments. ChangesBuild System & Documentation
Sequence DiagramsequenceDiagram
participant Host
participant DockerDaemon
participant BuildImage
participant BuildContainer
participant Make
participant HostFS
Host->>DockerDaemon: docker pull $SMP_DOCKER_IMAGE
DockerDaemon->>BuildImage: build or reuse smp-build-cache:<tag>
Host->>DockerDaemon: docker run --user UID:GID -v repo:/workspace smp-build-cache:<tag>
DockerDaemon->>BuildContainer: start container with mounted /workspace
BuildContainer->>Make: run `make clean all` with PS5_SCE_STUBS_DIR
Make->>HostFS: write shadowmountplus.elf to mounted workspace
BuildContainer->>Host: exit and expose artifact path
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
🚥 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)
⚔️ Resolve merge conflicts
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 Infer (1.2.0)src/sm_image.csrc/sm_image.c:1:10: fatal error: 'sm_platform.h' file not found ... [truncated 682 characters] ... "/opt/infer-linux-x86_64-v1.2.0/lib/infer/facebook-clang-plugins/clang/install/lib/clang/18/include" 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. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 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 @.gitignore:
- Around line 11-12: The .gitignore currently ignores /pacbrew-repo/ and
/src/libkernel_sys_ext.so but not the SDK checkout; update the .gitignore to
also ignore the PS5 SDK checkout directory by adding an entry for ps5-sdk/
(e.g., ps5-sdk/ or /ps5-sdk/) so the directory created by build.sh (ps5-sdk) is
treated as untracked and not shown after Docker builds.
In `@build.sh`:
- Around line 80-93: The cloned /workspace/pacbrew-repo and /workspace/ps5-sdk
are created as root but makepkg is executed under su - "$BUILDER_USER", causing
permission errors; fix by ensuring the repo directories are owned by
BUILDER_USER before running makepkg (either chown -R
"$BUILDER_USER":"$BUILDER_USER" /workspace/pacbrew-repo and /workspace/ps5-sdk
after each clone, or perform the git clone as the BUILDER_USER), so that su -
"$BUILDER_USER" -c 'cd /workspace/pacbrew-repo/sdk && makepkg ...' and the
subsequent makepkg in /workspace/pacbrew-repo/sqlite can write build artifacts
successfully.
🪄 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: 3c7feb30-99c8-4c88-a2c1-691dac273af0
📒 Files selected for processing (3)
.gitignoreREADME.mdbuild.sh
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (2)
README.md (1)
340-344: 💤 Low valueAdd a language to the fenced code block.
The example block on Line 342 has no language hint (markdownlint MD040). Use
bashfor consistency with the other examples in this section.📝 Proposed fix
**Optional:** - `SMP_FORCE_REBUILD_IMAGE=1` to rebuild the cached Docker image before building - ``` + ```bash SMP_FORCE_REBUILD_IMAGE=1 ./build.sh</details> <details> <summary>🤖 Prompt for AI Agents</summary>Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.In
@README.mdaround lines 340 - 344, The fenced code block showing the example
"SMP_FORCE_REBUILD_IMAGE=1 ./build.sh" is missing a language hint; update that
code fence to use bash by changing the opening fence fromtobash so the
block becomesbash\nSMP_FORCE_REBUILD_IMAGE=1 ./build.sh\n(ensure
spacing/indentation matches surrounding examples).</details> </blockquote></details> <details> <summary>build.sh (1)</summary><blockquote> `89-91`: _⚖️ Poor tradeoff_ **Pin SDK/SQLite sources for reproducibility.** `--depth 1` clones track the moving default branch, so the cached image content varies over time and across rebuilds — at odds with the PR's reproducible-build goal. Consider pinning to a tag or commit SHA (e.g., `git clone --depth 1 --branch <tag> ...` or clone + `git checkout <sha>`). <details> <summary>🤖 Prompt for AI Agents</summary> ``` Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@build.sh` around lines 89 - 91, The three RUN git clone lines (cloning pacbrew-repo and sdk with --depth 1) should be made reproducible by pinning to a specific tag or commit: replace the shallow moving-branch clones with either git clone --depth 1 --branch <tag-or-release> ... or keep the clone and immediately run git checkout <commit-sha> (and optionally git reset --hard) for both https://github.com/ps5-payload-dev/pacbrew-repo and https://github.com/ps5-payload-dev/sdk; keep the subsequent chown -R smpbuilder:smpbuilder /opt/pacbrew-repo /opt/ps5-sdk as-is. Ensure the chosen tags or SHAs are inserted into the appropriate RUN lines so rebuilds are deterministic. ``` </details> </blockquote></details> </blockquote></details> <details> <summary>🤖 Prompt for all review comments with AI agents</summary>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@build.sh:
- Around line 83-97: The FORCE rebuild logic currently calls docker build for
LOCAL_BUILD_IMAGE but reuses cache so cloned SDK/sqlite layers aren't refreshed;
modify the branch where FORCE_REBUILD_IMAGE (or SMP_FORCE_REBUILD_IMAGE) is true
so the docker build invocation for LOCAL_BUILD_IMAGE adds the --no-cache flag
(only when the force condition is set) to ensure all layers (the RUN git clone /
makepkg steps) are rebuilt and fresh sources are fetched.- Around line 104-119: The build script sets
PS5_SCE_STUBS_DIR="/opt/ps5-sdk/sce_stubs" inside BUILD_CMD and then runs "make
clean all", but the Makefile requires $(PS5_SCE_STUBS_DIR)/libkernel_sys.c; add
a pre-flight check or stub population step inside BUILD_CMD (before invoking
make clean all) to either generate/populate the sce_stubs (using the upstream
stubs generator or copying populated stubs into /opt/ps5-sdk/sce_stubs) or fail
fast with a clear error if /opt/ps5-sdk/sce_stubs/libkernel_sys.c is missing;
update BUILD_CMD (and/or the Docker image build) so that PS5_SCE_STUBS_DIR
points to a directory that contains libkernel_sys.c before running make.
Nitpick comments:
In@build.sh:
- Around line 89-91: The three RUN git clone lines (cloning pacbrew-repo and sdk
with --depth 1) should be made reproducible by pinning to a specific tag or
commit: replace the shallow moving-branch clones with either git clone --depth 1
--branch ... or keep the clone and immediately run git checkout
(and optionally git reset --hard) for both
https://github.com/ps5-payload-dev/pacbrew-repo and
https://github.com/ps5-payload-dev/sdk; keep the subsequent chown -R
smpbuilder:smpbuilder /opt/pacbrew-repo /opt/ps5-sdk as-is. Ensure the chosen
tags or SHAs are inserted into the appropriate RUN lines so rebuilds are
deterministic.In
@README.md:
- Around line 340-344: The fenced code block showing the example
"SMP_FORCE_REBUILD_IMAGE=1 ./build.sh" is missing a language hint; update that
code fence to use bash by changing the opening fence fromtobash so the
block becomesbash\nSMP_FORCE_REBUILD_IMAGE=1 ./build.sh\n(ensure
spacing/indentation matches surrounding examples).</details> <details> <summary>🪄 Autofix (Beta)</summary> Fix all unresolved CodeRabbit comments on this PR: - [ ] <!-- {"checkboxId": "4b0d0e0a-96d7-4f10-b296-3a18ea78f0b9"} --> Push a commit to this branch (recommended) - [ ] <!-- {"checkboxId": "ff5b1114-7d8c-49e6-8ac1-43f82af23a33"} --> Create a new PR with the fixes </details> --- <details> <summary>ℹ️ Review info</summary> <details> <summary>⚙️ Run configuration</summary> **Configuration used**: defaults **Review profile**: CHILL **Plan**: Pro Plus **Run ID**: `aee88ed0-297b-4fb7-bb5c-0f67165f1106` </details> <details> <summary>📥 Commits</summary> Reviewing files that changed from the base of the PR and between cc4d33dd7da3871bcc6855db0e76b52236c6a4b3 and c5572165a239c39b9a2fadb6e62d5834b19105f3. </details> <details> <summary>📒 Files selected for processing (3)</summary> * `.gitignore` * `README.md` * `build.sh` </details> <details> <summary>💤 Files with no reviewable changes (1)</summary> * .gitignore </details> </details> <!-- This is an auto-generated comment by CodeRabbit for review status -->
|
I've made a new commit to address the bot's suggestions. I believe this feature is ready to be merged, so I’ll move on to the next one. |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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 `@include/sm_mount_defs.h`:
- Line 61: The comment documenting default mkeymode is inconsistent with the
code: the comment says "AC" but the constant PFS_MOUNT_MKEYMODE is set to
DEVPFS_MKEYMODE_GD (which maps to "GD"); update one to match the other. Either
change the comment to document "GD" to reflect the current definition of
PFS_MOUNT_MKEYMODE/DEVPFS_MKEYMODE_GD, or if "AC" was intended, change
PFS_MOUNT_MKEYMODE to use the DEVPFS_MKEYMODE_AC symbol instead so the code and
comment agree.
🪄 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: 30076350-32c4-4007-8df1-544a508621c6
📒 Files selected for processing (4)
README.mdbuild.shinclude/sm_mount_defs.hsrc/sm_image.c
✅ Files skipped from review due to trivial changes (1)
- README.md
🚧 Files skipped from review as they are similar to previous changes (1)
- build.sh
| // PFS nmount key/value variants observed in refs: | ||
| // - fstype: "pfs", "transaction_pfs", "ppr_pfs" | ||
| // - mkeymode: "SD" | ||
| // - mkeymode: "AC" |
There was a problem hiding this comment.
Documentation inconsistency: comment claims "AC" but code uses "GD".
The comment on line 61 documents the default mkeymode as "AC", but line 72 defines PFS_MOUNT_MKEYMODE as DEVPFS_MKEYMODE_GD (which resolves to "GD" per line 69). These are distinct master key modes.
📝 Proposed fix to align documentation with code
-// - mkeymode: "AC"
+// - mkeymode: "GD"If "AC" was the intended mode, then line 72 should be changed instead:
-#define PFS_MOUNT_MKEYMODE DEVPFS_MKEYMODE_GD
+#define PFS_MOUNT_MKEYMODE DEVPFS_MKEYMODE_AC📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| // - mkeymode: "AC" | |
| // - mkeymode: "GD" |
🤖 Prompt for 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.
In `@include/sm_mount_defs.h` at line 61, The comment documenting default mkeymode
is inconsistent with the code: the comment says "AC" but the constant
PFS_MOUNT_MKEYMODE is set to DEVPFS_MKEYMODE_GD (which maps to "GD"); update one
to match the other. Either change the comment to document "GD" to reflect the
current definition of PFS_MOUNT_MKEYMODE/DEVPFS_MKEYMODE_GD, or if "AC" was
intended, change PFS_MOUNT_MKEYMODE to use the DEVPFS_MKEYMODE_AC symbol instead
so the code and comment agree.
RenanGBarreto
force-pushed
the
docker-build-support
branch
from
4ec1392 to
b6f4316
Compare
1 participant
This PR adds a local docker-based build workflow and aligns documentation with the current build paths.
The problem
Sometimes depevelopers can't easily install the required dependencies in order to use make.
Proposed solution
Adds a
build.shthat fetches all necessary files and builds the correct payloads.Why
What changed
build.sh## Buildingsection inside READMEResult
You can now build locally with:
Additional information
I am working on other pull requests that will be submited soon related to PFS image mount.
If you have a specif way you want me to send the PRs or information, please let me know.
Summary by CodeRabbit
Chores
Documentation
Bug Fixes