Allow POSIX variable substitution to remove optional colon

[jedevc]

🛠️ Partial implementation of #2389 (comment)

POSIX allows the colon in variable expansion to be optional, see https://pubs.opengroup.org/onlinepubs/9699919799/utilities/V3_chap02.html#tag_18_05

This PR updates buildkit's variable expansion to include this, so all the following are now supported:

• ${FOO-bar}, ${FOO:-bar}
• ${FOO+bar}, ${FOO:+bar}
• ${FOO?bar}, ${FOO:?bar}

Additionally, I've updated the tests to use matrix-style tests, essentially filling out the table from "2.6.2 Parameter Expansion", to track for regressions.

As a next-step, I'm looking at implementing support for the rest of #2389, with ${parameter%[word]} and ${parameter#[word]} as well as hopefully ${parameter//[word]//[new_word]}.

[crazy-max]

LGTM, nice enhancement. PTAL @tonistiigi

[jedevc]

If interested in comparing the results with the previous implementation:

FROM scratch
ENV foo=foo
ENV bar=

COPY <<EOF /test
${foo}
${bar}
${baz}
---
${foo:-FOO}
${bar:-BAR}
${baz:-BAZ}
---
${foo:+FOO}
${bar:+BAR}
${baz:+BAZ}
---
EOF

Both master and this patch produce the same results into /test:

foo


---
foo
BAR
BAZ
---
FOO


---

[tonistiigi]

This SGTM but rather than getting more compatible with posix I'm more interested in support for the specific new patterns #1772 (comment) . @thaJeztah @tianon Do you want to get this in?

[thaJeztah]

Overall SGTM to support the syntax without : (I must admit that I never use that syntax, and always forget it's also supported). Do we need changes in the classic builder to keep the format compatible?

I was curious what we did before with PR with - (without a :), as I wasn't sure if we previously considered ${BUILD-ARG} to be a build-arg named BUILD-ARG (with a hyphen in the name), but looks like we're good there; previously it was an error, so nobody should be able to depend on that.

docker build -t foo -<<'EOF'
# syntax=docker/dockerfile:1
FROM alpine
ARG BUILD-ARG=hello world
LABEL mylabel=${BUILD-ARG}
EOF

With BuildKit:

Dockerfile:4
--------------------
   2 |     FROM alpine
   3 |     ARG BUILD-ARG=hello world
   4 | >>> LABEL mylabel=${BUILD-ARG}
   5 |
--------------------
ERROR: failed to solve: failed to process "${BUILD-ARG}": missing ':' in substitution

Classic builder:

Step 3/3 : LABEL mylabel=${BUILD-ARG}
failed to process "${BUILD-ARG}": missing ':' in substitution

When used without ${}; (both with BuildKit and the classic builder);

docker build -t foo -<<'EOF'
# syntax=docker/dockerfile:1
FROM alpine
ARG BUILD-ARG=hello world
LABEL mylabel=$BUILD-ARG
EOF

docker image inspect foo --format='{{json .Config.Labels}}' | jq .
{
  "mylabel": "-ARG"
}

Which looks to match behavior in the shell;

echo "${BUILD-ARG}"
ARG
echo "$BUILD-ARG"
-ARG

[tianon]

Sorry for missing this ping sooner!!! 🙇 ❤️

I looked over the test cases, and they all looked correct to me, and pretty easy to verify in shell too: 👀

$ FOO=xxx BAR= sh -c 'echo "${FOO-aaa} ${BAR-bbb} ${BAZ-ccc}"'
xxx  ccc
$ FOO=xxx BAR= sh -c 'echo "${FOO:-aaa} ${BAR:-bbb} ${BAZ:-ccc}"'
xxx bbb ccc
$ FOO=xxx BAR= sh -c 'echo "--${FOO+aaa} ${BAR+bbb} ${BAZ+ccc}--"'
--aaa bbb --
$ FOO=xxx BAR= sh -c 'echo "--${FOO:+aaa} ${BAR:+bbb} ${BAZ:+ccc}--"'
--aaa  --

(I don't personally use these often, but the distinction is useful sometimes, so I'm in favor of supporting them 👍)