Skip to content
This repository has been archived by the owner on Mar 3, 2021. It is now read-only.

Fix multirun execution of scripts #83

Closed

Conversation

saksmt
Copy link

@saksmt saksmt commented Apr 7, 2020

Fixes this:

multirun(
    name = "a",
    commands = [],
    parallel = False
)

multirun(
    name = "b",
    commands = ["//:a"],
    parallel = False
)

# results in: failed to start process for "a.bash": exec: "a.bash": executable file not found in $PATH

@saksmt
Copy link
Author

saksmt commented Apr 7, 2020

So there is no CI...
Can anyone check if this actually works with existing tests? (have a problem running go under nixos)

Also need some guidance to how tests work and how to add this test case

@ash2k
Copy link
Contributor

ash2k commented Apr 8, 2020

Thanks for the contribution! The builds are there but cannot report back. I've created #84 to look into it properly. Builds for your branch are here https://buildkite.com/bazel/atlassian-bazel-tools/builds?branch=saksmt%3Abugfix%2Fmultirun-bash-execution

Please see how tests are written in multirun/test directory. The idea is to run something simple that produces some output and compare it with expected output.

@@ -53,7 +53,7 @@ def _multirun_impl(ctx):
runfiles = runfiles.merge(default_runfiles)
commands.append(struct(
tag = tag,
path = exe.short_path,
path = "./" + exe.short_path,
Copy link
Contributor

Choose a reason for hiding this comment

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

I think a better approach would be to resolve relative paths in the instructions file taking the directory of that file as the base.

Copy link
Contributor

Choose a reason for hiding this comment

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

I've tried this but it's not that straightforward because the path of the instruction file is relative to the exec root (or whatever) and it's just not worth the effort to do this.

@ash2k ash2k mentioned this pull request Apr 10, 2020
@ash2k
Copy link
Contributor

ash2k commented Apr 10, 2020

I'm closing this in favor of #86. Thank you for reporting the issue!

@ash2k ash2k closed this Apr 10, 2020
@saksmt saksmt deleted the bugfix/multirun-bash-execution branch April 15, 2020 17:24
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants