-
Notifications
You must be signed in to change notification settings - Fork 830
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
Add config check automation tool for wolfSSL configure #5876
Conversation
@jpbland1 is going to review and give this a try. |
retest this please. |
config_check/README.md
Outdated
The multi-line `./configure` command with all parameters on subsequent lines is stripped of comments and everything placed on a single line statement when executed. | ||
|
||
Upon execution, the entire output is set to a file called [output.txt](./output.txt). Additionally, the enabled/disabled features (those items with an asterisk in the output and the word "yes" or "no") | ||
are separated and stored in the respective [Enabled-Features.txt](Enabled-Features.txt) and [Enabled-Features.txt](Enabled-Features.txt) files. |
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.
should be Disabled-Features.txt
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.
good catch. updated.
config_check/refresh.sh
Outdated
echo "" | ||
|
||
# get the contents of the command file, trimming all text after the # character | ||
WOLFSSL_CMD="$(cat $WOLFSSL_CMD_FILE | cut -d'#' -f1)" |
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.
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
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.
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 |
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.
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 |
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.
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 |
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.
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
|
||
#### `WOLFSSL_CMD_FILE` | ||
|
||
This the `.configure` command to edit. This is typically the location of the [cmd.txt](./cmd.txt) file |
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.
should be This is the
not This the
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.
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:
|
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.
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
./configure
command 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_check
directory:cd ./scripts/config_check ./refresh.sh
Observe files created in the
config_check
directory.Checklist