Get rid of "list (internal) identifiers" scripts #92

New issue

Open

opened 2024-12-04 05:30:43 -06:00 by mpg · 1 comment

mpg commented

2024-12-04 05:30:43 -06:00

(Migrated from github.com)

Currently, abi_check.py relies on the user (inc. CI) to generate a list of internal identifiers and pass the resulting file as the argument to the command-line option -s / --skip-file. On the CI, this is done by calling tests/scripts/list-identifiers.sh --internal which is a thing wrapper around tests/scripts/list_internal_identifiers.py which itself has more lines of boilerplate than actual code (8 lines to get the list, everything else to parse command line and write to a file).

It would be more user-friendly, less error-prone and more maintainer-friendly to cut the middlemen and have abi_check.py get the list of files by itself. Suggesting a 3-step plan to do this.

Step 1: Change the groovy script on the CI to something like

if [ -x tests/scripts/list-identifiers.sh ]; then
    tests/scripts/list-identifiers.sh --internal
    scripts/abi_check.py -o FETCH_HEAD -n HEAD -s identifiers --brief
else
    scripts/abi_check.py -o FETCH_HEAD -n HEAD --brief
fi

(with an appropriate comment).

Step 2:

Have abi_check.py ignore the skipfile and instead generate the list itself by using the relevant code from list_internal_identifiers.py. The list should be generated using the old revision.
Remove the -s option of abi_check.py.
Remove tests/scripts/list-identifiers.sh.
Remove tests/scripts/list_internal_identifiers.py and change the reference to it in check_names.py to refer to abi_check.py instead.

Step 3: When enough time has passed (that is, when the last release of each maintained branch has the new code), remove the if from step 2, keeping only the else path.

This issue is only about doing steps 1-2, then an issue can be created in mbedtls-test to track step 3.

Currently, `abi_check.py` relies on the user (inc. CI) to generate a list of internal identifiers and pass the resulting file as the argument to the command-line option `-s` / `--skip-file`. On the CI, this is done by calling `tests/scripts/list-identifiers.sh --internal` which is a thing wrapper around `tests/scripts/list_internal_identifiers.py` which itself has more lines of boilerplate than actual code (8 lines to get the list, everything else to parse command line and write to a file). It would be more user-friendly, less error-prone and more maintainer-friendly to cut the middlemen and have `abi_check.py` get the list of files by itself. Suggesting a 3-step plan to do this. Step 1: Change the groovy script on the CI to something like ``` if [ -x tests/scripts/list-identifiers.sh ]; then tests/scripts/list-identifiers.sh --internal scripts/abi_check.py -o FETCH_HEAD -n HEAD -s identifiers --brief else scripts/abi_check.py -o FETCH_HEAD -n HEAD --brief fi ``` (with an appropriate comment). Step 2: - Have `abi_check.py` ignore the skipfile and instead generate the list itself by using the relevant code from `list_internal_identifiers.py`. The list should be generated using the old revision. - Remove the `-s` option of `abi_check.py`. - Remove `tests/scripts/list-identifiers.sh`. - Remove `tests/scripts/list_internal_identifiers.py` and change the reference to it in `check_names.py` to refer to `abi_check.py` instead. Step 3: When enough time has passed (that is, when the last release of each maintained branch has the new code), remove the `if` from step 2, keeping only the `else` path. This issue is only about doing steps 1-2, then an issue can be created in mbedtls-test to track step 3.

mpg commented

2024-12-24 04:57:52 -06:00

(Migrated from github.com)

Note: I had started working on this but won't continue. In case it can be useful, I'm sharing the proof-of-concept I had working locally for part of step 2: https://github.com/mpg/mbedtls/tree/abi-check-identifiers

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#92

No description provided.

Rows
Columns