Fixed bad quoting of variable expansions - #2038
Conversation
You would normally use double quotes when expanding variables to prevent word splitting. However, these variables contain options to `rsync`, hence we want word splitting to occur. This should fix the issue where no test results are reported to the Jenkins CI. Signed-off-by: Lars Erik Wik <lars.erik.wik@northern.tech>
|
@cf-bottom Jenkins please :) |
|
Sure, I triggered a build: Jenkins: https://ci.cfengine.com/job/pr-pipeline/13014/ Packages: http://buildcache.cfengine.com/packages/testing-pr/jenkins-pr-pipeline-13014/ |
vpodzime
left a comment
vpodzime
left a comment
There was a problem hiding this comment.
Looks good to me otherwise.
| # shellcheck disable=SC2086 | ||
| # > Double quote to prevent globbing and word splitting. | ||
| # We want word splitting | ||
| rsync -rv $INCLUDES --exclude="*" \ |
There was a problem hiding this comment.
Please double-check that the * is not expanded locally without quotes.
There was a problem hiding this comment.
Why? The quotes are there to prevent globbing. And they should be there? At least they have been there for over 10 years without any trouble.
There was a problem hiding this comment.
Do you mean the * from here INCLUDES='--include=test.* --include=summary.*' ?
There was a problem hiding this comment.
Either way, I think it will be fine. I recently added double quotes on the INCLUDES variable expansion to fix shellcheck warnings. And the quotes should not really have been there. Now, I'm just putting things back to the way they where and disabling the shellcheck warnings from those lines.
4 participants
You would normally use double quotes when expanding variables to prevent
word splitting. However, these variables contain options to
rsync,hence we want word splitting to occur.
This should fix the issue where no test results are reported to the
Jenkins CI.
Signed-off-by: Lars Erik Wik lars.erik.wik@northern.tech