You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@apisix.apache.org by GitBox <gi...@apache.org> on 2022/02/03 10:15:11 UTC
[GitHub] [apisix] kwanhur opened a new pull request #6246: ci: runner shell sudo doesn't affect redirects SC2024
kwanhur opened a new pull request #6246:
URL: https://github.com/apache/apisix/pull/6246
### What this PR does / why we need it:
<!--- Why is this change required? What problem does it solve? -->
fix ci shell scripts warning `sudo` doesn't affect redirects [SC2024](https://github.com/koalaman/shellcheck/wiki/SC2024)
- use `tee` copies standard input to standard output, instead of `>` to write
<!--- If it fixes an open issue, please link to the issue here. -->
### Pre-submission checklist:
<!--
Please follow the PR manners:
1. Use Draft if the PR is not ready to be reviewed
2. Test is required for the feat/fix PR, unless you have a good reason
3. Doc is required for the feat PR
4. Use a new commit to resolve review instead of `push -f`
5. If you need to resolve merge conflicts after the PR is reviewed, please merge master but do not rebase
6. Use "request review" to notify the reviewer once you have resolved the review
7. Only reviewer can click "Resolve conversation" to mark the reviewer's review resolved
-->
* [x] Did you explain what problem does this PR solve? Or what new features have been added?
* [ ] Have you added corresponding test cases?
* [ ] Have you modified the corresponding document?
* [ ] Is this PR backward compatible? **If it is not backward compatible, please discuss on the [mailing list](https://github.com/apache/apisix/tree/master#community) first**
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@apisix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix] spacewander commented on pull request #6246: ci: runner shell sudo doesn't affect redirects SC2024
Posted by GitBox <gi...@apache.org>.
spacewander commented on pull request #6246:
URL: https://github.com/apache/apisix/pull/6246#issuecomment-1032171354
The SC2024 requires `sudo tee` because `sudo` doesn't affect `>`.
See the example, `echo 3 | sudo tee /proc/sys/vm/drop_caches > /dev/null`.
However, as I mentioned several days ago, we don't need `sudo` to write to `build.log`. There is no need to introduce an extra cmd to do what `>` can do.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@apisix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix] kwanhur commented on a change in pull request #6246: ci: runner shell sudo doesn't affect redirects SC2024
Posted by GitBox <gi...@apache.org>.
kwanhur commented on a change in pull request #6246:
URL: https://github.com/apache/apisix/pull/6246#discussion_r800192300
##########
File path: ci/linux_apisix_current_luarocks_runner.sh
##########
@@ -33,8 +33,8 @@ script() {
sudo rm -rf /usr/local/share/lua/5.1/apisix
# install APISIX with local version
- sudo luarocks install rockspec/apisix-master-0.rockspec --only-deps > build.log 2>&1 || (cat build.log && exit 1)
- sudo luarocks make rockspec/apisix-master-0.rockspec > build.log 2>&1 || (cat build.log && exit 1)
+ sudo luarocks install rockspec/apisix-master-0.rockspec --only-deps | sudo tee build.log > /dev/null 2>&1 || (cat build.log && exit 1)
Review comment:
The original redirects stdout into `build.log`, it isn't recommended for using `sudo`.
If it needs log into `build.log`, suggest using `tee` to make it.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@apisix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix] spacewander closed pull request #6246: ci: runner shell sudo doesn't affect redirects SC2024
Posted by GitBox <gi...@apache.org>.
spacewander closed pull request #6246:
URL: https://github.com/apache/apisix/pull/6246
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@apisix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix] spacewander commented on a change in pull request #6246: ci: runner shell sudo doesn't affect redirects SC2024
Posted by GitBox <gi...@apache.org>.
spacewander commented on a change in pull request #6246:
URL: https://github.com/apache/apisix/pull/6246#discussion_r800342456
##########
File path: ci/linux_apisix_current_luarocks_runner.sh
##########
@@ -33,8 +33,8 @@ script() {
sudo rm -rf /usr/local/share/lua/5.1/apisix
# install APISIX with local version
- sudo luarocks install rockspec/apisix-master-0.rockspec --only-deps > build.log 2>&1 || (cat build.log && exit 1)
- sudo luarocks make rockspec/apisix-master-0.rockspec > build.log 2>&1 || (cat build.log && exit 1)
+ sudo luarocks install rockspec/apisix-master-0.rockspec --only-deps | sudo tee build.log > /dev/null 2>&1 || (cat build.log && exit 1)
Review comment:
No, use no-sudo user is enough to write log to `build.log`.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@apisix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix] spacewander commented on a change in pull request #6246: ci: runner shell sudo doesn't affect redirects SC2024
Posted by GitBox <gi...@apache.org>.
spacewander commented on a change in pull request #6246:
URL: https://github.com/apache/apisix/pull/6246#discussion_r801220032
##########
File path: ci/linux_apisix_master_luarocks_runner.sh
##########
@@ -37,7 +37,7 @@ script() {
cp -r ../utils ./
# install APISIX by luarocks
- sudo luarocks install $APISIX_MAIN > build.log 2>&1 || (cat build.log && exit 1)
+ sudo luarocks install $APISIX_MAIN | tee build.log > /dev/null 2>&1 || (cat build.log && exit 1)
Review comment:
There is no need to use `tee` to redirect. The original way is good enough.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@apisix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix] spacewander commented on a change in pull request #6246: ci: runner shell sudo doesn't affect redirects SC2024
Posted by GitBox <gi...@apache.org>.
spacewander commented on a change in pull request #6246:
URL: https://github.com/apache/apisix/pull/6246#discussion_r800171007
##########
File path: ci/linux_apisix_current_luarocks_runner.sh
##########
@@ -33,8 +33,8 @@ script() {
sudo rm -rf /usr/local/share/lua/5.1/apisix
# install APISIX with local version
- sudo luarocks install rockspec/apisix-master-0.rockspec --only-deps > build.log 2>&1 || (cat build.log && exit 1)
- sudo luarocks make rockspec/apisix-master-0.rockspec > build.log 2>&1 || (cat build.log && exit 1)
+ sudo luarocks install rockspec/apisix-master-0.rockspec --only-deps | sudo tee build.log > /dev/null 2>&1 || (cat build.log && exit 1)
Review comment:
We don't need sudo to write to `build.log`? So the original code's OK.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@apisix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org