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

bugfix: do not expand the script path in run/setup/teardown if it is not executable #459

Merged
merged 2 commits into from
Jul 27, 2023

Conversation

motus
Copy link
Member

@motus motus commented Jul 27, 2023

Checking for an actual executable is a bit tricky on Windows, so we'll do it properly later; (also, it is ok to have X bits not set for Python scripts). For now, add some minimal checks to prevent . (as in . script.sh) from expanding into a current directory path instead of being treated as source script.sh in bash.
Also, remove extra newlines when logging the script's stdout/stderr.

@motus motus requested a review from a team as a code owner July 27, 2023 18:20
@motus motus enabled auto-merge (squash) July 27, 2023 18:20
@motus motus added bug Something isn't working ready for review Ready for review labels Jul 27, 2023
Co-authored-by: Brian Kroth <bpkroth@users.noreply.github.com>
Copy link
Contributor

@bpkroth bpkroth left a comment

Choose a reason for hiding this comment

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

Comments inline. Will fix more later.

@motus motus merged commit 755950c into microsoft:main Jul 27, 2023
11 checks passed
@motus motus deleted the sergiym/service/local_exec branch July 28, 2023 18:01
motus added a commit that referenced this pull request Jul 28, 2023
Adds support for splitting a single shell command line into subcommands
and applying the first token script name full path resolution logic to
it.
Also cleans up slightly the handling of `.` directories from #459.
For example, in the following:
```sh
. env.sh && partial/path/to/app/env/script.py || another/script.sh
```
`.` should be left alone as equivalent to the shell keyword `source`,
and both scripts should resolved to an expanded full path, with the
python script prefixed with a python executable path.

Tests for both are included.

---------

Co-authored-by: Sergiy Matusevych <sergiym@microsoft.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working ready for review Ready for review
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants