-
Notifications
You must be signed in to change notification settings - Fork 2.6k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Fix fragility issues in all.sh #2031
Fix fragility issues in all.sh #2031
Conversation
The clean rule was not using the correct names for the compiled executable files.
Add record_status in front of the invocation of several scripts where it was missing.
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
Should all commits be backported? Which ones depend on #930 being backported first?
f12f9ca
I added one commit for another related issue that I noticed while doing the backports. I'll start CI runs of |
I made another commit that will allow us to run the CI with ASLR enforced. |
Generate the documentation from include and doxygen/input only. Don't get snared by files containing Doxygen comments that lie in other directories such as tests, yotta, crypto/include, ... The only difference this makes in a fresh checkout is that the documentation no longer lists target_config.h. This file is from yotta, does not contain any Doxygen comment, and its inclusion in the rendered documentation was clearly an oversight.
Add if_build_succeeded in front of the invocation of some test runs where it was missing.
test -s can't fail if the subsequent grep succeeds.
We don't need to disable ASLR, so don't try. If gdb tries but fails, the test runs normally, but all.sh then trips up because it sees `warning: Error disabling address space randomization: Operation not permitted` and interprets it as an error that indicates a test failure.
d4e40f8
25e6c1d
to
d4e40f8
Compare
@Patater @sbutcher-arm I rewrote the commit that modifies |
New run of |
tests/scripts/check-files.py
Outdated
self.excluded_paths = list(map(os.path.normpath, [ | ||
'cov-int', | ||
'examples', | ||
'yotta/module' |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The commit message said yotta/modules
. Should this be module
or modules
? Update here or the commit message to be correct.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's module
. I amended the last commit to fix the commit message.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The PR to remove yotta support will be merged before this one. I don't think you need that line.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
And as an aside - there's no need to block it because check-files.py
passes that directory in our repo anyway.
Exclude ".git" directories anywhere. This avoids spurious errors in git checkouts that contain branch names that look like a file check-files.py would check. Fix Mbed-TLS#1713 Exclude "mbed-os" anywhere and "examples" from the root. Switch to the new mechanism to exclude "yotta/module". These are directories where we store third-party files that do not need to match our preferences. Exclude "cov-int" from the root. Fix Mbed-TLS#1691
d4e40f8
to
95c5575
Compare
I took the liberty of editing the PR description by adding "fix" in front of an issue number so that github picks it up as fixed by this PR. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think you should remove yotta support, but it's not a blocker.
tests/scripts/check-files.py
Outdated
self.excluded_paths = list(map(os.path.normpath, [ | ||
'cov-int', | ||
'examples', | ||
'yotta/module' |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The PR to remove yotta support will be merged before this one. I don't think you need that line.
Complements "Remove Yotta support from the docs, tests and build scripts".
96f3b4e
@sbutcher-arm Thanks for reviewing. I removed the yotta line from |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
retest |
1 similar comment
retest |
Jenkins failed on the BSD timing test, which is a known issue and as good as a pass. Therefore marking as 'ready for merge'. |
Fix some issues that don't break
all.sh
on a pristine checkout, but make it fragile in a tree where you've been doing development.make WINDOWS_BUILD=1 clean
on non-Windows hosts.check-files.py
complaining about files in.git
or in third-party subtrees.make apidoc
looking for files in unintended subdirectories.Backports: #2032 for 2.7, #2033 for 2.1.
Precursor to PSA.