Skip to content

Conversation

@pkolbus
Copy link
Contributor

@pkolbus pkolbus commented Feb 2, 2019

Description

When all.sh invokes check_headers_in_cpp, a backup config.h exists. This
causes a stray difference vs cpp_dummy_build.cpp. Fix by only collecting
the *.h files in include/mbedtls.

Fixes #2406.

Status

READY

Requires Backporting

[edited by mpg]

Migrations

If there is any API change, what's the incentive and logic for it.

NO

Additional comments

Any additional information that could be of interest

Todos

  • Tests
  • Documentation
  • Changelog updated
  • Backported

Steps to test or reproduce

./tests/scripts/all.sh build_default_make_gcc_and_cxx

When all.sh invokes check_headers_in_cpp, a backup config.h exists. This
causes a stray difference vs cpp_dummy_build.cpp. Fix by only collecting
the *.h files in include/mbedtls.

Change-Id: Ifd415027e856858579a6699538f06fc49c793570
@pkolbus pkolbus mentioned this pull request Feb 2, 2019
4 tasks
@RonEld RonEld added bug CLA valid needs-review Every commit must be reviewed by at least two team members, component-platform Portability layer and build scripts labels Feb 3, 2019
Copy link
Contributor

@RonEld RonEld left a comment

Choose a reason for hiding this comment

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

Code LGTM, however a ChangeLog entry is needed

@RonEld RonEld requested a review from k-stachowiak February 3, 2019 07:50
@pkolbus pkolbus force-pushed the fix-cpp-check-headers branch from 23a355c to 995d5c1 Compare February 3, 2019 15:02
Copy link
Contributor

@RonEld RonEld left a comment

Choose a reason for hiding this comment

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

LGTM

Copy link
Contributor

@mpg mpg left a comment

Choose a reason for hiding this comment

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

Looks good to me. Thanks for fixing this.

@mpg mpg removed the needs-review Every commit must be reviewed by at least two team members, label Feb 4, 2019
@mpg
Copy link
Contributor

mpg commented Feb 4, 2019

Note: I've taken the liberty of editing the PR description regarding backporting, which happens not to be needed here.

@mpg
Copy link
Contributor

mpg commented Feb 4, 2019

This has two approving reviews and a passing CI, a valid CLA, fixes a legitimate bug with an appropriate ChangeLog entry and is not waiting for backports, so labeling as ready for merge.

@mpg mpg added the approved Design and code approved - may be waiting for CI or backports label Feb 4, 2019
@Patater
Copy link
Contributor

Patater commented Feb 4, 2019

This PR didn't apply cleanly on mbedtls-2.16, so I've raised a backport PR at #2412

Please review. Thanks.

@Patater Patater added the needs-backports Backports are missing or are pending review and approval. label Feb 4, 2019
@pkolbus
Copy link
Contributor Author

pkolbus commented Feb 4, 2019

@mpg @Patater Thanks, LGTM

@mpg mpg removed the needs-backports Backports are missing or are pending review and approval. label Feb 11, 2019
@k-stachowiak k-stachowiak removed their request for review February 20, 2019 08:39
@Patater Patater merged commit 995d5c1 into Mbed-TLS:development Feb 25, 2019
@pkolbus pkolbus deleted the fix-cpp-check-headers branch February 26, 2019 22:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

approved Design and code approved - may be waiting for CI or backports bug component-platform Portability layer and build scripts

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants