Mac Add utility app to remove Podman#6851
Conversation
There was a problem hiding this comment.
3 issues found across 6 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="mac_RemovePodman/main.mm">
<violation number="1" location="mac_RemovePodman/main.mm:35">
P1: Misplaced closing brace causes code after line 41 to execute even when `f` is NULL. When the second `popen` fails, `path` may be uninitialized when passed to `stat()`. The closing brace should be outdented to match the `if (f) {` statement.</violation>
<violation number="2" location="mac_RemovePodman/main.mm:179">
P2: `chdir()` calls have no error checking. If they fail, subsequent `rm -Rf` commands will execute in the wrong directory, potentially causing unintended deletions.</violation>
</file>
<file name="mac_RemovePodman/Sign_Notarize_RemovePodman.sh">
<violation number="1" location="mac_RemovePodman/Sign_Notarize_RemovePodman.sh:59">
P2: Handle failures from the notarization step so the script stops instead of proceeding with an unnotarized app.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
There was a problem hiding this comment.
1 issue found across 2 files (changes from recent commits).
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="mac_RemovePodman/main.mm">
<violation number="1" location="mac_RemovePodman/main.mm:122">
P2: `err` is checked without being set, so this error check uses an uninitialized value and can skip real chdir failures or return random errors. Capture chdir’s return value before testing it.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
There was a problem hiding this comment.
1 issue found across 1 file (changes from recent commits).
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="mac_installer/uninstall.cpp">
<violation number="1" location="mac_installer/uninstall.cpp:525">
P2: The Podman removal block only executes when `podmanPath` is empty, so it skips removal when Podman is found and attempts to run with an empty path. Flip the condition to run only when a valid path is present.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
|
This PR is ready to be merged. |
|
@CharlieFenton, if you don't mind, I'll add a small addition to this PR to upload a new 'RemovePodman' application to the list of our artifacts. |
There was a problem hiding this comment.
2 issues found across 7 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="mac_build/buildMacBOINC-CI.sh">
<violation number="1" location="mac_build/buildMacBOINC-CI.sh:222">
P2: The new RemovePodman build step is unreachable in the default shared-headers path (share_paths="yes"), because the script returns before it. This means the default CI/local run won’t build RemovePodman unless --no_shared_headers is passed, which undermines the intent to include it in regular builds.</violation>
</file>
<file name="mac_RemovePodman/main.mm">
<violation number="1" location="mac_RemovePodman/main.mm:30">
P2: `DoCommand` discards the return value of `pclose()`, which contains the command's exit status. This makes all the `if (err) return err;` checks at every call site effectively dead code. Capture and return the command's exit status from `pclose`. Note: for a removal utility, you may want to log but not abort on individual command failures, so the call sites may also need adjustment.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
That should be fine. Please merge it if Cubic reports no more issues. |
There was a problem hiding this comment.
3 issues found across 7 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="mac_RemovePodman/main.mm">
<violation number="1" location="mac_RemovePodman/main.mm:43">
P2: `DoCommand` ignores the return value of `pclose(f)`, which contains the exit status of the command. The function always returns 0 on success of `popen`, so callers cannot detect command failures. Capture and return the `pclose` result (noting that `killall` failing because no processes exist may need special handling).</violation>
<violation number="2" location="mac_RemovePodman/main.mm:131">
P1: Resource leak: early `return err` inside the `while (dirp)` loop skips `closedir(dirp)` and doesn't restore euid to root. All early-return paths within this loop should call `closedir(dirp)` and `seteuid(0)` before returning. Consider using a `goto cleanup` pattern or restructuring the error handling.</violation>
<violation number="3" location="mac_RemovePodman/main.mm:283">
P2: `freopen` can return NULL on failure, and passing NULL to `setbuf` is undefined behavior (crash). Check the return value before calling `setbuf`.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
Signed-off-by: Vitalii Koshura <lestat.de.lionkur@gmail.com>
Add new utility app to completely remove Podman from a Mac computer.
It first warns the user that it will cancel any BOINC and non-BOINC Podman tasks in progress and will completely remove PODMAN from the computer and gives the user the opportunity to cancel.
If the user invokes
podman initandpodman startoutside of BOINC, Podman creates some items in the directory ~/.local/share/, creating those directories if they were not already present. This app does not delete some items from those directories because there is no way to know whether they are being shared with other processes. It does delete all items which were definitely created by podman.Summary by cubic
Adds a macOS utility app, RemovePodman, to fully remove Podman (BOINC-managed and user-installed). Integrates with local/CI builds and deployment artifacts, and updates the uninstaller to reliably find Podman, stop/remove BOINC’s Podman VM, and keep logs across privileged relaunch.
New Features
Migration
Written for commit b19847d. Summary will update on new commits.