Instead of fixing that crazy command, take a better approach

Signed-off-by: Emerson Knapp <eknapp@amazon.com>
ros-tooling · Dec 31, 2020 · 61234b7 · 61234b7
1 parent 593ad01
commit 61234b7
Show file tree

Hide file tree

Showing 3 changed files with 78 additions and 35 deletions.
diff --git a/__tests__/ros-ci.test.ts b/__tests__/ros-ci.test.ts
@@ -1,6 +1,6 @@
 import * as core from "@actions/core";
 import * as actionRosCi from "../src/action-ros-ci";
-import { execBashCommand } from "../src/action-ros-ci";
+import { execBashCommand, execBashCheckOutput } from "../src/action-ros-ci";
 
 jest.setTimeout(20000); // in milliseconds
 
@@ -26,6 +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
+	})
 });
 
 describe("validate distribution test", () => {

diff --git a/dist/index.js b/dist/index.js
@@ -10485,7 +10485,7 @@ var __importDefault = (this && this.__importDefault) || function (mod) {
     return (mod && mod.__esModule) ? mod : { "default": mod };
 };
 Object.defineProperty(exports, "__esModule", ({ value: true }));
-exports.validateDistros = exports.execBashCommand = void 0;
+exports.validateDistros = exports.execBashCheckOutput = exports.execBashCommand = void 0;
 const core = __importStar(__webpack_require__(2186));
 const github = __importStar(__webpack_require__(5438));
 const tr = __importStar(__webpack_require__(8159));
@@ -10592,6 +10592,21 @@ function execBashCommand(commandLine, commandPrefix, options, log_message) {
     });
 }
 exports.execBashCommand = execBashCommand;
+/**
+ * Execute a bash command, throwing on nonzero exit status, and returning the stdout as a string.
+ */
+function execBashCheckOutput(commandLine, options, logMessage) {
+    return __awaiter(this, void 0, void 0, function* () {
+        const stdoutLines = [];
+        yield execBashCommand(commandLine, undefined, Object.assign(Object.assign({}, options), { listeners: {
+                stdout: (data) => {
+                    stdoutLines.push(...data.toString('utf8').split('\n'));
+                }
+            } }), logMessage);
+        return stdoutLines;
+    });
+}
+exports.execBashCheckOutput = execBashCheckOutput;
 //Determine whether all inputs name supported ROS distributions.
 function validateDistros(ros1Distro, ros2Distro) {
     if (!ros1Distro && !ros2Distro) {
@@ -10609,6 +10624,15 @@ function validateDistros(ros1Distro, ros2Distro) {
     return true;
 }
 exports.validateDistros = validateDistros;
+function installRosdeps(fromPaths, 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);
+    });
+}
 function run() {
     return __awaiter(this, void 0, void 0, function* () {
         try {
@@ -10684,20 +10708,15 @@ function run() {
     version: '${commitRef}'`;
             fs_1.default.writeFileSync(repoFilePath, repoFileContent);
             yield execBashCommand("vcs import --force --recursive src/ < package.repo", undefined, options);
-            // Remove all repositories the package under test does not depend on, to
-            // avoid having rosdep installing unrequired dependencies.
-            const spaceSeparatedPackages = packageNameList.join(" ");
-            // Run the colcon list commands outside of diff, so that if they fail (due to a misspelled package perhaps),
-            // then the command will exit early. See issue #364
-            yield execBashCommand(`all_packages=$(colcon list -p) &&
-			desired_packages=$(colcon list -p --packages-up-to ${spaceSeparatedPackages}) &&
-			diff --new-line-format="" --unchanged-line-format="" <(echo $all_packages) <(echo $desired_packages) | xargs rm -rf`, 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}`);
             if (targetRos1Distro) {
-                yield execBashCommand(`DEBIAN_FRONTEND=noninteractive RTI_NC_LICENSE_ACCEPTED=yes rosdep install -r --from-paths src --ignore-src --rosdistro ${targetRos1Distro} -y || true`, undefined, options);
+                installRosdeps(desiredPackagePaths, targetRos1Distro, options);
             }
             if (targetRos2Distro) {
-                yield execBashCommand(`DEBIAN_FRONTEND=noninteractive RTI_NC_LICENSE_ACCEPTED=yes rosdep install -r --from-paths src --ignore-src --rosdistro ${targetRos2Distro} -y || true`, undefined, options);
+                installRosdeps(desiredPackagePaths, targetRos2Distro, options);
             }
             if (colconMixinName !== "" && colconMixinRepo !== "") {
                 yield execBashCommand(`colcon mixin add default '${colconMixinRepo}'`);

diff --git a/src/action-ros-ci.ts b/src/action-ros-ci.ts
@@ -118,6 +118,27 @@ export async function execBashCommand(
 	});
 }
 
+/**
+ * Execute a bash command, throwing on nonzero exit status, and returning the stdout as a string.
+ */
+export async function execBashCheckOutput(
+	commandLine: string,
+	options?: im.ExecOptions,
+	logMessage?: string
+): Promise<Array<string>> {
+	const stdoutLines: Array<string> = [];
+	await execBashCommand(commandLine, undefined, {
+		...options,
+		listeners: {
+			stdout: (data: Buffer) => {
+				stdoutLines.push(...data.toString('utf8').split('\n'));
+			}
+		}
+	}, logMessage);
+	return stdoutLines;
+}
+
+
 //Determine whether all inputs name supported ROS distributions.
 export function validateDistros(
 	ros1Distro: string,
@@ -144,6 +165,22 @@ export function validateDistros(
 	return true;
 }
 
+async function installRosdeps(
+	fromPaths: string[],
+	distro: string,
+	options?: im.ExecOptions
+): Promise<number> {
+	const prefixVars = [
+		"DEBIAN_FRONTEND=noninteractive",
+		"RTI_NC_LICENSE_ACCEPTED=yes"
+	].join(" ");
+	return await execBashCommand(
+		`rosdep install -r --from-paths ${fromPaths.join(" ")} --ignore-src --rosdistro ${distro} -y || true`,
+		prefixVars,
+		options
+	);
+}
+
 async function run() {
 	try {
 		const repo = github.context.repo;
@@ -244,33 +281,15 @@ async function run() {
 			options
 		);
 
-		// Remove all repositories the package under test does not depend on, to
-		// avoid having rosdep installing unrequired dependencies.
-		const spaceSeparatedPackages = packageNameList.join(" ");
-		// Run the colcon list commands outside of diff, so that if they fail (due to a misspelled package perhaps),
-		// then the command will exit early. See issue #364
-		await execBashCommand(
-			`all_packages=$(colcon list -p) &&
-			desired_packages=$(colcon list -p --packages-up-to ${spaceSeparatedPackages}) &&
-			diff --new-line-format="" --unchanged-line-format="" <(echo $all_packages) <(echo $desired_packages) | xargs rm -rf`,
-			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 = await execBashCheckOutput(`colcon list -p --packages-up-to ${spaceSeparatedPackages}`);
 		if (targetRos1Distro) {
-			await execBashCommand(
-				`DEBIAN_FRONTEND=noninteractive RTI_NC_LICENSE_ACCEPTED=yes rosdep install -r --from-paths src --ignore-src --rosdistro ${targetRos1Distro} -y || true`,
-				undefined,
-				options
-			);
+			installRosdeps(desiredPackagePaths, targetRos1Distro, options);
 		}
 		if (targetRos2Distro) {
-			await execBashCommand(
-				`DEBIAN_FRONTEND=noninteractive RTI_NC_LICENSE_ACCEPTED=yes rosdep install -r --from-paths src --ignore-src --rosdistro ${targetRos2Distro} -y || true`,
-				undefined,
-				options
-			);
+			installRosdeps(desiredPackagePaths, targetRos2Distro, options);
 		}
 
 		if (colconMixinName !== "" && colconMixinRepo !== "") {