Skip to content

Fixed bad quoting of variable expansions #2038

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.

Sign up for GitHub

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

Merged
larsewi merged 1 commit into from
Nov 14, 2025
Merged

Fixed bad quoting of variable expansions #2038

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
15 changes: 12 additions & 3 deletions build-scripts/test-on-testmachine
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -95,19 +95,28 @@ INCLUDES='--include=test.* --include=summary.*'
# Note: Don't use sudo or "rsync -a", because we don't want root-owned files
# to show up in the results.
log_debug "Collecting test results from core repo..."
rsync -rv "$INCLUDES" --exclude="*" \
# shellcheck disable=SC2086
# > Double quote to prevent globbing and word splitting.
# We want word splitting
rsync -rv $INCLUDES --exclude="*" \

@vpodzime vpodzime Nov 14, 2025

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please double-check that the * is not expanded locally without quotes.

@larsewi larsewi Nov 14, 2025

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

@larsewi larsewi Nov 14, 2025

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do you mean the * from here INCLUDES='--include=test.* --include=summary.*' ?

@larsewi larsewi Nov 14, 2025

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

"$TESTMACHINE_URI$BASEDIR"/core/tests/acceptance/ \
"$BASEDIR"/core/tests/acceptance/ \
>>/tmp/rsync.log

if [ "$PROJECT" = nova ]; then
log_debug "Collecting test results from enterprise repo..."
rsync -rv "$INCLUDES" --exclude="*" \
# shellcheck disable=SC2086
# > Double quote to prevent globbing and word splitting.
# We want word splitting
rsync -rv $INCLUDES --exclude="*" \
"$TESTMACHINE_URI$BASEDIR"/enterprise/tests/acceptance/ \
"$BASEDIR"/enterprise/tests/acceptance/ \
>>/tmp/rsync.log
log_debug "Collecting test results from masterfiles repo..."
rsync -rv "$INCLUDES" --exclude="*" \
# shellcheck disable=SC2086
# > Double quote to prevent globbing and word splitting.
# We want word splitting
rsync -rv $INCLUDES --exclude="*" \
"$TESTMACHINE_URI$BASEDIR"/masterfiles/tests/acceptance/ \
"$BASEDIR"/masterfiles/tests/acceptance/ \
>>/tmp/rsync.log
Expand Down