Skip to content

Conversation

@arthur-she
Copy link

Modify the build script to fit two CIs, Trustedfirmware OpenCI and ARM internal CI.

This patch updates docker's run.sh with the aforementioned parameter
which disables the ASLR for zeroize test. The readme.md file has
been updated accordingly
This patch updates the ubuntu-16.04 & ubuntu-18.04 Dockerfiles
to use the up-to-date license license information, and removes the
workaround in gen_jobs.groovy script.
This patch installs the official distribution selected packages for
`abi-dumper` -> (version>=1.1-1) and
`abi-compliance-checker` -> (version>=2.3-0.2)
This patch updates GnuTLS 3.7 configure command to use the
distribution’s `libtasn1` -> (version>=4.13).

Comments have been amended accordingly
This patch updates ubuntu18-04 Dockerfiles to
compile abi-compliance-checker 2.3 from source.

Both files are slightly modified to remove
unessary apt-get updates when using the package
manager.

It also adds a note to OpenSSL comments, to
serve as reminder of the optional
`--with-included-libtasn1` parameter.
@arthur-she
Copy link
Author

Test PR: arthur-she/mbedtls#4
Test job:
PR head
PR merge

@arthur-she
Copy link
Author

Hi @yanesca ,
Could you please have a look?

Thank you

Arthur

@yanesca yanesca added enhancement New feature or request needs: review needs: reviewer Product Backlog size-s Estimated task size: small (~2d) labels Jan 31, 2022
@bensze01 bensze01 self-requested a review January 31, 2022 11:13
Copy link
Contributor

@bensze01 bensze01 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Most of the changes seem reasonable, but removing the static qualifier will break our jobs on the internal CI.

@arthur-she arthur-she force-pushed the leo-mbedtls-open-ci-notify-github branch 2 times, most recently from db86f5a to 5430592 Compare February 2, 2022 04:22
Copy link
Contributor

@gilles-peskine-arm gilles-peskine-arm left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

On code inspection, this looks correct (for the Arm CI), but slightly overcomplicated in a couple of places.

Test runs on Arm CI:

I haven't reviewed the suitability for Open CI.

Copy link
Contributor

@bensze01 bensze01 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Agree with Gilles, this looks good overall. Just one additional warning I'd like to fix on the OpenCI side.

@arthur-she arthur-she force-pushed the leo-mbedtls-open-ci-notify-github branch 5 times, most recently from 8d7dd05 to 19206df Compare February 5, 2022 05:23
minosgalanakis and others added 11 commits February 15, 2022 22:37
This patch updates the ubuntu-16.04 & ubuntu-18.04 Dockerfiles
to use the up-to-date license license information, and removes the
workaround in gen_jobs.groovy script.
This patch installs the official distribution selected packages for
`abi-dumper` -> (version>=1.1-1) and
`abi-compliance-checker` -> (version>=2.3-0.2)
This patch updates GnuTLS 3.7 configure command to use the
distribution’s `libtasn1` -> (version>=4.13).

Comments have been amended accordingly
This patch updates ubuntu18-04 Dockerfiles to
compile abi-compliance-checker 2.3 from source.

Both files are slightly modified to remove
unessary apt-get updates when using the package
manager.

It also adds a note to OpenSSL comments, to
serve as reminder of the optional
`--with-included-libtasn1` parameter.
TF OpenCI: job runs on the OpenCI
Internal CI: job runs on the ARM internal CI

Signed-off-by: Arthur She <[email protected]>
Let ARMLMD_LICENSE_FILE could be overwritten on the build time

Signed-off-by: Arthur She <[email protected]>
two CI systems

Signed-off-by: Arthur She <[email protected]>
(cherry picked from commit 355bca2)
@arthur-she arthur-she force-pushed the leo-mbedtls-open-ci-notify-github branch from a866213 to 2978eb9 Compare February 16, 2022 06:38
@gilles-peskine-arm
Copy link
Contributor

We've merged #31. Sorry it took us a bit longer than anticipated. There are conflicts in the Docker files now, so please rebase this pull request.

Leonardo Sandoval and others added 5 commits February 22, 2022 13:41
TF OpenCI: job runs on the OpenCI
Internal CI: job runs on the ARM internal CI

Signed-off-by: Arthur She <[email protected]>
Let ARMLMD_LICENSE_FILE could be overwritten on the build time

Signed-off-by: Arthur She <[email protected]>
two CI systems

Signed-off-by: Arthur She <[email protected]>
(cherry picked from commit 355bca2)
…-she/mbedtls-test into leo-mbedtls-open-ci-notify-github
@arthur-she
Copy link
Author

Hi @gilles-peskine-arm ,
Thank you so much.
I've rebased my change.
Please have a look.

@gilles-peskine-arm
Copy link
Contributor

I'm afraid the rebase doesn't look right. You're re-applying some of Minos's changes that were later overridden. Can you please rebase your own changes on top of e1a6125? The commit list shouldn't have Minos's commits anymore.

@arthur-she
Copy link
Author

Sorry about that.
Since this branch is a mess and hard to rebase, I would prefer to close this pr and create another one.

@arthur-she arthur-she closed this Feb 24, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement New feature or request needs: work size-s Estimated task size: small (~2d)

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants