Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

CmdLine does not forwarding signals #13573

Merged
merged 6 commits into from
Sep 24, 2020
Merged
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
8 changes: 4 additions & 4 deletions Tasks/BashV3/Tests/L0.ts
Original file line number Diff line number Diff line change
Expand Up @@ -47,9 +47,9 @@ describe('Bash Suite', function () {
assert(tr.stderr.length === 0, 'Bash should not have written to stderr');
if (process.platform === 'win32') {
// This is different on windows because we change the script name to make sure the normalization call is happening.
assert(tr.stdout.indexOf(`Writing bash 'temp/path/script' to temp/path/fileName.sh`) > 0, 'Bash should have written the script to a file');
assert(tr.stdout.indexOf(`Writing exec bash 'temp/path/script' to temp/path/fileName.sh`) > 0, 'Bash should have written the script to a file');
} else {
assert(tr.stdout.indexOf(`Writing bash 'path/to/script' to temp/path/fileName.sh`) > 0, 'Bash should have written the script to a file');
assert(tr.stdout.indexOf(`Writing exec bash 'path/to/script' to temp/path/fileName.sh`) > 0, 'Bash should have written the script to a file');
}

assert(tr.stdout.indexOf('my script output') > 0,'Bash should have correctly run the script');
Expand Down Expand Up @@ -93,9 +93,9 @@ describe('Bash Suite', function () {
assert(tr.stderr.length === 0, 'Bash should not have written to stderr');
if (process.platform === 'win32') {
// This is different on windows because we change the script name to make sure the normalization call is happening.
assert(tr.stdout.indexOf(`Writing bash 'temp/path/script' myCustomArg to temp/path/fileName.sh`) > 0, 'Bash should have written the script to a file');
assert(tr.stdout.indexOf(`Writing exec bash 'temp/path/script' myCustomArg to temp/path/fileName.sh`) > 0, 'Bash should have written the script to a file');
} else {
assert(tr.stdout.indexOf(`Writing bash 'path/to/script' myCustomArg to temp/path/fileName.sh`) > 0, 'Bash should have written the script to a file');
assert(tr.stdout.indexOf(`Writing exec bash 'path/to/script' myCustomArg to temp/path/fileName.sh`) > 0, 'Bash should have written the script to a file');
}

assert(tr.stdout.indexOf('my script output') > 0,'Bash should have correctly run the script');
Expand Down
16 changes: 15 additions & 1 deletion Tasks/BashV3/bash.ts
Original file line number Diff line number Diff line change
Expand Up @@ -79,7 +79,7 @@ async function run() {
if (old_source_behavior) {
contents = `. '${targetFilePath.replace(/'/g, "'\\''")}' ${input_arguments}`.trim();
} else {
contents = `bash '${targetFilePath.replace(/'/g, "'\\''")}' ${input_arguments}`.trim();
contents = `exec bash '${targetFilePath.replace(/'/g, "'\\''")}' ${input_arguments}`.trim();
EzzhevNikita marked this conversation as resolved.
Show resolved Hide resolved
}
console.log(tl.loc('JS_FormattedCommand', contents));
}
Expand Down Expand Up @@ -128,6 +128,11 @@ async function run() {
ignoreReturnCode: true
};

process.on("SIGINT", () => {
tl.debug('Started cancellation of executing script');
bash.killChildProcess();
EzzhevNikita marked this conversation as resolved.
Show resolved Hide resolved
});

// Listen for stderr.
let stderrFailure = false;
const aggregatedStderr: string[] = [];
Expand All @@ -153,6 +158,15 @@ async function run() {

let result = tl.TaskResult.Succeeded;

/**
* Exit code null could appeared in situations if executed script don't process cancellation signal,
* as we already have message after operation cancellation, we can avoid processing null code here.
*/
if (exitCode === null) {
tl.debug('Script execution cancelled');
return;
EzzhevNikita marked this conversation as resolved.
Show resolved Hide resolved
}

// Fail on exit code.
if (exitCode !== 0) {
tl.error(tl.loc('JS_ExitCode', exitCode));
Expand Down
7 changes: 4 additions & 3 deletions Tasks/BashV3/package-lock.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion Tasks/BashV3/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,6 @@
"homepage": "https://github.com/Microsoft/azure-pipelines-tasks#readme",
"dependencies": {
"uuid": "^3.0.1",
"azure-pipelines-task-lib": "2.9.3"
"azure-pipelines-task-lib": "2.11.1"
}
}
4 changes: 2 additions & 2 deletions Tasks/BashV3/task.json
Original file line number Diff line number Diff line change
Expand Up @@ -17,8 +17,8 @@
"author": "Microsoft Corporation",
"version": {
"Major": 3,
"Minor": 171,
"Patch": 1
"Minor": 176,
"Patch": 0
},
"releaseNotes": "Script task consistency. Added support for multiple lines and added support for Windows.",
"minimumAgentVersion": "2.115.0",
Expand Down
4 changes: 2 additions & 2 deletions Tasks/BashV3/task.loc.json
Original file line number Diff line number Diff line change
Expand Up @@ -17,8 +17,8 @@
"author": "Microsoft Corporation",
"version": {
"Major": 3,
"Minor": 171,
"Patch": 1
"Minor": 176,
"Patch": 0
},
"releaseNotes": "ms-resource:loc.releaseNotes",
"minimumAgentVersion": "2.115.0",
Expand Down
18 changes: 18 additions & 0 deletions Tasks/CmdLineV2/cmdline.ts
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,10 @@ async function run() {
let failOnStderr = tl.getBoolInput('failOnStderr', false);
let script: string = tl.getInput('script', false) || '';
let workingDirectory = tl.getPathInput('workingDirectory', /*required*/ true, /*check*/ true);

if (fs.existsSync(script)) {
script = `exec ${script}`;
}
EzzhevNikita marked this conversation as resolved.
Show resolved Hide resolved

// Write the script to disk.
console.log(tl.loc('GeneratingScript'));
Expand Down Expand Up @@ -65,11 +69,25 @@ async function run() {
});
}

process.on("SIGINT", () => {
tl.debug('Started cancellation of executing script');
bash.killChildProcess();
EzzhevNikita marked this conversation as resolved.
Show resolved Hide resolved
});

// Run bash.
let exitCode: number = await bash.exec(options);

let result = tl.TaskResult.Succeeded;

/**
* Exit code null could appeared in situations if executed script don't process cancellation signal,
* as we already have message after operation cancellation, we can avoid processing null code here.
*/
if (exitCode === null) {
tl.debug('Script execution cancelled');
return;
EzzhevNikita marked this conversation as resolved.
Show resolved Hide resolved
}

// Fail on exit code.
if (exitCode !== 0) {
tl.error(tl.loc('JS_ExitCode', exitCode));
Expand Down
6 changes: 3 additions & 3 deletions Tasks/CmdLineV2/package-lock.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion Tasks/CmdLineV2/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@
"uuid": "^3.0.1",
"@types/mocha": "^8.0.3",
"@types/node": "^6.0.0",
"azure-pipelines-task-lib": "^2.10.1"
"azure-pipelines-task-lib": "^2.11.1"
},
"devDependencies": {
"typescript": "4.0.2"
Expand Down
2 changes: 1 addition & 1 deletion Tasks/CmdLineV2/task.json
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@
"version": {
"Major": 2,
"Minor": 176,
"Patch": 0
"Patch": 1
},
"releaseNotes": "Script task consistency. Added support for multiple lines.",
"showEnvironmentVariables": true,
Expand Down
2 changes: 1 addition & 1 deletion Tasks/CmdLineV2/task.loc.json
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@
"version": {
"Major": 2,
"Minor": 176,
"Patch": 0
"Patch": 1
},
"releaseNotes": "ms-resource:loc.releaseNotes",
"showEnvironmentVariables": true,
Expand Down