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: allow user functions in variables and vice-versa #575

Merged
merged 2 commits into from
Jun 23, 2021

Conversation

tonistiigi
Copy link
Member

@tonistiigi tonistiigi commented Mar 27, 2021

depends on #541

Removes previous limitations where user-defined functions could not be used in variables and function definitions could not call other functions.

I at least temporarily needed to fork userfunc package to make this work. The package is not very big but I already sent a PR to upstream with the same refactor. A more complicated story is parsing function dependencies from JSON that isn't really available with the current API. I added a hacky version with reflect package and opened an issue in upstream to find a better solution.

Some of the parsing logic could use a reorganization but didn't want to do it yet as much work is still needed to get #445 working first. After that, we can think about cleanups.

Signed-off-by: Tonis Tiigi <tonistiigi@gmail.com>
@tonistiigi tonistiigi force-pushed the user-func-vars branch 2 times, most recently from 2ca6218 to ce655c4 Compare March 27, 2021 00:13
bake/hcl.go Show resolved Hide resolved

def, ok := sc.attrs[name]
func appendJSONFuncCalls(exp hcl.Expression, m map[string]struct{}) error {
v := reflect.ValueOf(exp)
Copy link
Collaborator

Choose a reason for hiding this comment

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

if you want to make this hackier but more readable, you might as well abuse //go:linkname to import those private structs.

Copy link
Member Author

Choose a reason for hiding this comment

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

not that it is any cleaner but I don't think it would even work. I don't just need access to a private function/var in here.

@crazy-max
Copy link
Member

@tonistiigi Tested on my side and LGTM. Maybe we should wait for hashicorp/hcl#455 before merge?

@tonistiigi
Copy link
Member Author

I don't think we can wait for hashicorp/hcl#455 . If there is movement we can replace the code.

Signed-off-by: Tonis Tiigi <tonistiigi@gmail.com>
@tonistiigi tonistiigi merged commit 03b7128 into docker:master Jun 23, 2021
@crazy-max crazy-max mentioned this pull request Jul 27, 2021
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.

3 participants