Skip to content

Commit

Permalink
Require the driver binary as a test input instead of trying to find i…
Browse files Browse the repository at this point in the history 
…t via hardcoded paths. (#4977)

Signed-off-by: fruffy <fruffy@nyu.edu>
  • Loading branch information
@fruffy @ParthShitole
fruffy authored and ParthShitole committed Oct 24, 2024
1 parent 90939af commit a5c9015
Show file tree
Hide file tree
Showing 6 changed files with 91 additions and 141 deletions.
14 changes: 7 additions & 7 deletions tools/driver/CMakeLists.txt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand All @@ -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})
26 changes: 14 additions & 12 deletions tools/driver/test_scripts/driver_inputs_test_1___one_bad_pathname.bash
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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 <path-to-driver-binary>"

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
Expand Down
27 changes: 16 additions & 11 deletions tools/driver/test_scripts/driver_inputs_test_2___two_bad_pathnames.bash
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2,38 +2,43 @@

### 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 <path-to-driver-binary>"

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.
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

Expand Down
29 changes: 16 additions & 13 deletions tools/driver/test_scripts/driver_inputs_test_3___three_bad_pathnames.bash
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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 <path-to-driver-binary>"

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.
Expand All @@ -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

Expand Down
24 changes: 15 additions & 9 deletions ...s/driver_inputs_test_4___two_good_pathnames___check_for_no_misleading_error_messages.bash
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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 <path-to-driver-binary>"

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`
Expand All @@ -32,7 +45,7 @@ ls -dl p4c
echo
ls -dl p4c*
echo
ls -l
ls -l
echo
echo === env ===
env
Expand All @@ -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
Expand Down
112 changes: 23 additions & 89 deletions tools/driver/test_scripts/driver_inputs_test___shared_code.bash
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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<<<///” to make it clear that the extra characters are just delimiters
### [which, BTW, are here so that space characters, especially trailing ones, will become visible].
done
return 1
# Validates input and checks if the driver binary is valid
# Arguments: $1 - path to the binary file
validate_driver_binary() {
local binary_path="$1"
check_file_exists "$binary_path"
check_is_executable "$binary_path"
}


Expand Down

0 comments on commit a5c9015

Please sign in to comment.