Skip to content

Commit 75ea563

Browse files
authored
Require the driver binary as a test input instead of trying to find it via hardcoded paths. (#4977)
Signed-off-by: fruffy <[email protected]>
1 parent c05b04c commit 75ea563

6 files changed

+91
-141
lines changed

tools/driver/CMakeLists.txt

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -53,12 +53,14 @@ foreach (__f IN LISTS P4C_DRIVER_SRCS P4C_TARGET_CFGS)
5353
list (APPEND P4C_DRIVER_DST "${P4C_BINARY_DIR}/${__f}")
5454
endforeach(__f)
5555

56+
set (P4C_DRIVER_PATH ${P4C_BINARY_DIR}/${P4C_DRIVER_NAME})
57+
5658
add_custom_command(OUTPUT ${P4C_DRIVER_DST}
5759
COMMAND ${CMAKE_COMMAND} -E make_directory ${P4C_BINARY_DIR}/p4c_src &&
5860
for f in ${P4C_DRIVER_SRCS} ${P4C_TARGET_CFGS} \; do
5961
${CMAKE_COMMAND} -E copy_if_different \$$f ${P4C_BINARY_DIR}/p4c_src \;
6062
done
61-
COMMAND chmod a+x ${P4C_BINARY_DIR}/${P4C_DRIVER_NAME}
63+
COMMAND chmod a+x ${P4C_DRIVER_PATH}
6264
DEPENDS ${P4C_DRIVER_SRCS}
6365
WORKING_DIRECTORY ${CMAKE_CURRENT_SOURCE_DIR}
6466
COMMENT "Copying p4c driver"
@@ -68,9 +70,7 @@ add_custom_target(p4c_driver ALL DEPENDS ${P4C_DRIVER_DST})
6870

6971

7072

71-
### Added by Abe starting April 6 2022; these tests are all expected to fail until Abe`s branch with driver improvements is merged.
72-
73-
add_test(NAME driver_inputs_test_1 COMMAND ${P4C_SOURCE_DIR}/tools/driver/test_scripts/driver_inputs_test_1)
74-
add_test(NAME driver_inputs_test_2 COMMAND ${P4C_SOURCE_DIR}/tools/driver/test_scripts/driver_inputs_test_2)
75-
add_test(NAME driver_inputs_test_3 COMMAND ${P4C_SOURCE_DIR}/tools/driver/test_scripts/driver_inputs_test_3)
76-
add_test(NAME driver_inputs_test_4 COMMAND ${P4C_SOURCE_DIR}/tools/driver/test_scripts/driver_inputs_test_4)
73+
add_test(NAME driver_inputs_test_1 COMMAND ${P4C_SOURCE_DIR}/tools/driver/test_scripts/driver_inputs_test_1 ${P4C_DRIVER_PATH})
74+
add_test(NAME driver_inputs_test_2 COMMAND ${P4C_SOURCE_DIR}/tools/driver/test_scripts/driver_inputs_test_2 ${P4C_DRIVER_PATH})
75+
add_test(NAME driver_inputs_test_3 COMMAND ${P4C_SOURCE_DIR}/tools/driver/test_scripts/driver_inputs_test_3 ${P4C_DRIVER_PATH})
76+
add_test(NAME driver_inputs_test_4 COMMAND ${P4C_SOURCE_DIR}/tools/driver/test_scripts/driver_inputs_test_4 ${P4C_DRIVER_PATH})

tools/driver/test_scripts/driver_inputs_test_1___one_bad_pathname.bash

Lines changed: 14 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -2,30 +2,32 @@
22

33
### a simple test: ensure that a _single_ non-existant input pathname results in an error output that mentions that pathname.
44

5+
SCRIPT_NAME=$(basename "$0")
6+
USAGE="Usage: $SCRIPT_NAME <path-to-driver-binary>"
57

8+
show_usage() {
9+
echo "$USAGE"
10+
}
611

7-
source $(dirname "`realpath "${BASH_SOURCE[0]}"`")/driver_inputs_test___shared_code.bash
12+
if [ $# -eq 0 ]; then
13+
echo "Error: No path to driver binary provided."
14+
show_usage
15+
exit 1
16+
fi
817

18+
source $(dirname "`realpath "${BASH_SOURCE[0]}"`")/driver_inputs_test___shared_code.bash
919
check_for_inadvisable_sourcing; returned=$?
1020
if [ $returned -ne 0 ]; then return $returned; fi ### simulating exception handling for an exception that is not caught at this level
1121

12-
22+
driver_path="$1"
23+
validate_driver_binary "$driver_path"
1324

1425
humanReadable_test_pathname="`resolve_symlink_only_of_basename "$0"`"
1526

1627

17-
18-
if ! P4C=`try_to_find_the_driver`; then
19-
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
20-
exit 255
21-
fi
22-
echo "In ''$humanReadable_test_pathname'', using ''$P4C'' as the path to the driver of the P4 compiler." >& 2
23-
24-
25-
2628
BAD_PATHNAME=/path/to/a/nonexistant/supposedly-P4/source/file
2729

28-
"$P4C" $BAD_PATHNAME 2>&1 | grep --ignore-case --quiet "error.*$BAD_PATHNAME"
30+
"$driver_path" $BAD_PATHNAME 2>&1 | grep --ignore-case --quiet "error.*$BAD_PATHNAME"
2931
exit_status_from_grep=$?
3032

3133
if [ $exit_status_from_grep -eq 0 ]; then

tools/driver/test_scripts/driver_inputs_test_2___two_bad_pathnames.bash

Lines changed: 16 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -2,38 +2,43 @@
22

33
### ensure that when _two_ non-existant input pathnames are given, each one results in an error output that mentions that pathname.
44

5+
SCRIPT_NAME=$(basename "$0")
6+
USAGE="Usage: $SCRIPT_NAME <path-to-driver-binary>"
7+
8+
show_usage() {
9+
echo "$USAGE"
10+
}
11+
12+
if [ $# -eq 0 ]; then
13+
echo "Error: No path to driver binary provided."
14+
show_usage
15+
exit 1
16+
fi
517

618

719
source $(dirname "`realpath "${BASH_SOURCE[0]}"`")/driver_inputs_test___shared_code.bash
820

921
check_for_inadvisable_sourcing; returned=$?
1022
if [ $returned -ne 0 ]; then return $returned; fi ### simulating exception handling for an exception that is not caught at this level
1123

24+
driver_path="$1"
25+
validate_driver_binary "$driver_path"
1226

1327

1428
humanReadable_test_pathname="`resolve_symlink_only_of_basename "$0"`"
1529

1630

17-
18-
if ! P4C=`try_to_find_the_driver`; then
19-
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
20-
exit 255
21-
fi
22-
echo "In ''$humanReadable_test_pathname'', using ''$P4C'' as the path to the driver of the P4 compiler." >& 2
23-
24-
25-
2631
BAD_PATHNAME_BASE=/path/to/a/nonexistant/supposedly-P4/source/file
2732
### Technically, these don`t need to be _unique_ pathnames in order to trigger the bad/confusing behavior of the driver that Abe
2833
### saw before he started working on it, but unique pathnames will more helpful for debugging, in case that will ever be needed.
2934
BAD_PATHNAME_1=$BAD_PATHNAME_BASE/1
3035
BAD_PATHNAME_2=$BAD_PATHNAME_BASE/2
3136

3237
### 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.
33-
check_for_pathname_error_in_P4_compiler_driver_output "$P4C" "$BAD_PATHNAME_1" 1 2
38+
check_for_pathname_error_in_P4_compiler_driver_output "$driver_path" "$BAD_PATHNAME_1" 1 2
3439
result_1=$?
3540
echo
36-
check_for_pathname_error_in_P4_compiler_driver_output "$P4C" "$BAD_PATHNAME_2" 2 2
41+
check_for_pathname_error_in_P4_compiler_driver_output "$driver_path" "$BAD_PATHNAME_2" 2 2
3742
result_2=$?
3843
echo
3944

tools/driver/test_scripts/driver_inputs_test_3___three_bad_pathnames.bash

Lines changed: 16 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -2,27 +2,30 @@
22

33
### ensure that when _three_ non-existant input pathnames are given, each one results in an error output that mentions that pathname.
44

5+
SCRIPT_NAME=$(basename "$0")
6+
USAGE="Usage: $SCRIPT_NAME <path-to-driver-binary>"
57

8+
show_usage() {
9+
echo "$USAGE"
10+
}
11+
12+
if [ $# -eq 0 ]; then
13+
echo "Error: No path to driver binary provided."
14+
show_usage
15+
exit 1
16+
fi
617

718
source $(dirname "`realpath "${BASH_SOURCE[0]}"`")/driver_inputs_test___shared_code.bash
819

920
check_for_inadvisable_sourcing; returned=$?
1021
if [ $returned -ne 0 ]; then return $returned; fi ### simulating exception handling for an exception that is not caught at this level
1122

12-
23+
driver_path="$1"
24+
validate_driver_binary "$driver_path"
1325

1426
humanReadable_test_pathname="`resolve_symlink_only_of_basename "$0"`"
1527

1628

17-
18-
if ! P4C=`try_to_find_the_driver`; then
19-
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
20-
exit 255
21-
fi
22-
echo "In ''$humanReadable_test_pathname'', using ''$P4C'' as the path to the driver of the P4 compiler." >& 2
23-
24-
25-
2629
BAD_PATHNAME_BASE=/path/to/a/nonexistant/supposedly-P4/source/file
2730
### Technically, these don`t need to be _unique_ pathnames in order to trigger the bad/confusing behavior of the driver that Abe
2831
### 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
3134
BAD_PATHNAME_3=$BAD_PATHNAME_BASE/3
3235

3336
### 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.
34-
check_for_pathname_error_in_P4_compiler_driver_output "$P4C" "$BAD_PATHNAME_1" 1 3
37+
check_for_pathname_error_in_P4_compiler_driver_output "$driver_path" "$BAD_PATHNAME_1" 1 3
3538
result_1=$?
3639
echo
37-
check_for_pathname_error_in_P4_compiler_driver_output "$P4C" "$BAD_PATHNAME_2" 2 3
40+
check_for_pathname_error_in_P4_compiler_driver_output "$driver_path" "$BAD_PATHNAME_2" 2 3
3841
result_2=$?
3942
echo
40-
check_for_pathname_error_in_P4_compiler_driver_output "$P4C" "$BAD_PATHNAME_3" 3 3
43+
check_for_pathname_error_in_P4_compiler_driver_output "$driver_path" "$BAD_PATHNAME_3" 3 3
4144
result_3=$?
4245
echo
4346

tools/driver/test_scripts/driver_inputs_test_4___two_good_pathnames___check_for_no_misleading_error_messages.bash

Lines changed: 15 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -12,13 +12,26 @@
1212
###
1313
### ... 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].
1414

15+
SCRIPT_NAME=$(basename "$0")
16+
USAGE="Usage: $SCRIPT_NAME <path-to-driver-binary>"
1517

18+
show_usage() {
19+
echo "$USAGE"
20+
}
21+
22+
if [ $# -eq 0 ]; then
23+
echo "Error: No path to driver binary provided."
24+
show_usage
25+
exit 1
26+
fi
1627

1728
source $(dirname "`realpath "${BASH_SOURCE[0]}"`")/driver_inputs_test___shared_code.bash
1829

1930
check_for_inadvisable_sourcing; returned=$?
2031
if [ $returned -ne 0 ]; then return $returned; fi ### simulating exception handling for an exception that is not caught at this level
2132

33+
driver_path="$1"
34+
validate_driver_binary "$driver_path"
2235

2336

2437
output_dir=`mktemp -d /tmp/P4C_driver_testing___XXXXXXXXXX`
@@ -32,7 +45,7 @@ ls -dl p4c
3245
echo
3346
ls -dl p4c*
3447
echo
35-
ls -l
48+
ls -l
3649
echo
3750
echo === env ===
3851
env
@@ -46,15 +59,8 @@ echo '===== ^^^ ===== test debugging ===== ^^^ ====='
4659

4760
humanReadable_test_pathname="`resolve_symlink_only_of_basename "$0"`"
4861

49-
if ! P4C=`try_to_find_the_driver`; then
50-
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
51-
exit 255
52-
fi
53-
echo "In ''$humanReadable_test_pathname'', using ''$P4C'' as the path to the driver of the P4 compiler." >& 2
54-
55-
5662

57-
if [ ! -x "$P4C" ] ; then
63+
if [ ! -x "$driver_path" ] ; then
5864
echo "Test ''$humanReadable_test_pathname'' failed due to not being able to execute ''$P4C''." >& 2
5965
exit 1
6066
elif "$P4C" p4include/core.p4 p4include/pna.p4 -o "$output_dir" 2>&1 | grep --ignore-case --quiet 'unrecognized.*arguments'; then

tools/driver/test_scripts/driver_inputs_test___shared_code.bash

Lines changed: 23 additions & 89 deletions
Original file line numberDiff line numberDiff line change
@@ -62,99 +62,33 @@ function report___num_failures___and_clamp_it_to_an_inclusive_maximum_of_255 {
6262
if [ $num_failures -gt 255 ]; then num_failures=255; fi
6363
}
6464

65-
66-
67-
### requires a single arg./param., which had better be a pathname [doesn`t need to be absolute]
68-
function check_if_this_seems_to_be_our_driver {
69-
echo "INFO: searching for the P4 compiler driver at ''$1''..." >& 2 ### reminder: “>& 2” is equivalent to “> /dev/stderr”
70-
71-
if [ -L "$1" ]; then
72-
### NOTE: is “-L” a GNUism? “-h” might be the POSIX “spelling”.
73-
### Either way, it should work OK in {Bash on non-GNU OS} as long as I use Bash`s built-in ‘[’/“test”,
74-
### not e.g. “/bin/[” or “/bin/test” or “/usr/bin/[” or “/usr/bin/test”.
75-
76-
echo "INFO: detected a symlink at ''$1'' [starting at ''$PWD'', if that is a relative pathname], so checking the target of the symlink." >& 2
77-
if realpath --version | grep --quiet GNU; then ### happy-happy-joy-joy: GNU realpath is available and is the default “realpath”
78-
### try the “logical” interpretation first, i.e. give “../” components priority over symlink components
79-
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 ;-)
80-
if next_turtle=`realpath --canonicalize-existing --physical "$1"`; then check_if_this_seems_to_be_our_driver "$next_turtle"; return; fi
81-
### If we are “still here”, then canonicalization failed. :-(
82-
echo "ERROR: failed to canonicalize the symlink ''$1'' while using GNU ''realpath''." >& 2
83-
return 1
84-
fi
85-
86-
if readlink --version | grep --quiet GNU; then ### second-best: GNU readlink is available and is the default “readlink”
87-
if next_turtle=`readlink --canonicalize-existing "$1"`; then check_if_this_seems_to_be_our_driver "$next_turtle"; return; fi
88-
### If we are “still here”, then canonicalization failed. :-(
89-
echo "ERROR: failed to canonicalize the symlink ''$1'' while using GNU ''readlink''." >& 2
90-
return 2
65+
# Function: Check if the driver binary exists
66+
# Arguments: $1 - path to the file
67+
check_driver_binary_exists() {
68+
local file_path="$1"
69+
if [ ! -e "$file_path" ]; then
70+
echo "Error: The driver binary '$file_path' does not exist."
71+
show_usage
72+
exit 1
9173
fi
74+
}
9275

93-
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)
94-
if next_turtle=`realpath "$1"`; then check_if_this_seems_to_be_our_driver "$next_turtle"; return; fi
95-
### If we are “still here”, then canonicalization failed. :-(
96-
echo "ERROR: failed to canonicalize the symlink ''$1'' while using [presumed non-GNU] ''realpath''." >& 2
97-
return 3
76+
# Function: Check if the driver binary is executable
77+
# Arguments: $1 - path to the file
78+
check_driver_is_executable() {
79+
local file_path="$1"
80+
if [ ! -x "$file_path" ]; then
81+
echo "Error: The driver binary '$file_path' is not executable."
82+
exit 1
9883
fi
84+
}
9985

100-
### The “readlink” in BSD [well, at least the one I _know_ of ;-)] is _extremely_ minimal, and AFAIK can`t do cycle detection,
101-
### so I`m not even going to _try_ non-GNU readlink... mainly because I don`t want to get
102-
### “INFO: searching for the P4 compiler driver”[...] until Bash runs out of function-call stack and crashes,
103-
### in the case of a symlink cycle. At this point, I will just pretend I never detected a symlink and let it go forward.
104-
### This way, if the symlink _is_ part of a cycle, this whole process should crash in a way that is “nicer”
105-
### than flooding the output with “INFO: searching for the P4 compiler driver”[...] lines.
106-
107-
fi ### Done checking for a symlink. At this point, "$1" is either (1) _not_ a symlink or (2) a symlink we couldn`t canonicalize.
108-
109-
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
110-
if [ ! -x "$1" ]; then echo "INFO: not using ''$1'', because it is not executable." >& 2; return 2; fi
111-
if [ -d "$1" ]; then echo "INFO: not using ''$1'', because it is a directory." >& 2; return 3; fi
112-
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
113-
114-
### NOTE on the following: I _could_ be more strict, e.g. requiring that the output of “$foo --version” start with “p4c” or “p4c ”,
115-
### 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,
116-
### e.g. to start with “P4C ” or “Version of P4C: ” instead of with “p4c ” as it is as of this writing
117-
### [and has been for some time, TTBOMK].
118-
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
119-
120-
### OK; at this point, we have given up on finding “reasons” to reject "$1" as a supposedly-good P4 compiler driver. ;-)
121-
echo "INFO: accepting ''$1'' as a presumed P4 compiler driver." >& 2
122-
return 0
123-
} ### end of function “check_if_this_seems_to_be_our_driver”
124-
125-
126-
127-
### IMPORTANT: do _not_ include any human-oriented “fluff” in this function`s standard-out output
128-
function try_to_find_the_driver {
129-
### NOTES re "$GITHUB_WORKSPACE", "$RUNNER_TEMP", and "$RUNNER_WORKSPACE":
130-
### these were all found by Abe in the GitHub CI/CD environment on April 14 2022
131-
###
132-
### here are the values they had at that time [which explains why I am not checking "$RUNNER_TEMP" by itself]:
133-
###
134-
### GITHUB_WORKSPACE=/__w/p4c/p4c
135-
### RUNNER_TEMP=/__w/_temp
136-
### RUNNER_WORKSPACE=/__w/p4c
137-
138-
### 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
139-
to_probe=(./p4c ../p4c ../../p4c ../p4c/p4c ../../p4c/p4c)
140-
if [ -n "$GITHUB_WORKSPACE" ]; then to_probe+=("$GITHUB_WORKSPACE" "$GITHUB_WORKSPACE"/p4c "$GITHUB_WORKSPACE"/p4c/p4c); fi
141-
if [ -n "$RUNNER_WORKSPACE" ]; then to_probe+=("$RUNNER_WORKSPACE" "$RUNNER_WORKSPACE"/p4c "$RUNNER_WORKSPACE"/p4c/p4c); fi
142-
if [ -n "$RUNNER_TEMP" ]; then to_probe+=( "$RUNNER_TEMP"''''//p4c "$RUNNER_TEMP"''''//p4c/p4c); fi
143-
144-
for probe in ${to_probe[@]}; do
145-
if check_if_this_seems_to_be_our_driver "$probe"; then
146-
echo "$probe" ### IMPORTANT: do _not_ include any human-oriented “fluff” in this output
147-
return 0
148-
fi
149-
done
150-
151-
echo "FATAL ERROR: could not find the P4 compiler driver. Searched in the following locations..." >& 2
152-
for probe in ${to_probe[@]}; do
153-
echo "///>>>$probe<<<///" >& 2
154-
### Using “///>>>$foo<<<///” to make it clear that the extra characters are just delimiters
155-
### [which, BTW, are here so that space characters, especially trailing ones, will become visible].
156-
done
157-
return 1
86+
# Validates input and checks if the driver binary is valid
87+
# Arguments: $1 - path to the binary file
88+
validate_driver_binary() {
89+
local binary_path="$1"
90+
check_file_exists "$binary_path"
91+
check_is_executable "$binary_path"
15892
}
15993

16094

0 commit comments

Comments
 (0)