bazel-contrib / rules_fuzzing

Bazel Starlark extensions for defining fuzz tests in Bazel projects
Apache License 2.0
87 stars 20 forks source link

Refactor the inline Python commands into a Python binary. #180

Open xinhaoyuan opened 3 years ago

xinhaoyuan commented 3 years ago

This is needed when python3 does not exist in the runtime environment.

fmeum commented 3 years ago

Don't worry about the Jazzer launcher script. I am currently working on completely redoing the logic for how Jazzer finds the JVM. As a side effect of this effort, it will no longer need Python.

stefanbucur commented 3 years ago

Thanks for simplifying the launcher scripts, and sorry for the delay.

I am wondering if there is a clean way to eliminate the (new) duplication in the launcher scripts, too, since I expect every launcher script would need to add this data dependency.

Could we somehow make this tool available as a data dependency on the action that invokes the launcher? This way, we could augment the script calling the launcher to extend the $PATH with the location of the tool, so the script can directly use $(realpath ${FUZZER_BINARY}).

I've just realize we might have another alternative: Guarantee that $FUZZER_BINARY is an absolute path, and make sure the places that set it do so correctly. For the fuzzing launcher, this is conveniently Python code so we can inline the contents of realpath there: https://github.com/bazelbuild/rules_fuzzing/blob/4bafba51ffd9d418d236adb61de36fda1a90e764/fuzzing/tools/launcher.py#L95

There is one more place which is currently Bash, but I'm wondering if that rule is actually still needed since we have the more general launcher rule: https://github.com/bazelbuild/rules_fuzzing/blob/4bafba51ffd9d418d236adb61de36fda1a90e764/fuzzing/private/regression.bzl#L25

xinhaoyuan commented 3 years ago

I tried to put the tool dependency inside the launcher rule, but I didn't like it since it makes the testing hard (matching the runfiles in engine_test.bzl). Now thinking of it, I think not all engines require realpath (e.g. honggfuzz), so it might be nicer to have each engine decide to depend on the tool or not.

If canonical path is not required, we can rewrite the bash script, e.g. append ./ to the path if it does not start with /. WDYT?