From a5c9015c491382206139d98924d72becfdd6af8a Mon Sep 17 00:00:00 2001 From: Fabian Ruffy <5960321+fruffy@users.noreply.github.com> Date: Wed, 23 Oct 2024 20:58:39 +0200 Subject: [PATCH] Require the driver binary as a test input instead of trying to find it via hardcoded paths. (#4977) Signed-off-by: fruffy --- tools/driver/CMakeLists.txt | 14 +-- ...iver_inputs_test_1___one_bad_pathname.bash | 26 ++-- ...ver_inputs_test_2___two_bad_pathnames.bash | 27 +++-- ...r_inputs_test_3___three_bad_pathnames.bash | 29 +++-- ...heck_for_no_misleading_error_messages.bash | 24 ++-- .../driver_inputs_test___shared_code.bash | 112 ++++-------------- 6 files changed, 91 insertions(+), 141 deletions(-) diff --git a/tools/driver/CMakeLists.txt b/tools/driver/CMakeLists.txt index e28b87c7a16..789f729201d 100644 --- a/tools/driver/CMakeLists.txt +++ b/tools/driver/CMakeLists.txt @@ -53,12 +53,14 @@ foreach (__f IN LISTS P4C_DRIVER_SRCS P4C_TARGET_CFGS) list (APPEND P4C_DRIVER_DST "${P4C_BINARY_DIR}/${__f}") endforeach(__f) +set (P4C_DRIVER_PATH ${P4C_BINARY_DIR}/${P4C_DRIVER_NAME}) + add_custom_command(OUTPUT ${P4C_DRIVER_DST} COMMAND ${CMAKE_COMMAND} -E make_directory ${P4C_BINARY_DIR}/p4c_src && for f in ${P4C_DRIVER_SRCS} ${P4C_TARGET_CFGS} \; do ${CMAKE_COMMAND} -E copy_if_different \$$f ${P4C_BINARY_DIR}/p4c_src \; done - COMMAND chmod a+x ${P4C_BINARY_DIR}/${P4C_DRIVER_NAME} + COMMAND chmod a+x ${P4C_DRIVER_PATH} DEPENDS ${P4C_DRIVER_SRCS} WORKING_DIRECTORY ${CMAKE_CURRENT_SOURCE_DIR} COMMENT "Copying p4c driver" @@ -68,9 +70,7 @@ add_custom_target(p4c_driver ALL DEPENDS ${P4C_DRIVER_DST}) -### Added by Abe starting April 6 2022; these tests are all expected to fail until Abe`s branch with driver improvements is merged. - -add_test(NAME driver_inputs_test_1 COMMAND ${P4C_SOURCE_DIR}/tools/driver/test_scripts/driver_inputs_test_1) -add_test(NAME driver_inputs_test_2 COMMAND ${P4C_SOURCE_DIR}/tools/driver/test_scripts/driver_inputs_test_2) -add_test(NAME driver_inputs_test_3 COMMAND ${P4C_SOURCE_DIR}/tools/driver/test_scripts/driver_inputs_test_3) -add_test(NAME driver_inputs_test_4 COMMAND ${P4C_SOURCE_DIR}/tools/driver/test_scripts/driver_inputs_test_4) +add_test(NAME driver_inputs_test_1 COMMAND ${P4C_SOURCE_DIR}/tools/driver/test_scripts/driver_inputs_test_1 ${P4C_DRIVER_PATH}) +add_test(NAME driver_inputs_test_2 COMMAND ${P4C_SOURCE_DIR}/tools/driver/test_scripts/driver_inputs_test_2 ${P4C_DRIVER_PATH}) +add_test(NAME driver_inputs_test_3 COMMAND ${P4C_SOURCE_DIR}/tools/driver/test_scripts/driver_inputs_test_3 ${P4C_DRIVER_PATH}) +add_test(NAME driver_inputs_test_4 COMMAND ${P4C_SOURCE_DIR}/tools/driver/test_scripts/driver_inputs_test_4 ${P4C_DRIVER_PATH}) diff --git a/tools/driver/test_scripts/driver_inputs_test_1___one_bad_pathname.bash b/tools/driver/test_scripts/driver_inputs_test_1___one_bad_pathname.bash index e6e4a386ee1..d3c68d648b3 100755 --- a/tools/driver/test_scripts/driver_inputs_test_1___one_bad_pathname.bash +++ b/tools/driver/test_scripts/driver_inputs_test_1___one_bad_pathname.bash @@ -2,30 +2,32 @@ ### a simple test: ensure that a _single_ non-existant input pathname results in an error output that mentions that pathname. +SCRIPT_NAME=$(basename "$0") +USAGE="Usage: $SCRIPT_NAME " +show_usage() { + echo "$USAGE" +} -source $(dirname "`realpath "${BASH_SOURCE[0]}"`")/driver_inputs_test___shared_code.bash +if [ $# -eq 0 ]; then + echo "Error: No path to driver binary provided." + show_usage + exit 1 +fi +source $(dirname "`realpath "${BASH_SOURCE[0]}"`")/driver_inputs_test___shared_code.bash check_for_inadvisable_sourcing; returned=$? if [ $returned -ne 0 ]; then return $returned; fi ### simulating exception handling for an exception that is not caught at this level - +driver_path="$1" +validate_driver_binary "$driver_path" humanReadable_test_pathname="`resolve_symlink_only_of_basename "$0"`" - -if ! P4C=`try_to_find_the_driver`; then - echo "Unable to find the driver of the P4 compiler. Aborting the test ''$humanReadable_test_pathname'' with a non-zero exit code. This test failed." >& 2 - exit 255 -fi -echo "In ''$humanReadable_test_pathname'', using ''$P4C'' as the path to the driver of the P4 compiler." >& 2 - - - BAD_PATHNAME=/path/to/a/nonexistant/supposedly-P4/source/file -"$P4C" $BAD_PATHNAME 2>&1 | grep --ignore-case --quiet "error.*$BAD_PATHNAME" +"$driver_path" $BAD_PATHNAME 2>&1 | grep --ignore-case --quiet "error.*$BAD_PATHNAME" exit_status_from_grep=$? if [ $exit_status_from_grep -eq 0 ]; then diff --git a/tools/driver/test_scripts/driver_inputs_test_2___two_bad_pathnames.bash b/tools/driver/test_scripts/driver_inputs_test_2___two_bad_pathnames.bash index 2c8f00dcd81..c7314695df1 100755 --- a/tools/driver/test_scripts/driver_inputs_test_2___two_bad_pathnames.bash +++ b/tools/driver/test_scripts/driver_inputs_test_2___two_bad_pathnames.bash @@ -2,6 +2,18 @@ ### ensure that when _two_ non-existant input pathnames are given, each one results in an error output that mentions that pathname. +SCRIPT_NAME=$(basename "$0") +USAGE="Usage: $SCRIPT_NAME " + +show_usage() { + echo "$USAGE" +} + +if [ $# -eq 0 ]; then + echo "Error: No path to driver binary provided." + show_usage + exit 1 +fi source $(dirname "`realpath "${BASH_SOURCE[0]}"`")/driver_inputs_test___shared_code.bash @@ -9,20 +21,13 @@ source $(dirname "`realpath "${BASH_SOURCE[0]}"`")/driver_inputs_test___shared_c check_for_inadvisable_sourcing; returned=$? if [ $returned -ne 0 ]; then return $returned; fi ### simulating exception handling for an exception that is not caught at this level +driver_path="$1" +validate_driver_binary "$driver_path" humanReadable_test_pathname="`resolve_symlink_only_of_basename "$0"`" - -if ! P4C=`try_to_find_the_driver`; then - echo "Unable to find the driver of the P4 compiler. Aborting the test ''$humanReadable_test_pathname'' with a non-zero exit code. This test failed." >& 2 - exit 255 -fi -echo "In ''$humanReadable_test_pathname'', using ''$P4C'' as the path to the driver of the P4 compiler." >& 2 - - - BAD_PATHNAME_BASE=/path/to/a/nonexistant/supposedly-P4/source/file ### Technically, these don`t need to be _unique_ pathnames in order to trigger the bad/confusing behavior of the driver that Abe ### saw before he started working on it, but unique pathnames will more helpful for debugging, in case that will ever be needed. @@ -30,10 +35,10 @@ BAD_PATHNAME_1=$BAD_PATHNAME_BASE/1 BAD_PATHNAME_2=$BAD_PATHNAME_BASE/2 ### Using ASCII double quotes to guard against bugs due to ASCII spaces, even though this test-script file is free of such bugs as of this writing. -check_for_pathname_error_in_P4_compiler_driver_output "$P4C" "$BAD_PATHNAME_1" 1 2 +check_for_pathname_error_in_P4_compiler_driver_output "$driver_path" "$BAD_PATHNAME_1" 1 2 result_1=$? echo -check_for_pathname_error_in_P4_compiler_driver_output "$P4C" "$BAD_PATHNAME_2" 2 2 +check_for_pathname_error_in_P4_compiler_driver_output "$driver_path" "$BAD_PATHNAME_2" 2 2 result_2=$? echo diff --git a/tools/driver/test_scripts/driver_inputs_test_3___three_bad_pathnames.bash b/tools/driver/test_scripts/driver_inputs_test_3___three_bad_pathnames.bash index 2642e4fa683..c0751da6dd1 100755 --- a/tools/driver/test_scripts/driver_inputs_test_3___three_bad_pathnames.bash +++ b/tools/driver/test_scripts/driver_inputs_test_3___three_bad_pathnames.bash @@ -2,27 +2,30 @@ ### ensure that when _three_ non-existant input pathnames are given, each one results in an error output that mentions that pathname. +SCRIPT_NAME=$(basename "$0") +USAGE="Usage: $SCRIPT_NAME " +show_usage() { + echo "$USAGE" +} + +if [ $# -eq 0 ]; then + echo "Error: No path to driver binary provided." + show_usage + exit 1 +fi source $(dirname "`realpath "${BASH_SOURCE[0]}"`")/driver_inputs_test___shared_code.bash check_for_inadvisable_sourcing; returned=$? if [ $returned -ne 0 ]; then return $returned; fi ### simulating exception handling for an exception that is not caught at this level - +driver_path="$1" +validate_driver_binary "$driver_path" humanReadable_test_pathname="`resolve_symlink_only_of_basename "$0"`" - -if ! P4C=`try_to_find_the_driver`; then - echo "Unable to find the driver of the P4 compiler. Aborting the test ''$humanReadable_test_pathname'' with a non-zero exit code. This test failed." >& 2 - exit 255 -fi -echo "In ''$humanReadable_test_pathname'', using ''$P4C'' as the path to the driver of the P4 compiler." >& 2 - - - BAD_PATHNAME_BASE=/path/to/a/nonexistant/supposedly-P4/source/file ### Technically, these don`t need to be _unique_ pathnames in order to trigger the bad/confusing behavior of the driver that Abe ### saw before he started working on it, but unique pathnames will more helpful for debugging, in case that will ever be needed. @@ -31,13 +34,13 @@ BAD_PATHNAME_2=$BAD_PATHNAME_BASE/2 BAD_PATHNAME_3=$BAD_PATHNAME_BASE/3 ### Using ASCII double quotes to guard against bugs due to ASCII spaces, even though this test-script file is free of such bugs as of this writing. -check_for_pathname_error_in_P4_compiler_driver_output "$P4C" "$BAD_PATHNAME_1" 1 3 +check_for_pathname_error_in_P4_compiler_driver_output "$driver_path" "$BAD_PATHNAME_1" 1 3 result_1=$? echo -check_for_pathname_error_in_P4_compiler_driver_output "$P4C" "$BAD_PATHNAME_2" 2 3 +check_for_pathname_error_in_P4_compiler_driver_output "$driver_path" "$BAD_PATHNAME_2" 2 3 result_2=$? echo -check_for_pathname_error_in_P4_compiler_driver_output "$P4C" "$BAD_PATHNAME_3" 3 3 +check_for_pathname_error_in_P4_compiler_driver_output "$driver_path" "$BAD_PATHNAME_3" 3 3 result_3=$? echo diff --git a/tools/driver/test_scripts/driver_inputs_test_4___two_good_pathnames___check_for_no_misleading_error_messages.bash b/tools/driver/test_scripts/driver_inputs_test_4___two_good_pathnames___check_for_no_misleading_error_messages.bash index 6de8ba31ac1..d65c94f8cf6 100755 --- a/tools/driver/test_scripts/driver_inputs_test_4___two_good_pathnames___check_for_no_misleading_error_messages.bash +++ b/tools/driver/test_scripts/driver_inputs_test_4___two_good_pathnames___check_for_no_misleading_error_messages.bash @@ -12,13 +12,26 @@ ### ### ... or if “p4include/core.p4” and/or “p4include/pna.p4” will ever _not_ be present at the top level of a valid build directory [of a built build]. +SCRIPT_NAME=$(basename "$0") +USAGE="Usage: $SCRIPT_NAME " +show_usage() { + echo "$USAGE" +} + +if [ $# -eq 0 ]; then + echo "Error: No path to driver binary provided." + show_usage + exit 1 +fi source $(dirname "`realpath "${BASH_SOURCE[0]}"`")/driver_inputs_test___shared_code.bash check_for_inadvisable_sourcing; returned=$? if [ $returned -ne 0 ]; then return $returned; fi ### simulating exception handling for an exception that is not caught at this level +driver_path="$1" +validate_driver_binary "$driver_path" output_dir=`mktemp -d /tmp/P4C_driver_testing___XXXXXXXXXX` @@ -32,7 +45,7 @@ ls -dl p4c echo ls -dl p4c* echo -ls -l +ls -l echo echo === env === env @@ -46,15 +59,8 @@ echo '===== ^^^ ===== test debugging ===== ^^^ =====' humanReadable_test_pathname="`resolve_symlink_only_of_basename "$0"`" -if ! P4C=`try_to_find_the_driver`; then - echo "Unable to find the driver of the P4 compiler. Aborting the test ''$humanReadable_test_pathname'' with a non-zero exit code. This test failed." >& 2 - exit 255 -fi -echo "In ''$humanReadable_test_pathname'', using ''$P4C'' as the path to the driver of the P4 compiler." >& 2 - - -if [ ! -x "$P4C" ] ; then +if [ ! -x "$driver_path" ] ; then echo "Test ''$humanReadable_test_pathname'' failed due to not being able to execute ''$P4C''." >& 2 exit 1 elif "$P4C" p4include/core.p4 p4include/pna.p4 -o "$output_dir" 2>&1 | grep --ignore-case --quiet 'unrecognized.*arguments'; then diff --git a/tools/driver/test_scripts/driver_inputs_test___shared_code.bash b/tools/driver/test_scripts/driver_inputs_test___shared_code.bash index 56082652e8e..8285262c972 100644 --- a/tools/driver/test_scripts/driver_inputs_test___shared_code.bash +++ b/tools/driver/test_scripts/driver_inputs_test___shared_code.bash @@ -62,99 +62,33 @@ function report___num_failures___and_clamp_it_to_an_inclusive_maximum_of_255 { if [ $num_failures -gt 255 ]; then num_failures=255; fi } - - -### requires a single arg./param., which had better be a pathname [doesn`t need to be absolute] -function check_if_this_seems_to_be_our_driver { - echo "INFO: searching for the P4 compiler driver at ''$1''..." >& 2 ### reminder: “>& 2” is equivalent to “> /dev/stderr” - - if [ -L "$1" ]; then - ### NOTE: is “-L” a GNUism? “-h” might be the POSIX “spelling”. - ### Either way, it should work OK in {Bash on non-GNU OS} as long as I use Bash`s built-in ‘[’/“test”, - ### not e.g. “/bin/[” or “/bin/test” or “/usr/bin/[” or “/usr/bin/test”. - - echo "INFO: detected a symlink at ''$1'' [starting at ''$PWD'', if that is a relative pathname], so checking the target of the symlink." >& 2 - if realpath --version | grep --quiet GNU; then ### happy-happy-joy-joy: GNU realpath is available and is the default “realpath” - ### try the “logical” interpretation first, i.e. give “../” components priority over symlink components - if next_turtle=`realpath --canonicalize-existing --logical "$1"`; then check_if_this_seems_to_be_our_driver "$next_turtle"; return; fi ### please see Knuth`s definition of recursion ;-) - if next_turtle=`realpath --canonicalize-existing --physical "$1"`; then check_if_this_seems_to_be_our_driver "$next_turtle"; return; fi - ### If we are “still here”, then canonicalization failed. :-( - echo "ERROR: failed to canonicalize the symlink ''$1'' while using GNU ''realpath''." >& 2 - return 1 - fi - - if readlink --version | grep --quiet GNU; then ### second-best: GNU readlink is available and is the default “readlink” - if next_turtle=`readlink --canonicalize-existing "$1"`; then check_if_this_seems_to_be_our_driver "$next_turtle"; return; fi - ### If we are “still here”, then canonicalization failed. :-( - echo "ERROR: failed to canonicalize the symlink ''$1'' while using GNU ''readlink''." >& 2 - return 2 +# Function: Check if the driver binary exists +# Arguments: $1 - path to the file +check_driver_binary_exists() { + local file_path="$1" + if [ ! -e "$file_path" ]; then + echo "Error: The driver binary '$file_path' does not exist." + show_usage + exit 1 fi +} - if realpath / > /dev/null; then ### I hope that the “realpath” implementations in e.g. BSD all do symlink-cycle detection, as GNU realpath does (at least as of GNU coreutils version 8.30) - if next_turtle=`realpath "$1"`; then check_if_this_seems_to_be_our_driver "$next_turtle"; return; fi - ### If we are “still here”, then canonicalization failed. :-( - echo "ERROR: failed to canonicalize the symlink ''$1'' while using [presumed non-GNU] ''realpath''." >& 2 - return 3 +# Function: Check if the driver binary is executable +# Arguments: $1 - path to the file +check_driver_is_executable() { + local file_path="$1" + if [ ! -x "$file_path" ]; then + echo "Error: The driver binary '$file_path' is not executable." + exit 1 fi +} - ### The “readlink” in BSD [well, at least the one I _know_ of ;-)] is _extremely_ minimal, and AFAIK can`t do cycle detection, - ### so I`m not even going to _try_ non-GNU readlink... mainly because I don`t want to get - ### “INFO: searching for the P4 compiler driver”[...] until Bash runs out of function-call stack and crashes, - ### in the case of a symlink cycle. At this point, I will just pretend I never detected a symlink and let it go forward. - ### This way, if the symlink _is_ part of a cycle, this whole process should crash in a way that is “nicer” - ### than flooding the output with “INFO: searching for the P4 compiler driver”[...] lines. - - fi ### Done checking for a symlink. At this point, "$1" is either (1) _not_ a symlink or (2) a symlink we couldn`t canonicalize. - - if [ ! -e "$1" ]; then echo "INFO: not using ''$1'', because it is not found in the filesystem [starting at ''$PWD'', if that is a relative pathname]." >& 2; return 1; fi - if [ ! -x "$1" ]; then echo "INFO: not using ''$1'', because it is not executable." >& 2; return 2; fi - if [ -d "$1" ]; then echo "INFO: not using ''$1'', because it is a directory." >& 2; return 3; fi - if [ ! -s "$1" ]; then echo "INFO: not using ''$1'', because either it does not exist [starting at ''$PWD'', if that is a relative pathname] or it exists but is empty." >& 2; return 4; fi - - ### NOTE on the following: I _could_ be more strict, e.g. requiring that the output of “$foo --version” start with “p4c” or “p4c ”, - ### but that might be a bad idea in the long run, e.g. if the output of the driver`s version report will be changed, - ### e.g. to start with “P4C ” or “Version of P4C: ” instead of with “p4c ” as it is as of this writing - ### [and has been for some time, TTBOMK]. - if ! "$1" --version > /dev/null; then echo "INFO: not using ''$1'', because it did not accept the arg./flag/param. ''--version''" >& 2; return 5; fi - - ### OK; at this point, we have given up on finding “reasons” to reject "$1" as a supposedly-good P4 compiler driver. ;-) - echo "INFO: accepting ''$1'' as a presumed P4 compiler driver." >& 2 - return 0 -} ### end of function “check_if_this_seems_to_be_our_driver” - - - -### IMPORTANT: do _not_ include any human-oriented “fluff” in this function`s standard-out output -function try_to_find_the_driver { - ### NOTES re "$GITHUB_WORKSPACE", "$RUNNER_TEMP", and "$RUNNER_WORKSPACE": - ### these were all found by Abe in the GitHub CI/CD environment on April 14 2022 - ### - ### here are the values they had at that time [which explains why I am not checking "$RUNNER_TEMP" by itself]: - ### - ### GITHUB_WORKSPACE=/__w/p4c/p4c - ### RUNNER_TEMP=/__w/_temp - ### RUNNER_WORKSPACE=/__w/p4c - - ### IMPORTANT: the ordering in the following *_IS_* important, and may need to be changed at a later time due to e.g. changes in the GitHub CI/CD - to_probe=(./p4c ../p4c ../../p4c ../p4c/p4c ../../p4c/p4c) - if [ -n "$GITHUB_WORKSPACE" ]; then to_probe+=("$GITHUB_WORKSPACE" "$GITHUB_WORKSPACE"/p4c "$GITHUB_WORKSPACE"/p4c/p4c); fi - if [ -n "$RUNNER_WORKSPACE" ]; then to_probe+=("$RUNNER_WORKSPACE" "$RUNNER_WORKSPACE"/p4c "$RUNNER_WORKSPACE"/p4c/p4c); fi - if [ -n "$RUNNER_TEMP" ]; then to_probe+=( "$RUNNER_TEMP"''''//p4c "$RUNNER_TEMP"''''//p4c/p4c); fi - - for probe in ${to_probe[@]}; do - if check_if_this_seems_to_be_our_driver "$probe"; then - echo "$probe" ### IMPORTANT: do _not_ include any human-oriented “fluff” in this output - return 0 - fi - done - - echo "FATAL ERROR: could not find the P4 compiler driver. Searched in the following locations..." >& 2 - for probe in ${to_probe[@]}; do - echo "///>>>$probe<<& 2 - ### Using “///>>>$foo<<