DO NOT MERGE: Shellcheck script files in initrd #872
Closed
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
@Thrilleratplay what is your preferate approach between #862 and here? Seems lik here wins to me. |
|
@tlaurion I would probably vote for this version, fake BASH. It will likely be easier to verify the changes and run spellcheck against. Again, this could only be a temporary solution with an long term goal of rewriting everything in POSIX. |
Thrilleratplay
force-pushed
the
shellcheck_bash
branch
from
cbbf253 to
6357d41
Compare
51 tasks
|
@Thrilleratplay : can you make circleCi follow your github so that automatic builds are made at each commit? This PR seems mature enough to test builds for regressions |
|
@tlaurion I will try. Currently I am still testing and running into issues. |
…ictyly excluding rules in make command
Thrilleratplay
changed the title
This was referenced Nov 2, 2020
|
This is not going to work. #862 approach is preferred. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This is an alternative approach to #862.
Instead of rewriting all BASH syntax to be POSIX, add busybox config to set
CONFIG_BASH_IS_ASH=ywhich I believe statically links/bin/ashto/bin/bash. This combined with the existingCONFIG_ASH_BASH_COMPATsetting should allow shellcheck to the check the scripts under/initrdas closer to how they already exists and with few large changes.While labeling these scripts as
#!/bin/bashmay be misleading, all scripts are read-only within the ROM and will still need to be tested before being committed in the official repo. The only problem I can foresee would be if users who try to execute BASH scripts from external media that may execute bash scripts with incompatible syntax, although, I think this is currently a possibility.Exceptions added for:
.shellcheckrcinitrd/init.shinitrd/sbin/config-dhcp.sh@osresearch @tlaurion @MrChromebox One of three possible paths: