Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

esptools/install.sh: Fix shellcheck issues #19325

Merged
merged 1 commit into from
Feb 27, 2023

Conversation

bergzand
Copy link
Member

Contribution description

Quote all the things!

Testing procedure

The script should still work as before

Issues/PRs references

None

@bergzand bergzand added Type: cleanup The issue proposes a clean-up / The PR cleans-up parts of the codebase / documentation Area: tools Area: Supplementary tools Platform: ESP Platform: This PR/issue effects ESP-based platforms labels Feb 27, 2023
@github-actions github-actions bot removed the Platform: ESP Platform: This PR/issue effects ESP-based platforms label Feb 27, 2023
@bergzand bergzand added the CI: skip compile test If set, CI server will run only non-compile jobs, but no compile jobs or their dependent jobs label Feb 27, 2023
@gschorcht gschorcht removed their request for review February 27, 2023 13:15
@gschorcht
Copy link
Contributor

The same is probably needed for dist/tools/esptools/export.sh.

@gschorcht
Copy link
Contributor

As long as the variables do not contain spaces, it should not really be necessary in any shell to enclose them in quotes. But indeed, for path names it is better to quote them.

@bergzand
Copy link
Member Author

As long as the variables do not contain spaces, it should not really be necessary in any shell to enclose them in quotes. But indeed, for path names it is better to quote them.

Yup, path names and argument should always be quoted as they could in theory contain whitespace. Variables, in this case ARCH_ALL rely on the whitespace and are not quoted.

@bergzand
Copy link
Member Author

The same is probably needed for dist/tools/esptools/export.sh.

To be honest this is a quick drive-by PR from me, so I'd rather handle that one in a follow up if you don't mind.

@gschorcht
Copy link
Contributor

Thanks for providing that fix.

@benpicco benpicco added the CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR label Feb 27, 2023
@riot-ci
Copy link

riot-ci commented Feb 27, 2023

Murdock results

✔️ PASSED

ea1ab1a esptools/install.sh: Fix shellcheck issues

Success Failures Total Runtime
1 0 1 57s

Artifacts

@benpicco
Copy link
Contributor

bors merge

bors bot added a commit that referenced this pull request Feb 27, 2023
19321: examples/gnrc_border_router: add BLE as downlink option r=benpicco a=benpicco



19325: esptools/install.sh: Fix shellcheck issues r=benpicco a=bergzand

### Contribution description

Quote all the things!


### Testing procedure

The script should still work as before


### Issues/PRs references

None

19327: shell/cmds: GNRC: replace puts() with printf() r=benpicco a=benpicco



19328: pkg/u8g2: bump version r=benpicco a=benpicco



Co-authored-by: Benjamin Valentin <benjamin.valentin@ml-pa.com>
Co-authored-by: Koen Zandberg <koen@bergzand.net>
Co-authored-by: Benjamin Valentin <benpicco@beuth-hochschule.de>
@bors
Copy link
Contributor

bors bot commented Feb 27, 2023

Build failed (retrying...):

@benpicco
Copy link
Contributor

bors cancel
bors merge

@bors
Copy link
Contributor

bors bot commented Feb 27, 2023

Canceled.

@bors bors bot merged commit 6b501f7 into RIOT-OS:master Feb 27, 2023
@bors
Copy link
Contributor

bors bot commented Feb 27, 2023

Build succeeded:

@MrKevinWeiss MrKevinWeiss added this to the Release 2023.04 milestone Apr 25, 2023
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
Area: tools Area: Supplementary tools CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR CI: skip compile test If set, CI server will run only non-compile jobs, but no compile jobs or their dependent jobs Type: cleanup The issue proposes a clean-up / The PR cleans-up parts of the codebase / documentation
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants