Add earliest and latest compilers #118

gowthamsk-arm · 2023-07-20T22:46:45Z

This commit updates the Ubuntu 16.04 docker file to include the earliest versions of GCC (4.7) and Clang (3.5) available in 16.04. For Ubuntu 22.04 the latest compiler versions of GCC (12) and Clang (16) are installed.

Also creates couple of symlinks of those compilers for simplified usage in the all.sh script.

all.sh will start using the new compiler versions in Mbed-TLS/mbedtls#7968 and Mbed-TLS/mbedtls#7969.

Resolves #110

gilles-peskine-arm · 2023-07-21T10:27:11Z

Test jobs on OpenCI:

development → PASS
mbedtls-2.28 → PASS
use_earliest_latest_compilers → PASS, the log from the new components looks good
use_earliest_latest_compilers_2.28 → PASS, the log from the new components looks good

Test jobs on internal CI:

use_earliest_latest_compilers → PASS
use_earliest_latest_compilers-2.28 → PASS

gilles-peskine-arm

A few minor change requests.

The test results look good, but we'll have to re-run at least some after updating the docker files.

gilles-peskine-arm · 2023-07-25T18:37:46Z

resources/docker_files/ubuntu-16.04/Dockerfile

-    rm -rf /var/lib/apt/lists/
+    rm -rf /var/lib/apt/lists/ && \
+    # create symbolic links for earliest gcc and clang versions
+    ln -s /usr/bin/gcc-4.7 /usr/bin/gcc-earliest && \


Please create the symlinks in /usr/local/bin. /usr is for what comes with the distribution. Also applies in the 22.04 dockerfile.

gilles-peskine-arm · 2023-07-25T18:39:58Z

resources/docker_files/ubuntu-22.04/Dockerfile


+RUN apt-get update -q && apt-get install -yq \
+    # provides some useful scripts for adding and removing repositories
+    software-properties-common && \


Please group all the installations from the standard repository in the same batch. It makes the file easier to read and the caching of intermediate steps more efficient.

gilles-peskine-arm · 2023-07-25T18:40:25Z

resources/docker_files/ubuntu-22.04/Dockerfile

+    apt-get install -yq \
+    # to build Mbed TLS using latest clang version
+    clang-16 \
+    # to build Mbed TLS using latest gcc version


More precisely, latest gcc version available from Ubuntu.

gilles-peskine-arm · 2023-07-25T18:41:16Z

resources/docker_files/ubuntu-22.04/Dockerfile

+    # add an apt package source from https://apt.llvm.org/ to install clang-16
+    add-apt-repository 'deb http://apt.llvm.org/jammy/ llvm-toolchain-jammy-16 main' -y && \
+    apt-get install -yq \
+    # to build Mbed TLS using latest clang version


More precisely, the latest clang version at the time of writing. (As opposed to: at the time the docker image is built, or at the time the tests run.)

This commit updates the Ubuntu 16.04 dockerfile to include the earliest versions of GCC (4.7) and Clang (3.5) available in 16.04. For Ubuntu 22.04 the latest compiler versions of GCC (12) and Clang (16) are installed. Also creates couple of symlinks of those compilers for simplified usage in the all.sh. Signed-off-by: Gowtham Suresh Kumar <[email protected]>

Signed-off-by: Gowtham Suresh Kumar <[email protected]>

gowthamsk-arm · 2023-07-27T07:22:24Z

Test jobs on OpenCI:

development → PASS
mbedtls-2.28 → PASS
use_earliest_latest_compilers → PASS
use_earliest_latest_compilers_2.28 → PASS

Test jobs on internal CI:

use_earliest_latest_compilers → PASS
use_earliest_latest_compilers-2.28 → PASS

tgonzalezorlandoarm

LGTM

gilles-peskine-arm

Just a few readability issues

gilles-peskine-arm · 2023-07-27T12:48:48Z

resources/docker_files/ubuntu-22.04/Dockerfile

        # to build Mbed TLS with MBEDTLS_ZILIB_SUPPORT (removed in 3.0)
        zlib1g-dev \
+        # to build Mbed TLS using latest gcc version available from Ubuntu
+        gcc-12 \


Please keep the list in alphabetical order, it makes it easier to eyeball what's in there.

Will fix it.

gilles-peskine-arm · 2023-07-27T12:51:18Z

resources/docker_files/ubuntu-22.04/Dockerfile

+# For installing clang-16, we add the llvm package source and add corresponding key 
+RUN apt-key adv --keyserver hkp://keyserver.ubuntu.com:80 --recv 15CF4D18AF4F7421 \
+        # add an apt package source from https://apt.llvm.org/ 
+        &&  add-apt-repository 'deb http://apt.llvm.org/jammy/ llvm-toolchain-jammy-16 main' -y \


Please be consistent about putting && at the end or beginning of the line. In this file the convention is end of line.

Will change it.

gilles-peskine-arm · 2023-07-27T12:52:56Z

resources/docker_files/ubuntu-22.04/Dockerfile

+
+# For installing clang-16, we add the llvm package source and add corresponding key 
+RUN apt-key adv --keyserver hkp://keyserver.ubuntu.com:80 --recv 15CF4D18AF4F7421 \
+        # add an apt package source from https://apt.llvm.org/ 


Please use more indentation when continuing arguments of a command than when starting a new command after &&. Following the established pattern, this line should be intended to 4 spaces, and the list of arguments to apt-get install should be intented to 8 spaces.

gilles-peskine-arm · 2023-07-27T13:17:59Z

Note: in the future, please address review comments in new commits, rather than by pushing a new history, unless there's a good reason to overwrite the existing history.

Signed-off-by: Gowtham Suresh Kumar <[email protected]>

gilles-peskine-arm

Looks good to me on code review. I'll check the CI logs when it's run on the latest release (no need to re-run all jobs, but at least build the image on both jenkins instances).

gowthamsk-arm · 2023-07-27T13:46:47Z

Thanks for the review :)
I will trigger a job on both internal and external CIs.

gowthamsk-arm · 2023-07-27T13:51:49Z

Triggered.
Internal CI run: https://jenkins-mbedtls.oss.arm.com/job/mbedtls-release-ci-testing/521/
External Ci run: https://mbedtls.trustedfirmware.org/job/mbedtls-release-ci-testing/27/

gowthamsk-arm · 2023-07-28T08:09:50Z

Both internal and external CI runs are green :)

gowthamsk-arm added enhancement New feature or request needs: ci size-s Estimated task size: small (~2d) priority-medium labels Jul 20, 2023

gowthamsk-arm self-assigned this Jul 20, 2023

This was referenced Jul 21, 2023

Backport 2.28: Use earliest latest compilers 2.28 Mbed-TLS/mbedtls#7969

Merged

Use earliest latest compilers Mbed-TLS/mbedtls#7968

Merged

gowthamsk-arm marked this pull request as ready for review July 25, 2023 13:36

gowthamsk-arm added needs: review needs: reviewer and removed needs: ci labels Jul 25, 2023

gilles-peskine-arm requested changes Jul 25, 2023

View reviewed changes

gilles-peskine-arm added needs: work and removed needs: review needs: reviewer labels Jul 25, 2023

gowthamsk-arm added 2 commits July 26, 2023 15:24

Update path in README

2c78a19

Signed-off-by: Gowtham Suresh Kumar <[email protected]>

gowthamsk-arm force-pushed the dev/gowsur01/add-latest-compilers branch from bd7c707 to 2c78a19 Compare July 26, 2023 14:28

gilles-peskine-arm added needs: review needs: reviewer and removed needs: work labels Jul 27, 2023

tgonzalezorlandoarm self-requested a review July 27, 2023 10:41

tgonzalezorlandoarm removed the needs: reviewer label Jul 27, 2023

tgonzalezorlandoarm previously approved these changes Jul 27, 2023

View reviewed changes

gilles-peskine-arm requested changes Jul 27, 2023

View reviewed changes

gilles-peskine-arm added needs: work and removed needs: review labels Jul 27, 2023

Fix indentation and minor issues

5bcad04

Signed-off-by: Gowtham Suresh Kumar <[email protected]>

gowthamsk-arm dismissed tgonzalezorlandoarm’s stale review via 5bcad04 July 27, 2023 13:38

gilles-peskine-arm approved these changes Jul 27, 2023

View reviewed changes

tgonzalezorlandoarm approved these changes Jul 27, 2023

View reviewed changes

gowthamsk-arm removed the needs: work label Jul 28, 2023

gilles-peskine-arm added the approved Approved in review. May need additional CI. label Jul 28, 2023

gilles-peskine-arm merged commit c91b434 into master Jul 28, 2023

Add earliest and latest compilers #118

Add earliest and latest compilers #118

Uh oh!

Conversation

gowthamsk-arm commented Jul 20, 2023 • edited by gilles-peskine-arm Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

gilles-peskine-arm commented Jul 21, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

gilles-peskine-arm left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

gowthamsk-arm commented Jul 27, 2023

Uh oh!

tgonzalezorlandoarm left a comment

Choose a reason for hiding this comment

Uh oh!

gilles-peskine-arm left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

gilles-peskine-arm commented Jul 27, 2023

Uh oh!

gilles-peskine-arm left a comment

Choose a reason for hiding this comment

Uh oh!

gowthamsk-arm commented Jul 27, 2023

Uh oh!

gowthamsk-arm commented Jul 27, 2023

Uh oh!

gowthamsk-arm commented Jul 28, 2023

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

gowthamsk-arm commented Jul 20, 2023 •

edited by gilles-peskine-arm

Loading

gilles-peskine-arm commented Jul 21, 2023 •

edited

Loading