[keyserver] Revise `dc.sh` passed parameters
ClosedPublic
Actions

Authored by • abosh on Jun 27 2022, 12:07 PM.

Details

Reviewers

atul
ashoat
tomek

Commits

rCOMM74bbe7e0491d: [keyserver] Revise `dc.sh` passed parameters

Summary

Relevant Linear issue with full context here.

This diff cleans up dc.sh by revising the passed parameters to docker compose. They should be double quoted to properly expand the argument list. Additionally, the @ parameter can be used singularly without curly braces (see inline comment).

This script was initially introduced a few weeks ago in D4213, adding those reviewers here.

Test Plan

ShellCheck, close reading.

Diff Detail

Repository

rCOMM Comm

Lint

Lint Not Applicable

Unit

Tests Not Applicable

Event Timeline

• abosh created this revision.Jun 27 2022, 12:07 PM

Herald added subscribers: • adrian, tomek. · View Herald TranscriptJun 27 2022, 12:07 PM

• abosh edited the summary of this revision. (Show Details)Jun 27 2022, 12:08 PM

• abosh edited the summary of this revision. (Show Details)

• abosh added a reviewer: tomek.

• abosh removed 2 blocking reviewer(s): tomek, atul.Jun 27 2022, 12:10 PM

Harbormaster completed remote builds in B10017: Diff 13852.Jun 27 2022, 12:13 PM

• abosh requested review of this revision.Jun 27 2022, 12:13 PM

• abosh added inline comments.Jun 27 2022, 12:14 PM

keyserver/bash/dc.sh
8 ↗	(On Diff #13852)	The double quotes were added to avoid re-splitting elements. The curly braces and the 1 were removed because `"$@"` has the same behavior as `${@:1}`. Source: Bash parameters and parameter expansions from IBM Developer Tutorials. Read more on this StackOverflow answer, from the same StackOverflow post @ashoat shared in D4213 here.

• abosh added inline comments.Jun 27 2022, 12:17 PM

keyserver/bash/dc.sh
8 ↗	(On Diff #13852)	Edit: The curly braces and the 1 were removed because `"$@"` has the same behavior as `"${@:1}"` (forgot the double quotes around `${@:1}`)

If the file in the inline comment cannot be seen.

Back to you with a small question – does this still work with something like bash/dc.sh up --build? Wondering if the double quotes will mess that up

This revision now requires changes to proceed.Jun 28 2022, 9:33 AM

No, it shouldn't. I'm not sure why double quotes would mess it up, was there a reason you thought it could?

To be sure, I quickly wrote this script to compare outputs and parsing: