Skip to content

Commit

Permalink
Restrict --context-set to one identical target-type and only one buil…
Browse files Browse the repository at this point in the history
…d-type per context
  • Loading branch information
grasci-arm authored May 15, 2024
1 parent 242db89 commit 6ebf694
Show file tree
Hide file tree
Showing 9 changed files with 226 additions and 4 deletions.
2 changes: 1 addition & 1 deletion tools/projmgr/include/ProjMgr.h
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* Copyright (c) 2020-2023 Arm Limited. All rights reserved.
* Copyright (c) 2020-2024 Arm Limited. All rights reserved.
*
* SPDX-License-Identifier: Apache-2.0
*/
Expand Down
7 changes: 7 additions & 0 deletions tools/projmgr/include/ProjMgrWorker.h
Original file line number Diff line number Diff line change
Expand Up @@ -645,6 +645,13 @@ class ProjMgrWorker {
*/
void ProcessExecutesDependencies();

/**
* @brief validates and restrict the input contexts
* @param contexts list to be validated
* @param fromCbuildSet true is the contexts are read from cbuild-set, otherwise false
* @return true if validation success
*/
bool ValidateContexts(const std::vector<std::string>& contexts, bool fromCbuildSet);
protected:
ProjMgrParser* m_parser = nullptr;
ProjMgrKernel* m_kernel = nullptr;
Expand Down
8 changes: 8 additions & 0 deletions tools/projmgr/src/ProjMgr.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -541,6 +541,14 @@ bool ProjMgr::Configure() {
if (!m_worker.ParseContextSelection(m_context, checkCbuildSet)) {
return false;
}

// validate and restrict the input contexts when used with -S option
if (!m_context.empty() && m_contextSet) {
if (!m_worker.ValidateContexts(m_context, false)){
return false;
}
}

if (m_worker.HasVarDefineError()) {
auto undefVars = m_worker.GetUndefLayerVars();
string errMsg = "undefined variables in "+ fs::path(m_csolutionFile).filename().generic_string() +":\n";
Expand Down
66 changes: 66 additions & 0 deletions tools/projmgr/src/ProjMgrWorker.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -4009,6 +4009,9 @@ bool ProjMgrWorker::ParseContextSelection(
if (m_selectedToolchain.empty()) {
m_selectedToolchain = cbuildSetItem.compiler;
}
if (!ValidateContexts(m_selectedContexts, true)) {
return false;
}
}
else {
const auto& filterError = ProjMgrUtils::GetSelectedContexts(
Expand Down Expand Up @@ -4596,3 +4599,66 @@ bool ProjMgrWorker::HasVarDefineError() {
const set<string>& ProjMgrWorker::GetUndefLayerVars() {
return m_undefLayerVars;
}

bool ProjMgrWorker::ValidateContexts(const std::vector<std::string>& contexts, bool fromCbuildSet) {
if (contexts.empty()) {
return true;
}

std::string selectedTarget = "";
std::map<std::string, std::string> projectBuildTypeMap;
std::vector<std::string> errorContexts;

// Check 1: Confirm if all contexts have the same target type
for (const auto& context : contexts) {
ContextName contextItem;
ProjMgrUtils::ParseContextEntry(context, contextItem);

if (selectedTarget == "") {
selectedTarget = contextItem.target;
}
else if (contextItem.target != selectedTarget) {
errorContexts.push_back(
" target-type does not match for '" + context + "' and '" + contexts[0] + "'\n");
continue;
}
}

string contextSource = "command line";
if (fromCbuildSet) {
auto csolutionItem = m_parser->GetCsolution();
contextSource = csolutionItem.name + ".cbuild-set.yml";
}
std::string errMsg = "invalid combination of contexts specified in " + contextSource + ":\n";

if (!errorContexts.empty()) {
for (const auto& msg : errorContexts) {
errMsg += msg;
}
ProjMgrLogger::Error(errMsg);
return false;
}

// Check 2: Verify whether the project and target-type combination have conflicting build types
for (const auto& context : contexts) {
ContextName contextItem;
ProjMgrUtils::ParseContextEntry(context, contextItem);

if (auto it = projectBuildTypeMap.find(contextItem.project); it != projectBuildTypeMap.end() && it->second != contextItem.build) {
errorContexts.push_back(
" build-type is not unique for project '" + contextItem.project + "' in '" + context + "' and '" + it->first + "." + it->second + "+" + selectedTarget + "'\n");
continue;
}

projectBuildTypeMap[contextItem.project] = contextItem.build;
}

if (!errorContexts.empty()) {
for (const auto& msg : errorContexts) {
errMsg += msg;
}
ProjMgrLogger::Error(errMsg);
return false;
}
return true;
}
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,5 @@ cbuild-set:
generated-by: csolution version 2.3.0
contexts:
- context: core0.Debug+MultiCore
- context: core0.Release+MultiCore
- context: core1.Debug+MultiCore
- context: core1.Release+MultiCore
- context: boot.Debug+MultiCore
- context: boot.Release+MultiCore
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
cbuild-set:
generated-by: csolution version 2.4.0
contexts:
- context: test1.Debug+CM0
- context: test2.Debug+CM0
- context: test1.Debug+CM3
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
# yaml-language-server: $schema=https://raw.githubusercontent.com/Open-CMSIS-Pack/devtools/main/tools/projmgr/schemas/csolution.schema.json

solution:
description: test description string
target-types:
- type: CM0
device: RteTest_ARMCM0
- type: CM3
device: RteTest_ARMCM3

build-types:
- type: Debug
- type: Release

packs:
- pack: ARM::[email protected]

projects:
- project: ./TestProject2/test2.cproject.yml
- project: ./TestProject1/test1.cproject.yml
88 changes: 88 additions & 0 deletions tools/projmgr/test/src/ProjMgrUnitTests.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -5625,3 +5625,91 @@ TEST_F(ProjMgrUnitTests, TestRelativeOutputOption) {
ProjMgrTestEnv::CompareFile(outputFolder + "/solution.cbuild-idx.yml",
testinput_folder + "/TestSolution/Executes/ref/solution.cbuild-idx.yml");
}

TEST_F(ProjMgrUnitTests, TestRestrictedContextsWithContextSet_Failed_Read_From_CbuildSet) {
char* argv[6];
StdStreamRedirect streamRedirect;
const string& csolution = testinput_folder + "/TestSolution/test_restricted_contexts.csolution.yml";
const string& expectedErrMsg = "\
error csolution: invalid combination of contexts specified in test_restricted_contexts.cbuild-set.yml:\n\
target-type does not match for 'test1.Debug+CM3' and 'test1.Debug+CM0'";

argv[1] = (char*)"convert";
argv[2] = (char*)csolution.c_str();
argv[3] = (char*)"--output";
argv[4] = (char*)testoutput_folder.c_str();
argv[5] = (char*)"-S";

EXPECT_EQ(1, RunProjMgr(6, argv, 0));
auto errMsg = streamRedirect.GetErrorString();
EXPECT_NE(string::npos, errMsg.find(expectedErrMsg));
}

TEST_F(ProjMgrUnitTests, TestRestrictedContextsWithContextSet_Failed1) {
char* argv[14];
StdStreamRedirect streamRedirect;
const string& csolution = testinput_folder + "/TestSolution/test.csolution.yml";
const string& expectedErrMsg = "\
error csolution: invalid combination of contexts specified in command line:\n\
target-type does not match for 'test2.Debug+CM3' and 'test1.Debug+CM0'";

argv[1] = (char*)"convert";
argv[2] = (char*)csolution.c_str();
argv[3] = (char*)"-c";
argv[4] = (char*)"test1.Debug+CM0";
argv[5] = (char*)"-c";
argv[6] = (char*)"test1.Release+CM0";
argv[7] = (char*)"-c";
argv[8] = (char*)"test2.Debug+CM0";
argv[9] = (char*)"-c";
argv[10] = (char*)"test2.Debug+CM3";
argv[11] = (char*)"--output";
argv[12] = (char*)testoutput_folder.c_str();
argv[13] = (char*)"-S";

EXPECT_EQ(1, RunProjMgr(14, argv, 0));
auto errMsg = streamRedirect.GetErrorString();
EXPECT_NE(string::npos, errMsg.find(expectedErrMsg));
}

TEST_F(ProjMgrUnitTests, TestRestrictedContextsWithContextSet_Failed2) {
char* argv[12];
StdStreamRedirect streamRedirect;
const string& csolution = testinput_folder + "/TestSolution/test.csolution.yml";
const string& expectedErrMsg = "\
error csolution: invalid combination of contexts specified in command line:\n\
build-type is not unique for project 'test1' in 'test1.Release+CM0' and 'test1.Debug+CM0'";

argv[1] = (char*)"convert";
argv[2] = (char*)csolution.c_str();
argv[3] = (char*)"-c";
argv[4] = (char*)"test1.Debug+CM0";
argv[5] = (char*)"-c";
argv[6] = (char*)"test1.Release+CM0";
argv[7] = (char*)"-c";
argv[8] = (char*)"test2.Debug+CM0";
argv[9] = (char*)"--output";
argv[10] = (char*)testoutput_folder.c_str();
argv[11] = (char*)"-S";

EXPECT_EQ(1, RunProjMgr(12, argv, 0));
auto errMsg = streamRedirect.GetErrorString();
EXPECT_NE(string::npos, errMsg.find(expectedErrMsg));
}

TEST_F(ProjMgrUnitTests, TestRestrictedContextsWithContextSet_Pass) {
char* argv[10];
const string& csolution = testinput_folder + "/TestSolution/test.csolution.yml";

argv[1] = (char*)"convert";
argv[2] = (char*)csolution.c_str();
argv[3] = (char*)"-c";
argv[4] = (char*)"test1.Debug+CM0";
argv[5] = (char*)"-c";
argv[6] = (char*)"test2.Debug+CM0";
argv[7] = (char*)"--output";
argv[8] = (char*)testoutput_folder.c_str();
argv[9] = (char*)"-S";

EXPECT_EQ(0, RunProjMgr(10, argv, 0));
}
30 changes: 30 additions & 0 deletions tools/projmgr/test/src/ProjMgrWorkerUnitTests.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1664,3 +1664,33 @@ TEST_F(ProjMgrWorkerUnitTests, CheckBoardDeviceInLayer) {
EXPECT_EQ(CheckBoardDeviceInLayer(context, clayer), expect);
}
}

TEST_F(ProjMgrWorkerUnitTests, ValidateContexts) {
const string& errMsg1 = "\
error csolution: invalid combination of contexts specified in command line:\n\
target-type does not match for 'project1.Debug+CM1' and 'project1.Debug+AVH'\n\
target-type does not match for 'project1.Debug+CM3' and 'project1.Debug+AVH'\n\n";

const string& errMsg2 = "\
error csolution: invalid combination of contexts specified in command line:\n\
build-type is not unique for project 'project1' in 'project1.Release+AVH' and 'project1.Debug+AVH'\n\
build-type is not unique for project 'project2' in 'project2.Release+AVH' and 'project2.Debug+AVH'\n\n";

vector<std::tuple<vector<string>, bool, string>> vecTestData = {
// inputContexts, expectedRetval, expectedErrMsg
{ {"project1.Debug+AVH", "project2.Release+AVH", "project1.Debug+CM1", "project1.Debug+CM3"}, false, errMsg1},
{ {"project1.Debug+AVH", "project1.Release+AVH", "project2.Debug+AVH", "project2.Release+AVH"}, false, errMsg2},
{ {"project1.Debug+AVH", "project1.Debug+AVH", "project2.Debug+AVH", "project3.Release+AVH"}, true, ""},
{ {""}, true, ""},
};

for (const auto& [inputContexts, expectedRetval, expectedErrMsg] : vecTestData) {
string input;
StdStreamRedirect streamRedirect;
std::for_each(inputContexts.begin(), inputContexts.end(),
[&](const std::string& item) { input += item + " "; });
EXPECT_EQ(expectedRetval, ValidateContexts(inputContexts, false)) << "failed for input \"" << input << "\"";
auto errStr = streamRedirect.GetErrorString();
EXPECT_STREQ(errStr.c_str(), expectedErrMsg.c_str()) << "failed for input \"" << input << "\"";
}
}

0 comments on commit 6ebf694

Please sign in to comment.