Skip to content

Commit

Permalink
[crossplatform, projmgr] Handle execution of generator bridge path wi…
Browse files Browse the repository at this point in the history
…th spaces (#882)

* [crossplatform] Handle execution of quoted commands and arguments

* [projmgr] Handle execution of generator bridge path with spaces
  • Loading branch information
brondani authored Apr 29, 2024
1 parent d42179e commit 2dabe6d
Show file tree
Hide file tree
Showing 22 changed files with 55 additions and 40 deletions.
1 change: 1 addition & 0 deletions libs/crossplatform/include/CrossPlatformUtils.h
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ class CrossPlatformUtils
private:
// private constructor to prevent instantiating an utility class
CrossPlatformUtils() {};
static const std::string PopenCmd(const std::string& cmd);

public:

Expand Down
2 changes: 1 addition & 1 deletion libs/crossplatform/src/CrossPlatformUtils.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -78,7 +78,7 @@ const std::pair<std::string, int> CrossPlatformUtils::ExecCommand(const std::str

auto deleter = [&close, &ret_code](FILE* cmd) { ret_code = close(cmd); };
{
const std::unique_ptr<FILE, decltype(deleter)> pipe(open(cmd.c_str(), "r"), deleter);
const std::unique_ptr<FILE, decltype(deleter)> pipe(open(PopenCmd(cmd).c_str(), "r"), deleter);
if (pipe) {
while (fgets(buffer.data(), static_cast<int>(buffer.size()), pipe.get()) != nullptr) {
result += buffer.data();
Expand Down
5 changes: 5 additions & 0 deletions libs/crossplatform/src/Darwin/Utils.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -97,4 +97,9 @@ std::filesystem::perms CrossPlatformUtils::GetCurrentUmask() {

return perm;
}

const std::string CrossPlatformUtils::PopenCmd(const std::string& cmd) {
return cmd;
}

// end of Utils.cpp
5 changes: 5 additions & 0 deletions libs/crossplatform/src/Linux/Utils.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -104,4 +104,9 @@ std::filesystem::perms CrossPlatformUtils::GetCurrentUmask() {

return perm;
}

const std::string CrossPlatformUtils::PopenCmd(const std::string& cmd) {
return cmd;
}

// end of Utils.cpp
7 changes: 7 additions & 0 deletions libs/crossplatform/src/Windows/Utils.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -161,4 +161,11 @@ std::filesystem::perms CrossPlatformUtils::GetCurrentUmask() {

return perm;
}

// surround the whole 'cmd' with quotes since they get stripped in _popen,
// avoiding breaking the command when there are multiple quotes pairs inside it
const std::string CrossPlatformUtils::PopenCmd(const std::string& cmd) {
return "\"" + cmd + "\"";
}

// end of Utils.cpp
4 changes: 2 additions & 2 deletions libs/crossplatform/test/src/UnitTests.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -112,12 +112,12 @@ TEST(CrossPlatformUnitTests, ExecCommand) {
auto result = CrossPlatformUtils::ExecCommand("invalid command");
EXPECT_EQ(false, (0 == result.second) ? true : false) << result.first;

string testdir = "mkdir_test_dir";
string testdir = "mkdir test dir";
error_code ec;
if (filesystem::exists(testdir)) {
filesystem::remove(testdir);
}
result = CrossPlatformUtils::ExecCommand("mkdir " + testdir);
result = CrossPlatformUtils::ExecCommand("mkdir \"" + testdir + "\"");
EXPECT_TRUE(filesystem::exists(testdir));
EXPECT_EQ(true, (0 == result.second) ? true : false) << result.first;
}
Expand Down
2 changes: 1 addition & 1 deletion tools/projmgr/src/ProjMgrWorker.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -4472,7 +4472,7 @@ bool ProjMgrWorker::ExecuteExtGenerator(std::string& generatorId) {
string binDir = ProjMgrKernel::Get()->GetCmsisToolboxDir() + "/bin";
string runCmd = m_extGenerator->GetGlobalGenRunCmd(generatorId);
RteFsUtils::NormalizePath(runCmd, binDir);
runCmd += " " + fs::path(cbuildgenOutput).append(m_parser->GetCsolution().name + ".cbuild-gen-idx.yml").generic_string();
runCmd = "\"" + runCmd + "\" \"" + fs::path(cbuildgenOutput).append(m_parser->GetCsolution().name + ".cbuild-gen-idx.yml").generic_string() + "\"";
error_code ec;
const auto& workingDir = fs::current_path(ec);
fs::current_path(genDir, ec);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -28,3 +28,6 @@ solution:
for-context: +MultiCore
- project: multi_1/core1.cproject.yml
for-context: +MultiCore

output-dirs:
intdir: ./tmp dir/$Project$/$TargetType$/$BuildType$/
Original file line number Diff line number Diff line change
Expand Up @@ -2,5 +2,5 @@ generator:
- id: RteTestExternalGenerator
description: Global Registered Generator
download-url: https://raw.githubusercontent.com/Open-CMSIS-Pack
run: ./bridge.sh
run: ./bridge tool.sh
path: $SolutionDir()$/generated/$TargetType$
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
generator:
- id: RteTestExternalGenerator
download-url: https://raw.githubusercontent.com/Open-CMSIS-Pack
run: ./bridge.sh
run: ./bridge tool.sh
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ build-gen:
- ${DEVTOOLS(data)}/ExternalGenerator/multi_0/RTE/_Debug_MultiCore
- ${DEVTOOLS(packs)}/ARM/RteTest_DFP/0.2.0/Device/ARM/ARMCM0/Include
output-dirs:
intdir: ${DEVTOOLS(data)}/ExternalGenerator/tmp/core0/MultiCore/Debug
intdir: ${DEVTOOLS(data)}/ExternalGenerator/tmp dir/core0/MultiCore/Debug
outdir: ${DEVTOOLS(data)}/ExternalGenerator/out/core0/MultiCore/Debug
rtedir: ${DEVTOOLS(data)}/ExternalGenerator/multi_0/RTE
output:
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ build-gen:
- ${DEVTOOLS(data)}/ExternalGenerator/multi_1/RTE/_Debug_MultiCore
- ${DEVTOOLS(packs)}/ARM/RteTest_DFP/0.2.0/Device/ARM/ARMCM0/Include
output-dirs:
intdir: ${DEVTOOLS(data)}/ExternalGenerator/tmp/core1/MultiCore/Debug
intdir: ${DEVTOOLS(data)}/ExternalGenerator/tmp dir/core1/MultiCore/Debug
outdir: ${DEVTOOLS(data)}/ExternalGenerator/out/core1/MultiCore/Debug
rtedir: ${DEVTOOLS(data)}/ExternalGenerator/multi_1/RTE
output:
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,11 +6,11 @@ build-gen-idx:
device: RteTest_ARMCM0_Dual
project-type: multi-core
cbuild-gens:
- cbuild-gen: ${DEVTOOLS(data)}/ExternalGenerator/tmp/core0/MultiCore/Debug/core0.Debug+MultiCore.cbuild-gen.yml
- cbuild-gen: ${DEVTOOLS(data)}/ExternalGenerator/tmp dir/core0/MultiCore/Debug/core0.Debug+MultiCore.cbuild-gen.yml
project: core0
configuration: .Debug+MultiCore
for-project-part: cm0_core0
- cbuild-gen: ${DEVTOOLS(data)}/ExternalGenerator/tmp/core1/MultiCore/Debug/core1.Debug+MultiCore.cbuild-gen.yml
- cbuild-gen: ${DEVTOOLS(data)}/ExternalGenerator/tmp dir/core1/MultiCore/Debug/core1.Debug+MultiCore.cbuild-gen.yml
project: core1
configuration: .Debug+MultiCore
for-project-part: cm0_core1
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,6 @@ build-gen-idx:
device: RteTestGen_ARMCM0
project-type: single-core
cbuild-gens:
- cbuild-gen: ${DEVTOOLS(data)}/ExternalGenerator/tmp/single-core/CM0/Debug/single-core.Debug+CM0.cbuild-gen.yml
- cbuild-gen: ${DEVTOOLS(data)}/ExternalGenerator/tmp dir/single-core/CM0/Debug/single-core.Debug+CM0.cbuild-gen.yml
project: single-core
configuration: .Debug+CM0
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ build-gen:
add-path:
- ${DEVTOOLS(data)}/ExternalGenerator/single/RTE/_Debug_CM0
output-dirs:
intdir: ${DEVTOOLS(data)}/ExternalGenerator/tmp/single-core/CM0/Debug
intdir: ${DEVTOOLS(data)}/ExternalGenerator/tmp dir/single-core/CM0/Debug
outdir: ${DEVTOOLS(data)}/ExternalGenerator/out/single-core/CM0/Debug
rtedir: ${DEVTOOLS(data)}/ExternalGenerator/single/RTE
output:
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ build:
- ${CMSIS_PACK_ROOT}/ARM/RteTest/0.1.0/GlobalLevel
- ${CMSIS_PACK_ROOT}/ARM/RteTest/0.1.0/Include
output-dirs:
intdir: ../tmp/single-core/CM0/Debug
intdir: ../tmp dir/single-core/CM0/Debug
outdir: ../out/single-core/CM0/Debug
rtedir: RTE
output:
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,11 +6,11 @@ build-gen-idx:
device: RteTestGen_ARMCM0
project-type: trustzone
cbuild-gens:
- cbuild-gen: ${DEVTOOLS(data)}/ExternalGenerator/tmp/ns/CM0/Debug/ns.Debug+CM0.cbuild-gen.yml
- cbuild-gen: ${DEVTOOLS(data)}/ExternalGenerator/tmp dir/ns/CM0/Debug/ns.Debug+CM0.cbuild-gen.yml
project: ns
configuration: .Debug+CM0
for-project-part: non-secure
- cbuild-gen: ${DEVTOOLS(data)}/ExternalGenerator/tmp/s/CM0/Debug/s.Debug+CM0.cbuild-gen.yml
- cbuild-gen: ${DEVTOOLS(data)}/ExternalGenerator/tmp dir/s/CM0/Debug/s.Debug+CM0.cbuild-gen.yml
project: s
configuration: .Debug+CM0
for-project-part: secure
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ build-gen:
add-path:
- ${DEVTOOLS(data)}/ExternalGenerator/tz_ns/RTE/_Debug_CM0
output-dirs:
intdir: ${DEVTOOLS(data)}/ExternalGenerator/tmp/ns/CM0/Debug
intdir: ${DEVTOOLS(data)}/ExternalGenerator/tmp dir/ns/CM0/Debug
outdir: ${DEVTOOLS(data)}/ExternalGenerator/out/ns/CM0/Debug
rtedir: ${DEVTOOLS(data)}/ExternalGenerator/tz_ns/RTE
output:
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ build-gen:
add-path:
- ${DEVTOOLS(data)}/ExternalGenerator/tz_s/RTE/_Debug_CM0
output-dirs:
intdir: ${DEVTOOLS(data)}/ExternalGenerator/tmp/s/CM0/Debug
intdir: ${DEVTOOLS(data)}/ExternalGenerator/tmp dir/s/CM0/Debug
outdir: ${DEVTOOLS(data)}/ExternalGenerator/out/s/CM0/Debug
rtedir: ${DEVTOOLS(data)}/ExternalGenerator/tz_s/RTE
output:
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
generator:
- id: RteTestExternalGenerator
download-url: https://raw.githubusercontent.com/Open-CMSIS-Pack
run: ./bridge.sh
run: ./bridge tool.sh
path: $UnknownAccessSequence()$
38 changes: 16 additions & 22 deletions tools/projmgr/test/src/ProjMgrUnitTests.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -4859,8 +4859,8 @@ TEST_F(ProjMgrUnitTests, ExternalGenerator) {
const string& dstGlobalGenerator = etc_folder + "/global.generator.yml";
RteFsUtils::CopyCheckFile(srcGlobalGenerator, dstGlobalGenerator, false);

const string& srcBridgeTool = testinput_folder + "/ExternalGenerator/bridge.sh";
const string& dstBridgeTool = bin_folder + "/bridge.sh";
const string& srcBridgeTool = testinput_folder + "/ExternalGenerator/bridge tool.sh";
const string& dstBridgeTool = bin_folder + "/bridge tool.sh";
RteFsUtils::CopyCheckFile(srcBridgeTool, dstBridgeTool, false);

// list generators
Expand All @@ -4880,10 +4880,6 @@ TEST_F(ProjMgrUnitTests, ExternalGenerator) {
argv[6] = (char*)"core0.Debug+MultiCore";
EXPECT_EQ(0, RunProjMgr(7, argv, 0));

EXPECT_TRUE(ProjMgrYamlSchemaChecker().Validate(testinput_folder + "/ExternalGenerator/tmp/core0/MultiCore/Debug/extgen.cbuild-gen-idx.yml"));
EXPECT_TRUE(ProjMgrYamlSchemaChecker().Validate(testinput_folder + "/ExternalGenerator/tmp/core0/MultiCore/Debug/core0.Debug+MultiCore.cbuild-gen.yml"));
EXPECT_TRUE(ProjMgrYamlSchemaChecker().Validate(testinput_folder + "/ExternalGenerator/tmp/core1/MultiCore/Debug/core1.Debug+MultiCore.cbuild-gen.yml"));

auto stripAbsoluteFunc = [](const std::string& in) {
std::string str = in;
RteUtils::ReplaceAll(str, testinput_folder, "${DEVTOOLS(data)}");
Expand All @@ -4892,38 +4888,31 @@ TEST_F(ProjMgrUnitTests, ExternalGenerator) {
};

ProjMgrTestEnv::CompareFile(testinput_folder + "/ExternalGenerator/ref/MultiCore/extgen.cbuild-gen-idx.yml",
testinput_folder + "/ExternalGenerator/tmp/core0/MultiCore/Debug/extgen.cbuild-gen-idx.yml", stripAbsoluteFunc);
testinput_folder + "/ExternalGenerator/tmp dir/core0/MultiCore/Debug/extgen.cbuild-gen-idx.yml", stripAbsoluteFunc);
ProjMgrTestEnv::CompareFile(testinput_folder + "/ExternalGenerator/ref/MultiCore/core0.Debug+MultiCore.cbuild-gen.yml",
testinput_folder + "/ExternalGenerator/tmp/core0/MultiCore/Debug/core0.Debug+MultiCore.cbuild-gen.yml", stripAbsoluteFunc);
testinput_folder + "/ExternalGenerator/tmp dir/core0/MultiCore/Debug/core0.Debug+MultiCore.cbuild-gen.yml", stripAbsoluteFunc);
ProjMgrTestEnv::CompareFile(testinput_folder + "/ExternalGenerator/ref/MultiCore/core1.Debug+MultiCore.cbuild-gen.yml",
testinput_folder + "/ExternalGenerator/tmp/core1/MultiCore/Debug/core1.Debug+MultiCore.cbuild-gen.yml", stripAbsoluteFunc);
testinput_folder + "/ExternalGenerator/tmp dir/core1/MultiCore/Debug/core1.Debug+MultiCore.cbuild-gen.yml", stripAbsoluteFunc);

// run single-core
argv[6] = (char*)"single-core.Debug+CM0";
EXPECT_EQ(0, RunProjMgr(7, argv, 0));

EXPECT_TRUE(ProjMgrYamlSchemaChecker().Validate(testinput_folder + "/ExternalGenerator/tmp/single-core/CM0/Debug/extgen.cbuild-gen-idx.yml"));
EXPECT_TRUE(ProjMgrYamlSchemaChecker().Validate(testinput_folder + "/ExternalGenerator/tmp/single-core/CM0/Debug/single-core.Debug+CM0.cbuild-gen.yml"));

ProjMgrTestEnv::CompareFile(testinput_folder + "/ExternalGenerator/ref/SingleCore/extgen.cbuild-gen-idx.yml",
testinput_folder + "/ExternalGenerator/tmp/single-core/CM0/Debug/extgen.cbuild-gen-idx.yml", stripAbsoluteFunc);
testinput_folder + "/ExternalGenerator/tmp dir/single-core/CM0/Debug/extgen.cbuild-gen-idx.yml", stripAbsoluteFunc);
ProjMgrTestEnv::CompareFile(testinput_folder + "/ExternalGenerator/ref/SingleCore/single-core.Debug+CM0.cbuild-gen.yml",
testinput_folder + "/ExternalGenerator/tmp/single-core/CM0/Debug/single-core.Debug+CM0.cbuild-gen.yml", stripAbsoluteFunc);
testinput_folder + "/ExternalGenerator/tmp dir/single-core/CM0/Debug/single-core.Debug+CM0.cbuild-gen.yml", stripAbsoluteFunc);

// run trustzone
argv[6] = (char*)"ns.Debug+CM0";
EXPECT_EQ(0, RunProjMgr(7, argv, 0));

EXPECT_TRUE(ProjMgrYamlSchemaChecker().Validate(testinput_folder + "/ExternalGenerator/tmp/ns/CM0/Debug/extgen.cbuild-gen-idx.yml"));
EXPECT_TRUE(ProjMgrYamlSchemaChecker().Validate(testinput_folder + "/ExternalGenerator/tmp/ns/CM0/Debug/ns.Debug+CM0.cbuild-gen.yml"));
EXPECT_TRUE(ProjMgrYamlSchemaChecker().Validate(testinput_folder + "/ExternalGenerator/tmp/s/CM0/Debug/s.Debug+CM0.cbuild-gen.yml"));

ProjMgrTestEnv::CompareFile(testinput_folder + "/ExternalGenerator/ref/TrustZone/extgen.cbuild-gen-idx.yml",
testinput_folder + "/ExternalGenerator/tmp/ns/CM0/Debug/extgen.cbuild-gen-idx.yml", stripAbsoluteFunc);
testinput_folder + "/ExternalGenerator/tmp dir/ns/CM0/Debug/extgen.cbuild-gen-idx.yml", stripAbsoluteFunc);
ProjMgrTestEnv::CompareFile(testinput_folder + "/ExternalGenerator/ref/TrustZone/ns.Debug+CM0.cbuild-gen.yml",
testinput_folder + "/ExternalGenerator/tmp/ns/CM0/Debug/ns.Debug+CM0.cbuild-gen.yml", stripAbsoluteFunc);
testinput_folder + "/ExternalGenerator/tmp dir/ns/CM0/Debug/ns.Debug+CM0.cbuild-gen.yml", stripAbsoluteFunc);
ProjMgrTestEnv::CompareFile(testinput_folder + "/ExternalGenerator/ref/TrustZone/s.Debug+CM0.cbuild-gen.yml",
testinput_folder + "/ExternalGenerator/tmp/s/CM0/Debug/s.Debug+CM0.cbuild-gen.yml", stripAbsoluteFunc);
testinput_folder + "/ExternalGenerator/tmp dir/s/CM0/Debug/s.Debug+CM0.cbuild-gen.yml", stripAbsoluteFunc);

// convert single core
argv[2] = (char*)"convert";
Expand Down Expand Up @@ -4962,6 +4951,10 @@ TEST_F(ProjMgrUnitTests, ExternalGenerator_WrongGenDir) {
const string& dstGlobalGenerator = etc_folder + "/global.generator.yml";
RteFsUtils::CopyCheckFile(srcGlobalGenerator, dstGlobalGenerator, false);

const string& srcBridgeTool = testinput_folder + "/ExternalGenerator/bridge tool.sh";
const string& dstBridgeTool = bin_folder + "/bridge tool.sh";
RteFsUtils::CopyCheckFile(srcBridgeTool, dstBridgeTool, false);

StdStreamRedirect streamRedirect;
char* argv[8];
const string& csolution = testinput_folder + "/ExternalGenerator/extgen.csolution.yml";
Expand All @@ -4972,7 +4965,7 @@ TEST_F(ProjMgrUnitTests, ExternalGenerator_WrongGenDir) {
argv[5] = (char*)"-c";
argv[6] = (char*)"core0.Debug+MultiCore";
argv[7] = (char*)"-n";
EXPECT_EQ(1, RunProjMgr(8, argv, 0));
EXPECT_EQ(0, RunProjMgr(8, argv, 0));

const string expected = "\
warning csolution: unknown access sequence: 'UnknownAccessSequence()'\n\
Expand All @@ -4981,6 +4974,7 @@ warning csolution: unknown access sequence: 'UnknownAccessSequence()'\n\
EXPECT_TRUE(errStr.find(expected) != string::npos);

RteFsUtils::RemoveFile(dstGlobalGenerator);
RteFsUtils::RemoveFile(dstBridgeTool);
}

TEST_F(ProjMgrUnitTests, ExternalGenerator_NoGenDir) {
Expand Down

0 comments on commit 2dabe6d

Please sign in to comment.