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

Source bash tasks instead of executing them #11760

Merged
merged 5 commits into from
Dec 16, 2019
Merged
Show file tree
Hide file tree
Changes from 1 commit
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
28 changes: 26 additions & 2 deletions Tasks/BashV3/Tests/L0.ts
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,30 @@ describe('Bash Suite', function () {
it('Runs a checked in script correctly', (done: MochaDone) => {
this.timeout(5000);

process.env['AZP_BASHV3_OLD_SOURCE_BEHAVIOR'] = false;
let tp: string = path.join(__dirname, 'L0External.js');
let tr: ttm.MockTestRunner = new ttm.MockTestRunner(tp);

tr.run();

runValidations(() => {
assert(tr.succeeded, 'Bash should have succeeded.');
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');
} 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('my script output') > 0,'Bash should have correctly run the script');
}, tr, done);
});

it('Runs a checked in script correctly when using the old behavior', (done: MochaDone) => {
this.timeout(5000);

process.env['AZP_BASHV3_OLD_SOURCE_BEHAVIOR'] = true;
let tp: string = path.join(__dirname, 'L0External.js');
let tr: ttm.MockTestRunner = new ttm.MockTestRunner(tp);

Expand Down Expand Up @@ -67,9 +91,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 . 'temp/path/script' myCustomArg to temp/path/fileName.sh`) > 0, 'Bash should have written the script to a file');
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');
} else {
assert(tr.stdout.indexOf(`Writing . 'path/to/script' myCustomArg to temp/path/fileName.sh`) > 0, 'Bash should have written the script to a file');
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('my script output') > 0,'Bash should have correctly run the script');
Expand Down
3 changes: 2 additions & 1 deletion Tasks/BashV3/Tests/L0Args.ts
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,8 @@ let a: ma.TaskLibAnswers = <ma.TaskLibAnswers>{
'path/to/script': {
isFile() {
return true;
}
},
mode: '777'
}
}
};
Expand Down
4 changes: 3 additions & 1 deletion Tasks/BashV3/Tests/L0External.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ let tmr: tmrm.TaskMockRunner = new tmrm.TaskMockRunner(taskPath);
tmr.setInput('targetType', 'filepath');
tmr.setInput('filePath', 'path/to/script');
tmr.setInput('workingDirectory', '/fakecwd');
tmr.setInput('AZP_BASHV3_OLD_SOURCE_BEHAVIOR', process.env['AZP_BASHV3_OLD_SOURCE_BEHAVIOR']);

//Create assertAgent and getVariable mocks, support not added in this version of task-lib
const tl = require('azure-pipelines-task-lib/mock-task');
Expand Down Expand Up @@ -50,7 +51,8 @@ let a: ma.TaskLibAnswers = <ma.TaskLibAnswers>{
'path/to/script': {
isFile() {
return true;
}
},
mode: '777'
}
}
};
Expand Down
20 changes: 17 additions & 3 deletions Tasks/BashV3/bash.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
import fs = require('fs');
import path = require('path');
import os = require('os');
import tl = require('azure-pipelines-task-lib/task');
import tr = require('azure-pipelines-task-lib/toolrunner');
var uuidV4 = require('uuid/v4');
Expand Down Expand Up @@ -45,8 +44,10 @@ async function run() {
let input_filePath: string;
let input_arguments: string;
let input_script: string;
let old_source_behavior: boolean;
let input_targetType: string = tl.getInput('targetType') || '';
if (input_targetType.toUpperCase() == 'FILEPATH') {
old_source_behavior = tl.getBoolInput('AZP_BASHV3_OLD_SOURCE_BEHAVIOR', /*required*/ false);
damccorm marked this conversation as resolved.
Show resolved Hide resolved
input_filePath = tl.getPathInput('filePath', /*required*/ true);
if (!tl.stats(input_filePath).isFile()) {
throw new Error(tl.loc('JS_InvalidFilePath', input_filePath));
Expand All @@ -72,7 +73,20 @@ async function run() {
targetFilePath = input_filePath;
}

contents = `. '${targetFilePath.replace("'", "'\\''")}' ${input_arguments}`.trim();
if (old_source_behavior) {
damccorm marked this conversation as resolved.
Show resolved Hide resolved
contents = `. '${targetFilePath.replace("'", "'\\''")}' ${input_arguments}`.trim();
} else {
// Check if executable bit is set
let stats: fs.Stats = tl.stats(input_filePath);
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is where the mistake was last time - in the last iteration of this PR, I was doing a stats call on targetFilePath instead of input_filePath. This is a problem because targetFilePath gets some modifications on just windows so that it will work with bash, that cause stats to fail.

damccorm marked this conversation as resolved.
Show resolved Hide resolved
if ((stats.mode & 1) > 0) {
damccorm marked this conversation as resolved.
Show resolved Hide resolved
contents = `bash '${targetFilePath.replace("'", "'\\''")}' ${input_arguments}`.trim();
}
else {
console.log(stats.mode);
damccorm marked this conversation as resolved.
Show resolved Hide resolved
tl.warning('Executable bit is not set on target script, sourcing instead of executing. More info at https://github.com/Microsoft/azure-pipelines-tasks/blob/master/docs/bashnote.md');
contents = `. '${targetFilePath.replace("'", "'\\''")}' ${input_arguments}`.trim();
}
}
console.log(tl.loc('JS_FormattedCommand', contents));
}
else {
Expand Down Expand Up @@ -152,4 +166,4 @@ async function run() {
}
}

run();
run();