fix(spm): recursively update submodules after checkout#7292
Conversation
There was a problem hiding this comment.
Pull request overview
This PR fixes a bug in the SPM backend where Swift packages with git submodules (e.g., apple/swift-protobuf >= 1.31.0) failed to install. The fix ensures submodules are recursively initialized after checking out a specific revision.
Key changes:
- Added
update_submodules()method to theGitstruct that runsgit submodule update --init --recursive - Integrated submodule update into the SPM backend's package cloning workflow after checkout
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| src/git.rs | Adds new update_submodules() method to handle recursive submodule initialization |
| src/backend/spm.rs | Calls update_submodules() after checking out the target revision to ensure submodules are properly initialized |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| let exec = |cmd: Expression| match cmd.stderr_to_stdout().stdout_capture().unchecked().run() | ||
| { | ||
| Ok(res) => { | ||
| if res.status.success() { | ||
| Ok(()) | ||
| } else { | ||
| Err(eyre!( | ||
| "git failed: {cmd:?} {}", | ||
| String::from_utf8(res.stdout).unwrap() | ||
| )) | ||
| } | ||
| } | ||
| Err(err) => Err(eyre!("git failed: {cmd:?} {err:#}")), | ||
| }; |
There was a problem hiding this comment.
The exec closure is duplicated from the update_ref method above (lines 73-86). Consider extracting this into a private helper method to eliminate code duplication and improve maintainability. This pattern appears to be used for executing git commands with consistent error handling.
| } else { | ||
| Err(eyre!( | ||
| "git failed: {cmd:?} {}", | ||
| String::from_utf8(res.stdout).unwrap() |
There was a problem hiding this comment.
The unwrap() call on String::from_utf8(res.stdout) could panic if the git output contains invalid UTF-8 sequences. Consider using String::from_utf8_lossy() or handling the error case explicitly to prevent potential panics.
| String::from_utf8(res.stdout).unwrap() | |
| String::from_utf8_lossy(&res.stdout) |
| repo.update_tag(revision.to_string())?; | ||
|
|
||
| // Updates submodules ensuring they match the checked-out revision | ||
| repo.update_submodules()?; |
There was a problem hiding this comment.
The new submodule update functionality lacks test coverage. Consider adding a unit test for the update_submodules method or an integration test that verifies submodules are properly initialized for SPM packages that depend on them (e.g., testing with a package like apple/swift-protobuf >= 1.31.0 which uses submodules).
|
bugbot run |
### 🚀 Features - **(java)** add created_at support to ls-remote --json by @jdx in [#7297](#7297) - **(ls-remote)** add created_at timestamps to ls-remote --json for more backends by @jdx in [#7295](#7295) - **(ls-remote)** add created_at timestamps to ls-remote --json for core plugins by @jdx in [#7294](#7294) - **(registry)** add --json flag to registry command by @jdx in [#7290](#7290) - **(ruby)** add created_at timestamps to ls-remote --json by @jdx in [#7296](#7296) ### 🐛 Bug Fixes - **(spm)** recursively update submodules after checkout by @JFej in [#7292](#7292) - prioritize raw task output over task_output setting by @skorfmann in [#7286](#7286) ### New Contributors - @skorfmann made their first contribution in [#7286](#7286) - @JFej made their first contribution in [#7292](#7292)
Description
Closes discussion #7231
The
spmbackend previously failed to install Swift packages that rely on git submodules (e.g.,apple/swift-protobuf>= 1.31.0).This PR updates
src/backend/spm.rsto explicitly initialize submodules recursively after checking out the target revision.Changes
Gitstruct insrc/git.rswithupdate_submodules().git submodule update --init --recursive.clone_package_repoinsrc/backend/spm.rsto call this method after checkout.Testing
apple/swift-protobuf@1.33.3(which previously failed).cargo test) to ensure no regressions.Note
Ensures Swift package repos update/init submodules after checking out the target revision.
src/backend/spm.rs):repo.update_tag(...), callrepo.update_submodules()to sync submodules to the checked-out revision.src/git.rs):Git::update_submodules()executinggit submodule update --init --recursivewithGIT_TERMINAL_PROMPT=0and consistent error handling/logging.Written by Cursor Bugbot for commit 7d22065. This will update automatically on new commits. Configure here.