Skip to content
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.

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

LABEL error is unclear when quotes are misused #2656

Closed
alexcb opened this issue Feb 7, 2023 · 6 comments
Closed

LABEL error is unclear when quotes are misused #2656

alexcb opened this issue Feb 7, 2023 · 6 comments
Labels
type:bug Something isn't working

Comments

@alexcb
Copy link
Collaborator

alexcb commented Feb 7, 2023

VERSION 0.7

test:
  FROM alpine
  RUN echo https://github.com/earthly/earthly > url
  LABEL "org.opencontainers.image.source=$(cat url)"

fails:

Error: build target: build main: failed to solve: resolve build context for target +test: Earthfile line 6:52 '
': invalid syntax
@alexcb alexcb added the type:bug Something isn't working label Feb 7, 2023
@alexcb
Copy link
Collaborator Author

alexcb commented Feb 7, 2023

https://github.com/earthly/earthly/blob/main/earthfile2llb/interpreter.go#L1423-L1444 makes calls to expand args, but it's not clear how "org.opencontainers.image.source=$(cat url)" ends up being split into "org.opencontainers.image.source", "=", $(cat url)"

@alexcb
Copy link
Collaborator Author

alexcb commented Feb 7, 2023

Note that this does work

VERSION 0.7

test:
  FROM alpine
  RUN echo https://github.com/earthly/earthly > url
  LABEL org.opencontainers.image.source="$(cat url)"   # note the quotes are located after the `=`.

@alexcb alexcb closed this as completed Feb 7, 2023
@alexcb
Copy link
Collaborator Author

alexcb commented Feb 7, 2023

this too works:

VERSION 0.7

test:
  FROM alpine
  RUN echo https://github.com/earthly/earthly > url
  LABEL org.opencontainers.image.source=$(cat url)

@alexcb alexcb reopened this Feb 7, 2023
@alexcb alexcb changed the title LABEL doesnt support shelling out LABEL struggles with quotes Feb 7, 2023
@alexcb
Copy link
Collaborator Author

alexcb commented Feb 7, 2023

stretch goal: currently LABEL only appears under:

$ head tests/smoke.earth
VERSION 0.7
# This is a smoke test for parsing. We don't actually check that the config was
# set correctly.
FROM alpine:3.15
test:
    LABEL a=b c=d "e    eee = ee"=fff
    LABEL x=y
    LABEL abc.def.ghi=jkl

we should create an integration test that validates the label is correctly produced in the built image.

@nelsam
Copy link
Contributor

nelsam commented Feb 7, 2023

For clarification's sake: LABEL "org.opencontainers.image.source"="$(cat url)" also works.

LABEL "somelabel=somevalue" should not be valid syntax, correct? The issue mentions "org.opencontainers.image.source=$(cat url)", but that is one value - we should not split on the equals sign there. Since the = is inside of the quote pair, that equates to LABEL labelKey, and our parser grammar requires a label statement to be LABEL (labelKey EQUALS labelValue)* - so the = cannot be inside of the quote pair to be valid syntax.

The error message is obtuse (and this is not the only case where an error message is obtuse), so I would like to improve the error message - but I want to make sure that the problem is specifically that the error message is not clear enough to track down why the LABEL failed.

@alexcb
Copy link
Collaborator Author

alexcb commented Feb 7, 2023

LABEL "somelabel=somevalue" should not be valid syntax, correct?

That's a really good point.

where as LABEL "$KEY"="$VALUE" should be valid (as you point out, it already works).

It seems like the only issue is the error is not obvious.

@alexcb alexcb changed the title LABEL struggles with quotes LABEL error is unclear when quotes are misused Feb 7, 2023
nelsam added a commit that referenced this issue Feb 9, 2023
Our parser/lexer syntax errors are extremely hard to comprehend and have
been causing confusion. This updates our ErrorListener and ErrorStrategy
with a few niceties, attempting to add extra context about the incorrect
syntax.

These cases are pretty minimal, but they're a start.

---

For issue #2656 in particular, here's the updated error message:

```
Error: build target: build main: failed to solve: resolve build context for target +foo: Earthfile line 5:45 '
': invalid syntax

Hints:
  - I got lost looking for '=' - did you define a key/value pair without a value?
```
@nelsam nelsam closed this as completed Feb 9, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type:bug Something isn't working
Projects
None yet
Development

No branches or pull requests

2 participants