Add config check automation tool for wolfSSL configure#5876
Add config check automation tool for wolfSSL configure#5876gojimmypi wants to merge 1 commit intowolfSSL:masterfrom
Conversation
|
@jpbland1 is going to review and give this a try. |
|
retest this please. |
config_check/README.md
Outdated
There was a problem hiding this comment.
should be Disabled-Features.txt
There was a problem hiding this comment.
good catch. updated.
config_check/refresh.sh
Outdated
There was a problem hiding this comment.
Is error checking important here? when I comment out ./configure in cmd.txt it doesn't work but it can't really do any damage and just leads to my terminal telling me command not found followed by all the text below
There was a problem hiding this comment.
I added some error checking and comments in the cmd.txt file. There's not much difference other than not doing the file operations after the ./configure command.
jpbland1
left a comment
There was a problem hiding this comment.
made some comments, tested it and it works on my setup
|
Heads up I added the include.am to Makefile.am. I also don't know why I was previously moving the the |
jpbland1
left a comment
There was a problem hiding this comment.
looks good, refresh is executable and an error message prints out when I remove configure from cmd.txt
|
jenkins retest this please |
|
github says this branch is behind master, maybe merge the new changes and see if that fixes the build problem |
dgarske
left a comment
There was a problem hiding this comment.
Please move this into scripts/config_check.
On Mac this doesn't work:
% ./refresh.sh
This WOLFSSL_REPO = /Users/davidgarske/GitHub/wolfssl
WOLFSSL_FILE_ROOT = /Users/davidgarske/GitHub/wolfssl/config_check
CMD File= /Users/davidgarske/GitHub/wolfssl/config_check/cmd.txt
tee: illegal option -- -
usage: tee [-ai] [file ...]
tee: illegal option -- -
usage: tee [-ai] [file ...]
tee: illegal option -- -
usage: tee [-ai] [file ...]
Error running command /Users/davidgarske/GitHub/wolfssl/config_check/cmd.txt
Mac uses -a for append -a Append the output to the files rather than overwriting them.. usage: tee [-ai] [file ...]
|
Replaced Renamed the refresh script to Now that I re-read that, @dgarske perhaps you meant literally the |
dgarske
left a comment
There was a problem hiding this comment.
Hey Jim,
Please move these files (named as they were) into a new directory scripts/config_check/.
The fix for tee -a works on my Mac.
Thanks
|
@dgarske - done; moved into scripts/config_check on this PR ConfigCheck branch. Should I try to squash commits during or after the PR review process? There are some rather petty ones cluttering up the history. Let me know the best practice for this. |
scripts/config_check/README.md
Outdated
There was a problem hiding this comment.
should be This is the not This the
There was a problem hiding this comment.
ah yes, I thought I fixed this, but there was another. good catch. thank you!
9337261 to
9b513fd
Compare
|
updated readme, resync with upstream and squashed commits, reopened for review. |
|
Initial thoughts on testing this out and reading the output:
|
kaleb-himes
left a comment
There was a problem hiding this comment.
Comment posted above.
|
@kaleb-himes I created this script mainly to have the ability to have a fully commented list of parameters to feed to the Note that in my case, I was using it for an embedded project. So I was manually copying some generated settings from the Good point on the
Minimal to none of there's no specific comments in the example. I've removed some of the specifics.
convenience of a
I don't think there's much technical debt here, particularly if the example |
|
I appreciate the review and attention to detail. I've made some changes as suggested. |
|
Can one of the admins verify this patch? |
6cb31d7 to
6f76592
Compare
|
I've refreshed from upstream, squashed & re-added files; also fixed the line endings on the prior Let me know what needs to be done to get this one merged. Another issue came up regarding an embedded application needing particular settings in |
|
Jenkins retest this please |
|
There are probably better tools available. I'm closing this lingering draft PR as I likely won't submit it in the near future. |
Description
The wolfSSL
./configurecommand has an extraordinary number of options and parameters that select target platforms, fine tune installation, optimize performance, select various features, and more. Managing and experimenting with the options can be a daunting experience, particularly for the new user.This tool allows options to be specified in a more readable command file with comments. The output of the refresh script breaks out the resultant enabled and disabled features into more easily viewed summary files.
See the README.md for full details.
Fixes zd# n/a
Testing
How did you test?
There's no change to the underlying wolfSSL library. This is only a configuration assistance tool.
Run the from the
config_checkdirectory:cd ./scripts/config_check ./refresh.shObserve files created in the
config_checkdirectory.Checklist