Older Docker accepted inline comments in some places in spite of documentation otherwise, leading to broken builds now that it's fixed, and opaque error messaging interferes with troubleshooting and repair.

[jhardin-accumula]

See moby/moby#48811 (comment)

Older versions of Docker accepted inline comments (at least in some places) in the Dockerfile and behaved as reasonably expected if that was done: the comment was ignored, rather than affecting the command on that line.

This has apparently been fixed to follow the documented behavior, leading to breakage of previously-working (though technically invalid) Dockerfiles. I haven't researched where/when the change occurred, but it bit me last week so it's recent.

Unfortunately the error message is not at all helpful in determining the root cause of the failure:

[internal] load build context
rpc error: code = Unknown desc = invalid includepatterns: []: syntax error in pattern

In this case, the now-fatal build command was:

COPY ["./Library", "."] # the entire Shared UtilityLibrary project folder

Can the error message for this sort of failure be improved somehow to aid troubleshooting? The current message is very opaque. For example, reporting the command line that failed to process as part of the error message would have greatly sped resolution of this problem.

Also, per suggestion by @thaJeztah, can inline comments (at least where they were working before) generate a warning message instead of an error to aid in troubleshooting and cleanup of Dockerfiles during transition to tighter enforcement of the documented behavior?

Thanks!

[thaJeztah]

Thanks!

cc @tonistiigi @crazy-max

[tonistiigi]

This is an update from #5107 .

There was never any special handling for comments together with JSON value, but there was a bug where any random bytes after the first JSON object were ignored.

The change is in parser pkg shared by BuildKit and legacy builder so even if not using buildkit, the behavior is now consistent.

[jhardin-accumula]

What is the new, consistent behavior?

[tonistiigi]

What is the new, consistent behavior?

COPY ["json", "here"] # text is always error. It is handled as non-JSON.

[jhardin-accumula]

Okay.

Would it be possible to get the failed raw command line included in the error message when that error is emitted, so that it can be corrected more easily? Please? It was not at all obvious from the error message what or where the actual root cause was.

[jhardin-accumula]

Can you post an example of what the error message would look like with that change? Thanks!

[tonistiigi]

invalid includepatterns: [# [Dockerfile, d2] some]: syntax error in pattern

The command is split by spaces. Last part is destination path, all else are source paths that become patterns for context upload step and now show up in the error message if they are invalid patterns.

[thaJeztah]

What was the input for that error? something like this?

COPY ["Dockerfile", "d2", "."] # some comment

I wonder why the error is about "syntax", and not "these (oddly named) file(s) could not be found"; comparing the error between buildkit and classic builder;

BuildKit;

echo -e 'FROM scratch\nCOPY ["foo", "bar", "."' | docker build -
...
------
 > [internal] load build context:
------
ERROR: failed to solve: rpc error: code = Unknown desc = invalid includepatterns: []: syntax error in pattern

Compared to;

echo -e 'FROM scratch\nCOPY ["foo", "bar", "."' | DOCKER_BUILDKIT=0 docker build -
...
Step 2/2 : COPY ["foo", "bar", "."
COPY failed: file not found in build context or excluded by .dockerignore: stat bar,: file does not exist

I wonder if we have ways to surface more what happened; i.e., “we fell back to handling this as non-JSON”. or even “WARNING: did you mean xxx here?” because as a human, this LOOKS perfectly valid;

COPY ["./foo", "."] # copy the foo dir

Overall, I think we can be pretty confident that (specifically with COPY, ADD, VOLUME) if the line starts with [ that the intent was “should be JSON”. RUN is the only case where I think it’s less confident ([ being used as part of the command), so I wonder if we could add warnings for those.

I guess that's more related to;

• Enhancement: print warnings when falling back from JSON to "shell" syntax (and other warnings) #1952