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

Execute bash script instead of sourcing #9534

Merged
merged 8 commits into from
Sep 19, 2019
Merged

Conversation

damccorm
Copy link

@damccorm damccorm commented Feb 8, 2019

  • Check a variable AZP_BASHV3_OLD_SOURCE_BEHAVIOR. If it's true, use the old (current) behavior.
  • Otherwise, check the executable bit. If set, run the script.
  • If not set, throw a warning and use the old behavior. This warning will include a pointer to docs which illustrate the back-compat variable.

Fixes #7934

docs/bashwarning.md Outdated Show resolved Hide resolved
docs/bashwarning.md Outdated Show resolved Hide resolved
docs/bashwarning.md Outdated Show resolved Hide resolved
@damccorm damccorm requested a review from jtpetty September 19, 2019 14:02
// Check if executable bit is set
let stats: fs.Stats = fs.statSync(targetFilePath);
if ((stats.mode & 1) > 0) {
contents = `sh '${targetFilePath.replace("'", "'\\''")}' ${input_arguments}`.trim();
Copy link
Contributor

Choose a reason for hiding this comment

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

is the purpose to run the script in a sub-shell? Should we use 'bash' instead of 'sh' since 'sh' is not guaranteed to the 'bash' on all systems.

Copy link
Author

Choose a reason for hiding this comment

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

Yeah, that's a good catch, updated appropriately

@jtpetty
Copy link
Contributor

jtpetty commented Sep 19, 2019

LGTM

@damccorm damccorm merged commit 676cd20 into master Sep 19, 2019
@damccorm damccorm deleted the users/damccorm/issue-7934 branch September 19, 2019 17:15
damccorm pushed a commit that referenced this pull request Oct 16, 2019
@damccorm damccorm mentioned this pull request Oct 16, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Bash V3 task sources script instead of executing it
3 participants