Skip to content
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions tools/projmgr/include/ProjMgr.h
Original file line number Diff line number Diff line change
Expand Up @@ -88,6 +88,12 @@ class ProjMgr {
bool RunConvert(const std::string&csolution = RteUtils::EMPTY_STRING,
const std::string& activeTargetSet = RteUtils::EMPTY_STRING, const bool& updateRte = false);

/**
* @brief call function to (re-)generate cbuild-idx.yml
* @return true if executed successfully
*/
bool CallGenerateCbuildIndex();

protected:
/**
* @brief parse command line options
Expand Down
30 changes: 19 additions & 11 deletions tools/projmgr/src/ProjMgr.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -657,9 +657,9 @@ bool ProjMgr::Configure() {

if (m_worker.HasVarDefineError()) {
auto undefVars = m_worker.GetUndefLayerVars();
string errMsg = "undefined variables in "+ fs::path(m_csolutionFile).filename().generic_string() +":\n";
string errMsg = "undefined variables in "+ fs::path(m_csolutionFile).filename().generic_string() +":";
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lines 660-665 are exact copies of the lines 1099-1104
Could you please create a function for that?

for (const string& var : undefVars) {
errMsg += " - $" + var + "$\n";
errMsg += "\n - $" + var + "$";
}
ProjMgrLogger::Get().Error(errMsg);
}
Expand Down Expand Up @@ -1097,9 +1097,9 @@ bool ProjMgr::RunListLayers(void) {
bool error = m_worker.HasVarDefineError() && !m_updateIdx;
if (error) {
auto undefVars = m_worker.GetUndefLayerVars();
string errMsg = "undefined variables in " + fs::path(m_csolutionFile).filename().generic_string() + ":\n";
string errMsg = "undefined variables in " + fs::path(m_csolutionFile).filename().generic_string() + ":";
for (const string& var : undefVars) {
errMsg += " - $" + var + "$\n";
errMsg += "\n - $" + var + "$";
}
ProjMgrLogger::Get().Error(errMsg);
}
Expand All @@ -1123,13 +1123,8 @@ bool ProjMgr::RunListLayers(void) {
// Update the cbuild-idx.yml file with layer information
if (m_updateIdx) {
// Generate Cbuild index
const auto& processedContexts = m_worker.GetProcessedContexts();
if (!processedContexts.empty()) {
m_worker.ElaborateVariablesConfigurations();
if (!m_emitter.GenerateCbuildIndex(processedContexts,
m_failedContext, map<string, ExecutesItem>())) {
return false;
}
if (!CallGenerateCbuildIndex()) {
return false;
}
}
return !error;
Expand Down Expand Up @@ -1373,3 +1368,16 @@ bool ProjMgr::IsSolutionImageOnly() {
}
return imageOnly;
}

bool ProjMgr::CallGenerateCbuildIndex() {
const auto& processedContexts = m_worker.GetProcessedContexts();
if (!processedContexts.empty()) {
m_worker.ElaborateVariablesConfigurations();
if (!m_emitter.GenerateCbuildIndex(processedContexts,
m_failedContext, map<string, ExecutesItem>())) {
return false;
}
}
return true;
Copy link

Copilot AI Mar 31, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This new method ends with a tab-indented return true;, which is inconsistent with the surrounding space indentation in this file. Please replace the tab with spaces to keep formatting consistent.

Suggested change
return true;
return true;

Copilot uses AI. Check for mistakes.
}

4 changes: 4 additions & 0 deletions tools/projmgr/src/ProjMgrRpcServer.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -826,13 +826,17 @@ RpcArgs::DiscoverLayersInfo RpcHandler::DiscoverLayers(const string& solution, c
}
if(!m_manager.SetupContexts(csolutionFile, activeTarget)) {
result.message = "Setup of solution contexts failed";
ProjMgrLogger::Get().Error(result.message.value());
m_manager.CallGenerateCbuildIndex();
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure if .cbuild-idx.yml should be generated here as a side effect.
Immediately before DiscoverLayers, ConvertSolution is called and generates .cbuild-idx.yml

Copy link
Copy Markdown
Contributor Author

@jthuangarm jthuangarm Mar 31, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I did some experiments on this. Previously, I had a working approach where I only appended new messages (errors and warnings) to the existing .cbuild-idx.yml generated by ConvertSolution. This allowed both the original messages and the new discoveryLayer errors to be visible in the the file and in the frontend.

messages:
  errors:
    - "undefined variables in Network, csolution. yml: \n - $Board-Layer$" 
    - No compatible software layer found. Review required connections of the project 

However, after discussing this with Joachim, he pointed out that the first error message can be misleading. In that case, regenerating the .cbuild-idx.yml and showing only the layer discovery error (i.e. the second message) might be more appropriate.

Please let me know what you think about it. We could also create a function to overwrite the messages, instead of appending or regenerating them.

Comment on lines 828 to +830
Copy link

Copilot AI Mar 31, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The indentation in this block uses tab characters (e.g., before the ProjMgrLogger call). The surrounding code in this file is consistently space-indented, so please replace the tabs with spaces to match the project’s formatting conventions.

Copilot uses AI. Check for mistakes.
return result;
Comment on lines +829 to 831
Copy link

Copilot AI Mar 31, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

CallGenerateCbuildIndex() returns false if cbuild-idx generation fails, but the return value is ignored here. Please handle the failure (e.g., log an additional error / adjust result.message) so index-generation errors don’t get silently swallowed.

Copilot uses AI. Check for mistakes.
}
m_worker.SetUpCommand(true);
StrVec layers;
StrSet fails;
if(!m_worker.ListLayers(layers, "", fails) || !m_worker.ElaborateVariablesConfigurations()) {
result.message = "No compatible software layer found. Review required connections of the project";
ProjMgrLogger::Get().Error(result.message.value());
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The same as at line 829

m_manager.CallGenerateCbuildIndex();
Comment on lines 836 to +839
Copy link

Copilot AI Mar 31, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The indentation in this block uses tab characters (before the ProjMgrLogger call). The rest of the file appears to use spaces for indentation, so please convert these tabs to spaces for consistent formatting.

Copilot uses AI. Check for mistakes.
return result;
Comment on lines +838 to 840
Copy link

Copilot AI Mar 31, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

CallGenerateCbuildIndex() can fail and returns false, but the result is ignored. Please check the return value and propagate/report a failure so callers can distinguish “layer discovery failed” from “also failed to update cbuild-idx.yml/log messages”.

Copilot uses AI. Check for mistakes.
} else {
// retrieve valid configurations
Expand Down
2 changes: 1 addition & 1 deletion tools/projmgr/test/src/ProjMgrUnitTests.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -4415,7 +4415,7 @@ TEST_F(ProjMgrUnitTests, RunCheckContextProcessing) {
EXPECT_EQ(2, RunProjMgr(8, argv, 0));

// Check error for processed context
const string expected = "error csolution: undefined variables in contexts.csolution.yml:\n - $LayerVar$\n\n";
const string expected = "error csolution: undefined variables in contexts.csolution.yml:\n - $LayerVar$\n";
auto errStr = streamRedirect.GetErrorString();
EXPECT_TRUE(errStr.find(expected) != string::npos);

Expand Down
Loading