Modify path handling in some build scripts to be closer to mbedtls #376

Merged

gilles-peskine-arm merged 7 commits from paths-mergeability-crypto into development

2020-03-04 04:20:00 -06:00

gilles-peskine-arm commented

2020-02-26 07:48:10 -06:00

(Migrated from github.com)

Mostly this aligns the Visual Studio templates with mbedtls and makes the handling of directories in generate_visual_studio.pl more generic.

There is no completeness goal in this pull request, only a goal of non-harmfulness (no semantic change).

Mostly this aligns the Visual Studio templates with mbedtls and makes the handling of directories in `generate_visual_studio.pl` more generic. There is no completeness goal in this pull request, only a goal of non-harmfulness (no semantic change).

mpg (Migrated from github.com) reviewed

2020-02-28 03:27:28 -06:00

mpg (Migrated from github.com) left a comment

No description provided.

I checked that the PR is not harmful and doesn't make any semantic change; the resulting code is more generic and cleaner too.

mpg (Migrated from github.com) approved these changes

2020-03-03 02:37:23 -06:00

mpg (Migrated from github.com) left a comment

LGTM

By the way over the long run perhaps we should have a rule about ordering of the components in all.sh (could be as simple as lexical order, which would also naturally group build/check/test together). But that's obviously out of scope for this PR.

LGTM By the way over the long run perhaps we should have a rule about ordering of the components in `all.sh` (could be as simple as lexical order, which would also naturally group build/check/test together). But that's obviously out of scope for this PR.

simonbutcher (Migrated from github.com) approved these changes

2020-03-03 09:18:51 -06:00

simonbutcher (Migrated from github.com) left a comment

I'm fine with the PR. There's one minor pre-existing issue, but I don't think it's worth rework, unless there's some being done anyway, so approving.

scripts/generate_visualc_files.pl

Minor - Pre-existing really, but the error message of "Must but run from mbedTLS root or scripts dir" doesn't quite match the failure condition, which is if a required directory is not found, the script fails.

I suggest the error message be changed to:

"Necessary source directory not found."
"This script must be but run from the mbedtls root directory or its scripts directory."

Minor - Pre-existing really, but the error message of `"Must but run from mbedTLS root or scripts dir"` doesn't quite match the failure condition, which is if a required directory is not found, the script fails. I suggest the error message be changed to: ``` "Necessary source directory not found." "This script must be but run from the mbedtls root directory or its scripts directory." ```

mpg commented

2020-03-04 03:24:17 -06:00

(Migrated from github.com)

I checked the CI results and the only failures are in mbed-os tests, which are known and expected to fail at this point, so can be ignored for this PR. Therefore I'm removing "needs: ci" and labeling "ready for merge".