Skip to content

Change to Dockerfiles that prevents them from masking failed builds - #670

Merged
jafingerhut merged 16 commits into
p4lang:mainfrom
jafingerhut:fix-dockerfile-masking-of-command-failures1
Sep 9, 2026
Merged

Change to Dockerfiles that prevents them from masking failed builds#670
jafingerhut merged 16 commits into
p4lang:mainfrom
jafingerhut:fix-dockerfile-masking-of-command-failures1

Conversation

@jafingerhut

Copy link
Copy Markdown
Contributor

No description provided.

Signed-off-by: Andy Fingerhut <andy_fingerhut@alum.wustl.edu>
@jafingerhut

Copy link
Copy Markdown
Contributor Author

The dockerfiles have been written in a way, since 2017, that commands that fail in the main build command sequence are masked, and the dockerfile build command has a successful exit status of 0 overall, even though the build failed.

This is because the structure of the commands was:

a && b && c && d && e || f

The operators && and || are left-associative with equal precedence, so the above is equivalent to:

(((((a && b) && c) && d) && e) || f

If any of a, b, c, d fail, the remaining commands up to e are skipped, but f is still executed, and f's exit status becomes the exit status of the entire sequence of commands. Command f in this case is a command that always succeeds.

The fix is to put (e || f) into a subshell, so that it is treated as a single command as far as the top level command sequence is concerned.

Signed-off-by: Andy Fingerhut <andy_fingerhut@alum.wustl.edu>
@github-actions github-actions Bot added the github_actions Pull requests that update GitHub Actions code label Sep 5, 2026
Signed-off-by: Andy Fingerhut <andy_fingerhut@alum.wustl.edu>
Signed-off-by: Andy Fingerhut <andy_fingerhut@alum.wustl.edu>
Signed-off-by: Andy Fingerhut <andy_fingerhut@alum.wustl.edu>
Signed-off-by: Andy Fingerhut <andy_fingerhut@alum.wustl.edu>
Signed-off-by: Andy Fingerhut <andy_fingerhut@alum.wustl.edu>
Signed-off-by: Andy Fingerhut <andy_fingerhut@alum.wustl.edu>
Signed-off-by: Andy Fingerhut <andy_fingerhut@alum.wustl.edu>
Signed-off-by: Andy Fingerhut <andy_fingerhut@alum.wustl.edu>
@jafingerhut

Copy link
Copy Markdown
Contributor Author

Just a note that as of commit 10 of this PR, it passes all tests, and keeps all configuration options to PI enabled, by means of a few small changes to the C++ code.

It does not yet at that point have any changes recommended by Fabian Ruffy to move the installation of Python packages from the test step of test.yml into the build step (within Dockerfile.bmv2). It also does not yet use uv for installing Python packages.

Signed-off-by: Andy Fingerhut <andy_fingerhut@alum.wustl.edu>
Signed-off-by: Andy Fingerhut <andy_fingerhut@alum.wustl.edu>
Signed-off-by: Andy Fingerhut <andy_fingerhut@alum.wustl.edu>
Signed-off-by: Andy Fingerhut <andy_fingerhut@alum.wustl.edu>
Signed-off-by: Andy Fingerhut <andy_fingerhut@alum.wustl.edu>
@jafingerhut

jafingerhut commented Sep 5, 2026

Copy link
Copy Markdown
Contributor Author

As of commit 15, all CI tests are green, but I think that is because some failures during the "Run tests" step in test.yml are being masked by the cmd1 || cmd2 || cmd3 style of combining shell commands, where cmd1 is failing but things proceed without failure anyway. I know how to change those commands so that the failures are not masked, but I do not yet know why the tests are failing.

@fruffy If you are willing to wait for fixes to the failing tests after this PR, I'm happy to create an issue to track fixing them, or at least determining their root cause and disabling them, in a follow-up PR later.

I consider this PR to replace my earlier one #669 that fruffy gave several useful comments on. I believe this one addresses all of the comments he made on that one.

Also install Python packages in Dockerfile, as well as Dockerfile.bmv2

Signed-off-by: Andy Fingerhut <andy_fingerhut@alum.wustl.edu>
@jafingerhut
jafingerhut merged commit 0e9ffd5 into p4lang:main Sep 9, 2026
6 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

github_actions Pull requests that update GitHub Actions code

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants