Log build steps in the outcome file #129

2025-01-24T13:05:46-06:00

gilles-peskine-arm commented

2025-01-24 13:05:46 -06:00

(Migrated from github.com)

Add some logging to the output file:

make invocation (including CMake-driven builds): "${MBEDTLS_TEST_PLATFORM};${MBEDTLS_TEST_CONFIGURATION};${TOOL};${target};${result};${cause}"
for each component: "${MBEDTLS_TEST_PLATFORM};${component_name};all.sh;${component_name};${result};"

Extracted from https://github.com/Mbed-TLS/mbedtls/pull/9286.

This is supposed to be transparent for a consuming branch, so we can merge it at our leisure.

Needs preceding PR: https://github.com/Mbed-TLS/mbedtls/pull/9932 and https://github.com/Mbed-TLS/mbedtls/pull/9933 to get the CI to pass.

Follow-up: https://github.com/Mbed-TLS/mbedtls/pull/9934

PR checklist

Please add the numbers (or links) of the associated pull requests for consuming branches. You can omit branches where this pull request is not needed.

crypto PR not needed
development PR not needed
3.6 PR not needed
2.28 PR no: we won't bother to backport this to a branch that's almost out of support.

Add some logging to the output file: * `make` invocation (including CMake-driven builds): `"${MBEDTLS_TEST_PLATFORM};${MBEDTLS_TEST_CONFIGURATION};${TOOL};${target};${result};${cause}"` * for each component: `"${MBEDTLS_TEST_PLATFORM};${component_name};all.sh;${component_name};${result};"` Extracted from https://github.com/Mbed-TLS/mbedtls/pull/9286. This is supposed to be transparent for a consuming branch, so we can merge it at our leisure. Needs preceding PR: https://github.com/Mbed-TLS/mbedtls/pull/9932 and https://github.com/Mbed-TLS/mbedtls/pull/9933 to get the CI to pass. Follow-up: https://github.com/Mbed-TLS/mbedtls/pull/9934 ## PR checklist Please add the numbers (or links) of the associated pull requests for consuming branches. You can omit branches where this pull request is not needed. - [x] **crypto PR** not needed - [x] **development PR** not needed - [x] **3.6 PR** not needed - [x] **2.28 PR** no: we won't bother to backport this to a branch that's almost out of support.

minosgalanakis (Migrated from github.com) reviewed

2025-04-24 06:04:58 -05:00

minosgalanakis (Migrated from github.com) left a comment

Some review comments at this stage, nothing major.

scripts/quiet/make

					
				@ -20,0 +43,4 @@

				            --directory=*|--file=*|-C?*|-f?*) # Modifier option with its argument

				                modifier="$modifier$arg ";;

				            --directory|--file|-C|-f) # Modifier option with separate argument

				                skip=modifier

Why are both "--directory=somedir" and "--directory somedir" required?

For example would passing "--directory" and nothing else assert skip=1?

Why are both "--directory=somedir" and "--directory somedir" required? For example would passing "--directory" and nothing else assert `skip=1`?

scripts/quiet/make

Those are temporary target files that will be cleaned up. So the logic is sound. It is not obscure but standard even though unfortunately undocumented behavior from cmake. ;(

scripts/quiet/quiet.sh

Why the indirection?

If we want to preserve the status, could't we just return "EXIT_STATUS" ? What am I missing?

Why the indirection? If we want to preserve the status, could't we just return "EXIT_STATUS" ? What am I missing?

gilles-peskine-arm (Migrated from github.com) reviewed

2025-04-24 06:14:06 -05:00

scripts/quiet/quiet.sh

make includes (“sources”) quiet.sh with the . command. Calling exit inside quiet.sh would exit the whole program. This was ok until now because make didn't want to do anything else in this case. But now make wants to run additional logging code. So we have to return from quiet.sh, and communicate the status.

Note that in sh, there is no language feature to return early from a sub-script sourced with the . command. In ksh, bash and zsh, you can do it with the return builtin, but that extension is not available in a basic sh shell such as dash.

`make` includes (“sources”) `quiet.sh` with the `.` command. Calling `exit` inside `quiet.sh` would exit the whole program. This was ok until now because `make` didn't want to do anything else in this case. But now `make` wants to run additional logging code. So we have to return from `quiet.sh`, and communicate the status. Note that in sh, there is no language feature to return early from a sub-script sourced with the `.` command. In ksh, bash and zsh, you can do it with the `return` builtin, but that extension is not available in a basic sh shell such as dash.

gilles-peskine-arm (Migrated from github.com) reviewed

2025-04-24 06:16:11 -05:00

scripts/quiet/make

					
				@ -20,0 +43,4 @@

				            --directory=*|--file=*|-C?*|-f?*) # Modifier option with its argument

				                modifier="$modifier$arg ";;

				            --directory|--file|-C|-f) # Modifier option with separate argument

				                skip=modifier

With --directory=..., we want to remember the option and move on to the next argument which is another option or a non-option argument. With --directory, we want to skip the next argument, which is the argument of the --directory option.

make --directory is invalid. I don't know what the code here does in this case, but I don't think this wrapper needs to care about invalid usage of make.

With `--directory=...`, we want to remember the option and move on to the next argument which is another option or a non-option argument. With `--directory`, we want to skip the next argument, which is the argument of the `--directory` option. `make --directory` is invalid. I don't know what the code here does in this case, but I don't think this wrapper needs to care about invalid usage of `make`.

This pull request is broken due to missing fork information.

View command line instructions

Checkout

From your project repository, check out a new branch and test the changes.

git fetch -u origin 'gilles-peskine-arm/outcome-log-make-3.6-framework:gilles-peskine-arm/outcome-log-make-3.6-framework'

git switch 'gilles-peskine-arm/outcome-log-make-3.6-framework'

Merge

Merge the changes and update on Forgejo.

Warning: The "Autodetect manual merge" setting is not enabled for this repository, you will have to mark this pull request as manually merged afterwards.

git switch 'main'

git merge --no-ff 'gilles-peskine-arm/outcome-log-make-3.6-framework'

git switch 'gilles-peskine-arm/outcome-log-make-3.6-framework'

git rebase 'main'

git switch 'main'

git merge --ff-only 'gilles-peskine-arm/outcome-log-make-3.6-framework'

git switch 'gilles-peskine-arm/outcome-log-make-3.6-framework'

git rebase 'main'

git switch 'main'

git merge --no-ff 'gilles-peskine-arm/outcome-log-make-3.6-framework'

git switch 'main'

git merge --squash 'gilles-peskine-arm/outcome-log-make-3.6-framework'

git switch 'main'

git merge --ff-only 'gilles-peskine-arm/outcome-log-make-3.6-framework'

git switch 'main'

git merge 'gilles-peskine-arm/outcome-log-make-3.6-framework'

git push origin 'main'

Sign in to join this conversation.

No reviewers

No milestone

No assignees

1 participant

Notifications

Due date

The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set

Reference

gh-Mbed-TLS/mbedtls-framework!129

Rows
Columns