client: show "install Podman" messages only in certain cases#7171
Open
davidpanderson wants to merge 5 commits into
Open
client: show "install Podman" messages only in certain cases#7171davidpanderson wants to merge 5 commits into
davidpanderson wants to merge 5 commits into
Conversation
Namely: - the client is attached to a project that has Docker apps - the client is using Science United Also: we had two separate chunks of code for parsing all_projects_list.xml. Combine these into one (in lib/util)
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
2 issues found across 9 files
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="lib/util.cpp">
<violation number="1" location="lib/util.cpp:1003">
P2: Each project-list read leaks every `PROJECT_LIST_ENTRY` when its `ALL_PROJECTS_LIST` is destroyed. Add a destructor that calls `clear()` (or use owning pointers) so the temporary project lists release their entries.</violation>
</file>
<file name="lib/gui_rpc_client_ops.cpp">
<violation number="1" location="lib/gui_rpc_client_ops.cpp:1555">
P3: The `if (retval) return retval;` check after `pl.parse(rpc.xp)` is dead code because `ALL_PROJECTS_LIST::parse()` unconditionally returns 0. While harmless, this is a minor cleanup opportunity — the check can be removed to make the code clearer.</violation>
</file>
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
| retval = entry->parse(xp); | ||
| if (!retval) { | ||
| entry->is_account_manager = false; | ||
| projects.push_back(entry); |
Contributor
There was a problem hiding this comment.
P2: Each project-list read leaks every PROJECT_LIST_ENTRY when its ALL_PROJECTS_LIST is destroyed. Add a destructor that calls clear() (or use owning pointers) so the temporary project lists release their entries.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At lib/util.cpp, line 1003:
<comment>Each project-list read leaks every `PROJECT_LIST_ENTRY` when its `ALL_PROJECTS_LIST` is destroyed. Add a destructor that calls `clear()` (or use owning pointers) so the temporary project lists release their entries.</comment>
<file context>
@@ -902,4 +905,130 @@ string docker_container_name(
+ retval = entry->parse(xp);
+ if (!retval) {
+ entry->is_account_manager = false;
+ projects.push_back(entry);
+ } else {
+ delete entry;
</file context>
| continue; | ||
| } | ||
| } | ||
| retval = pl.parse(rpc.xp); |
Contributor
There was a problem hiding this comment.
P3: The if (retval) return retval; check after pl.parse(rpc.xp) is dead code because ALL_PROJECTS_LIST::parse() unconditionally returns 0. While harmless, this is a minor cleanup opportunity — the check can be removed to make the code clearer.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At lib/gui_rpc_client_ops.cpp, line 1555:
<comment>The `if (retval) return retval;` check after `pl.parse(rpc.xp)` is dead code because `ALL_PROJECTS_LIST::parse()` unconditionally returns 0. While harmless, this is a minor cleanup opportunity — the check can be removed to make the code clearer.</comment>
<file context>
@@ -1655,37 +1547,13 @@ int RPC_CLIENT::get_project_status(PROJECTS& p) {
- continue;
- }
- }
+ retval = pl.parse(rpc.xp);
+ if (retval) return retval;
</file context>
Contributor
There was a problem hiding this comment.
All reported issues were addressed across 3 files (changes from recent commits).
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
Member
|
@davidpanderson, it looks like that the changed 'util.cpp' file now breaks the compilation of the 'PostInstall' project on Mac. Maybe some file is missed? Also, in the 'clint/client_state.cpp' file we still have a link to install 'Docker'. |
Contributor
Author
|
I can't figure out why the Mac build failed. Is there a more detailed error message? I'll fix the startup messages in a later PR. |
Member
|
@davidpanderson, from the build log file: |
Member
|
@davidpanderson, the same issue in 'Uninstaller': |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Namely:
Also: we had two separate chunks of code for parsing all_projects_list.xml. Combine these into one (in lib/util)
Summary by cubic
Show “Install Podman” only when it’s relevant, and consolidate
all_projects_list.xmlparsing intolib/utilto remove duplicate code and simplify lookups. Includes a small macOS build fix.New Features
dockerplatform.dont_use_dockerand, on Windows,dont_use_wsl; skip notices on unsupported Windows.Refactors
all_projects_list.xmlparsing tolib/utilasALL_PROJECTS_LISTwithparse(),read_file(),alpha_sort(), andlookup_id(); unified entries viaPROJECT_LIST_ENTRYwithidandis_account_manager.lib/gui_rpc_client*;get_all_projects_listnow usesALL_PROJECTS_LIST::parse(). Updated account attach to uselookup_id()andurl. Droppedproject_list.cppand the oldPROJECT_LISTmember.util.hinclientgui/mac/SetupSecurity.cppand rename the duplicatedtime()todtime2to fix the build.Written for commit 15b583c. Summary will update on new commits.