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

bake: avoid nesting error diagnostics #1607

Merged
merged 1 commit into from
Feb 7, 2023

Conversation

jedevc
Copy link
Collaborator

@jedevc jedevc commented Feb 7, 2023

With changes to the lazy evaluation, the evaluation order is no longer fixed - this means that we can follow long and confusing paths to get to an error.

Because of the co-recursive nature of the lazy evaluation, we need to take special care that the original HCL diagnostics are not discarded and are preserved so that the original source of the error can be detected. Preserving the full trace is not necessary, and probably not useful to the user - all of the file that is not lazily loaded will be eagerly loaded after all struct blocks are loaded - so the error would be found regardless.

For example, in the following docker-bake.hcl:

x = x
target "test" {
  dockerfile = x
}

We expect an error view like:

docker-bake.hcl:1
--------------------
   1 | >>> x = x
   2 |     
   3 |     target "test" {
--------------------
ERROR: docker-bake.hcl:1,5-6: Invalid expression; variable cycle not allowed for x

But in v0.10 we get:

docker-bake.hcl:4
--------------------
   2 |     
   3 |     target "test" {
   4 | >>>   dockerfile = x
   5 |     }
   6 |     
--------------------
ERROR: docker-bake.hcl:4,16-17: Invalid expression; docker-bake.hcl:1,5-6: Invalid expression; variable cycle not allowed for x

This occurs because we double wrap the error with two layers of hcl.Diagnostics, which we can avoid by only wrapping if the target error is not already an hcl.Diagnostics error type.

With changes to the lazy evaluation, the evaluation order is no longer
fixed - this means that we can follow long and confusing paths to get to
an error.

Because of the co-recursive nature of the lazy evaluation, we need to
take special care that the original HCL diagnostics are not discarded
and are preserved so that the original source of the error can be
detected. Preserving the full trace is not necessary, and probably not
useful to the user - all of the file that is not lazily loaded will be
eagerly loaded after all struct blocks are loaded - so the error would
be found regardless.

Signed-off-by: Justin Chadwell <[email protected]>
@jedevc jedevc merged commit ab7a9f0 into docker:master Feb 7, 2023
@jedevc jedevc added this to the v0.10.3 milestone Feb 13, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants