Parsing full bash output is unreliable cross-platform - running inlin…

…e is easier. But, this _still_ doesn't allow us to fail with existing semantics Signed-off-by: Emerson Knapp <eknapp@amazon.com>
ros-tooling · Dec 31, 2020 · 8e956e0 · 8e956e0
1 parent 61234b7
commit 8e956e0
Show file tree

Hide file tree

Showing 3 changed files with 25 additions and 29 deletions.
diff --git a/__tests__/ros-ci.test.ts b/__tests__/ros-ci.test.ts
@@ -26,11 +26,11 @@ describe("execBashCommand test suite", () => {
 		expect(mockGroup).toBeCalled();
 		expect(result).not.toEqual(0);
 	});
-	it("splits output lines in checkOutput", async () => {
-		const lines = await execBashCheckOutput('echo hi; echo hi');
-		console.log(lines);
-		expect(lines.length).toEqual(3); // newline at end of output adds 1
-	})
+	// it("splits output lines in checkOutput", async () => {
+	// 	const lines = await execBashCheckOutput('echo hi; echo hi');
+	// 	console.log(lines);
+	// 	expect(lines.length).toEqual(3); // newline at end of output adds 1
+	// });
 });
 
 describe("validate distribution test", () => {

diff --git a/dist/index.js b/dist/index.js
@@ -10624,13 +10624,13 @@ function validateDistros(ros1Distro, ros2Distro) {
     return true;
 }
 exports.validateDistros = validateDistros;
-function installRosdeps(fromPaths, distro, options) {
+function installRosdeps(upToPackages, distro, options) {
     return __awaiter(this, void 0, void 0, function* () {
         const prefixVars = [
             "DEBIAN_FRONTEND=noninteractive",
             "RTI_NC_LICENSE_ACCEPTED=yes"
         ].join(" ");
-        return yield execBashCommand(`rosdep install -r --from-paths ${fromPaths.join(" ")} --ignore-src --rosdistro ${distro} -y || true`, prefixVars, options);
+        return yield execBashCommand(`rosdep install -r --from-paths $(colcon list -p --packages-up-to ${upToPackages}) --ignore-src --rosdistro ${distro} -y`, prefixVars, options);
     });
 }
 function run() {
@@ -10643,8 +10643,7 @@ function run() {
             const extraCmakeArgs = core.getInput("extra-cmake-args");
             const colconExtraArgs = core.getInput("colcon-extra-args");
             const importToken = core.getInput("import-token");
-            const packageName = core.getInput("package-name", { required: true });
-            const packageNameList = packageName.split(RegExp("\\s"));
+            const packageNames = core.getInput("package-name", { required: true }).split(RegExp("\\s")).join(" ");
             const rosWorkspaceName = "ros_ws";
             core.setOutput("ros-workspace-directory-name", rosWorkspaceName);
             const rosWorkspaceDir = path.join(workspace, rosWorkspaceName);
@@ -10710,13 +10709,12 @@ function run() {
             yield execBashCommand("vcs import --force --recursive src/ < package.repo", undefined, options);
             // Install ROS dependencies for each distribution being sourced
             // Only install dependencies for the specific paths of the packages being built, not necessarily every package in the workspace
-            const spaceSeparatedPackages = packageNameList.join(" ");
-            const desiredPackagePaths = yield execBashCheckOutput(`colcon list -p --packages-up-to ${spaceSeparatedPackages}`);
+            // const desiredPackagePaths = await execBashCheckOutput(`colcon list -p --packages-up-to ${packageNames}`);
             if (targetRos1Distro) {
-                installRosdeps(desiredPackagePaths, targetRos1Distro, options);
+                installRosdeps(packageNames, targetRos1Distro, options);
             }
             if (targetRos2Distro) {
-                installRosdeps(desiredPackagePaths, targetRos2Distro, options);
+                installRosdeps(packageNames, targetRos2Distro, options);
             }
             if (colconMixinName !== "" && colconMixinRepo !== "") {
                 yield execBashCommand(`colcon mixin add default '${colconMixinRepo}'`);
@@ -10771,7 +10769,7 @@ function run() {
             let colconBuildCmd = [
                 `colcon build`,
                 `--event-handlers console_cohesion+`,
-                `--packages-up-to ${packageNameList.join(" ")}`,
+                `--packages-up-to ${packageNames}`,
                 `${extra_options.join(" ")}`,
                 `--cmake-args ${extraCmakeArgs}`,
             ].join(" ");
@@ -10791,23 +10789,23 @@ function run() {
                 `--event-handlers console_cohesion+`,
                 `--pytest-with-coverage`,
                 `--return-code-on-test-failure`,
-                `--packages-select ${packageNameList.join(" ")}`,
+                `--packages-select ${packageNames}`,
                 `${extra_options.join(" ")}`,
             ].join(" ");
             yield execBashCommand(colconTestCmd, colconCommandPrefix, options);
             // ignoreReturnCode, check comment above in --initial
             const colconLcovResultCmd = [
                 `colcon lcov-result`,
                 `--filter ${coverageIgnorePattern}`,
-                `--packages-select ${packageNameList.join(" ")}`,
+                `--packages-select ${packageNames}`,
             ].join(" ");
             yield execBashCommand(colconLcovResultCmd, colconCommandPrefix, {
                 cwd: rosWorkspaceDir,
                 ignoreReturnCode: true,
             });
             const colconCoveragepyResultCmd = [
                 `colcon coveragepy-result`,
-                `--packages-select ${packageNameList.join(" ")}`,
+                `--packages-select ${packageNames}`,
             ].join(" ");
             yield execBashCommand(colconCoveragepyResultCmd, colconCommandPrefix, options);
             core.setOutput("ros-workspace-directory-name", rosWorkspaceName);

diff --git a/src/action-ros-ci.ts b/src/action-ros-ci.ts
@@ -166,7 +166,7 @@ export function validateDistros(
 }
 
 async function installRosdeps(
-	fromPaths: string[],
+	upToPackages: string,
 	distro: string,
 	options?: im.ExecOptions
 ): Promise<number> {
@@ -175,7 +175,7 @@ async function installRosdeps(
 		"RTI_NC_LICENSE_ACCEPTED=yes"
 	].join(" ");
 	return await execBashCommand(
-		`rosdep install -r --from-paths ${fromPaths.join(" ")} --ignore-src --rosdistro ${distro} -y || true`,
+		`rosdep install -r --from-paths $(colcon list -p --packages-up-to ${upToPackages}) --ignore-src --rosdistro ${distro} -y`,
 		prefixVars,
 		options
 	);
@@ -191,8 +191,7 @@ async function run() {
 		const extraCmakeArgs = core.getInput("extra-cmake-args");
 		const colconExtraArgs = core.getInput("colcon-extra-args");
 		const importToken = core.getInput("import-token");
-		const packageName = core.getInput("package-name", { required: true });
-		const packageNameList = packageName.split(RegExp("\\s"));
+		const packageNames = core.getInput("package-name", { required: true }).split(RegExp("\\s")).join(" ");
 		const rosWorkspaceName = "ros_ws";
 		core.setOutput("ros-workspace-directory-name", rosWorkspaceName);
 		const rosWorkspaceDir = path.join(workspace, rosWorkspaceName);
@@ -283,13 +282,12 @@ async function run() {
 
 		// Install ROS dependencies for each distribution being sourced
 		// Only install dependencies for the specific paths of the packages being built, not necessarily every package in the workspace
-		const spaceSeparatedPackages = packageNameList.join(" ");
-		const desiredPackagePaths = await execBashCheckOutput(`colcon list -p --packages-up-to ${spaceSeparatedPackages}`);
+		// const desiredPackagePaths = await execBashCheckOutput(`colcon list -p --packages-up-to ${packageNames}`);
 		if (targetRos1Distro) {
-			installRosdeps(desiredPackagePaths, targetRos1Distro, options);
+			installRosdeps(packageNames, targetRos1Distro, options);
 		}
 		if (targetRos2Distro) {
-			installRosdeps(desiredPackagePaths, targetRos2Distro, options);
+			installRosdeps(packageNames, targetRos2Distro, options);
 		}
 
 		if (colconMixinName !== "" && colconMixinRepo !== "") {
@@ -348,7 +346,7 @@ async function run() {
 		let colconBuildCmd = [
 			`colcon build`,
 			`--event-handlers console_cohesion+`,
-			`--packages-up-to ${packageNameList.join(" ")}`,
+			`--packages-up-to ${packageNames}`,
 			`${extra_options.join(" ")}`,
 			`--cmake-args ${extraCmakeArgs}`,
 		].join(" ");
@@ -370,7 +368,7 @@ async function run() {
 			`--event-handlers console_cohesion+`,
 			`--pytest-with-coverage`,
 			`--return-code-on-test-failure`,
-			`--packages-select ${packageNameList.join(" ")}`,
+			`--packages-select ${packageNames}`,
 			`${extra_options.join(" ")}`,
 		].join(" ");
 		await execBashCommand(colconTestCmd, colconCommandPrefix, options);
@@ -379,7 +377,7 @@ async function run() {
 		const colconLcovResultCmd = [
 			`colcon lcov-result`,
 			`--filter ${coverageIgnorePattern}`,
-			`--packages-select ${packageNameList.join(" ")}`,
+			`--packages-select ${packageNames}`,
 		].join(" ");
 		await execBashCommand(colconLcovResultCmd, colconCommandPrefix, {
 			cwd: rosWorkspaceDir,
@@ -388,7 +386,7 @@ async function run() {
 
 		const colconCoveragepyResultCmd = [
 			`colcon coveragepy-result`,
-			`--packages-select ${packageNameList.join(" ")}`,
+			`--packages-select ${packageNames}`,
 		].join(" ");
 		await execBashCommand(
 			colconCoveragepyResultCmd,