bake: set attribute even if diagnosed as duplicated by crazy-max · Pull Request #1062 · docker/buildx (original) (raw)

Conversation

This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.Learn more about bidirectional Unicode characters

[ Show hidden characters]({{ revealButtonHref }})

@crazy-max

Carry changes from #1025 related to an issue with hcl attributes not set if diagnosed as duplicated: #1025 (comment).

Needs to fork the merge logic from hcl repo. It might be better if hcl exposed its mergedbodies interface. Don't think we should fork the repo and vendor it here with this small change though. Maybe there is a better way?

tonistiigi

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Did we try to upstream this?

Could we avoid copying this bunch of code by just providing our own JustAttributes() function that ranges over array and calls internal JustAttributes() for each item.

Add more context to this issue about the actual bake case it addresses and bake tests.

@jedevc

@crazy-max do you have a test case here? (sorry, I know this is an older issue)

I think there might be an easier way to resolve this using some of the bake refactors recently, but I can't seem to reproduce the issue being discussed - the test from #1025 (comment) fails for different reasons.

@crazy-max

do you have a test case here? (sorry, I know this is an older issue)

This is just for consistency with #1025 so we can merge global attributes like:

c1.hcl

FOO = "abc" target "app" { args = { v1 = "pre-${FOO}" } }

Which currently gives v1 = pre-abc. I'm looking at upstreaming my change.

@crazy-max

Signed-off-by: CrazyMax crazy-max@users.noreply.github.com

@crazy-max

Signed-off-by: CrazyMax crazy-max@users.noreply.github.com

@crazy-max

G601: Implicit memory aliasing in for loop. (gosec)

Signed-off-by: CrazyMax crazy-max@users.noreply.github.com

@crazy-max

Did we try to upstream this?

Could we avoid copying this bunch of code by just providing our own JustAttributes() function that ranges over array and calls internal JustAttributes() for each item.

Add more context to this issue about the actual bake case it addresses and bake tests.

Opened an issue on HCL repo and got some feedback hashicorp/hcl#636 (comment). Creating our own implementation like Terraform does looks to be the right way.

tonistiigi

Labels