Show types during variable list operation

[rrjjvv]

If a type was explicitly provided, it will be displayed in the variable listing. Inferred type names are not displayed, as they likely would not match the user's intent.

Previously only string and bool default values were displayed in the listing. All default values, regardless of type, are now displayed.

Fixes #3202

[crazy-max]

Thanks! If you could also update docs at

buildx/docs/reference/buildx_bake.md

Lines 197 to 205 in e3c6618

	To list variables:
	```console
	$ docker buildx bake --list=variables
	VARIABLE VALUE DESCRIPTION
	REGISTRY docker.io/username Registry and namespace
	IMAGE_NAME my-app Image name
	GO_VERSION <null>
	```

that would be great.

[crazy-max]

Thanks!

[tonistiigi]

nit: no ', /s/ever/never/

[rrjjvv]

A nit for you, but embarrassing for me 😄. Fixed. For future reference, let me know if you prefer these fixes as new commits or squashed/force-pushed.

[tonistiigi]

seem to be missing a case with just variable block without type and default, and implied string cases with default.

I think it would also be ok to show the type even if it is inferred from default, as long as it is not a string. But if it complicated things then we can skip that.

[rrjjvv]

it would also be ok to show the type even if it is inferred from default

It wouldn't complicate things. The main reason I didn't is for the case of

variable "VERSIONS" {
  default = ["1.2", "1.3"]
}

A reader "knows" that it's a list(string) ("list of string") or maybe set(string) ("set of string"), but the inferred type is tuple([string, string]) ("tuple").

The inferred types for a primitive is probably going to be correct 99% of the time, but thought it better to be consistent. But it is not complicated or difficult if that's what you'd prefer.

[tonistiigi]

Why is it detected as "tuple" in this case? I would guess that an array is more generic and a tuple is a subset of array.

[rrjjvv]

seem to be missing a case with just variable block without type and default, and implied string cases with default.

Both of these were covered in an existing test (foo and def):

buildx/tests/bake.go

Lines 1716 to 1724 in e3c6618

	variable "foo" {
	default = "bar"
	description = "This is foo"
	}
	variable "abc" {
	default = null
	}
	variable "def" {
	}

[rrjjvv]

Why is it detected as "tuple" in this case? I would guess that an array is more generic and a tuple is a subset of array.

The rationale is probably "since I can only see two elements both of which are strings, I can't guarantee you can safely add/remove elements at will". But more explicitly from the docs:

Only tuple and object values can be directly constructed via native syntax. Tuple and object values can in turn be converted to list, set and map values with other operations, which behaves as defined by the syntax-agnostic HCL information model.

[tonistiigi]

I think let's leave it with 1 then, unless there is some simple trick to make 4 possible

[rrjjvv]

In short, I think we'll have to go with (1); I'm not sure we'll find any truly simple solution for (4).

In hindsight, I should have never started batting around "inferred type". Hopefully I'm not responsible for any confusion. If we were truly dealing with inferred types (types inferred by how/where they're used), we probably wouldn't be having this discussion (to this extent at least). An actual inferred type would be the solution you're hoping for. We're just dealing with the simplest type that can be guaranteed to represent a specific value.

I'll take another stroll through the docs and code to see if there's a capability there that I missed, but I'm not hopeful. Probably the best we'll get is some insight or tools to make our own heuristics simpler/safer to implement. I'll let you know what I find.

[rrjjvv]

From what I can see, there is no general solution for (4) that is also simple. But if you're willing to sacrifice correctness for the 1% intending to use tuples for the sake of the 99% using lists (an earlier comment suggests you might), there could be a 'simple' (in terms of bake code) solution.

Via the cty "unification" functions, I was basically able to say "I have ["hi", "there"]; can that be made into a list of some type?" And it will (not just whether it can be done, but exactly what type).

But... given ["hello", 99], it says "it can be made into a list(string) (via coercion)". Under normal circumstances, something that superficially looks like a list but contains elements of various types would usually be considered a tuple. So increasing accuracy for list users may come at the expense of decreasing accuracy for tuple users.

There are two main variables in play specific to this functionality... 'safe' vs. 'unsafe', and whether cty.DynamicPseudoType is being utilized; I have not experimented with all combinations.

Let me know what you think... if you think this deserves more exploration, or if it's fine to leave as-is.

[tonistiigi]

Let's just leave it to 1. No need to overthink it.

[rrjjvv]

k. I guess there's another flavor of (4)... rather than try guessing the complex types, we don't even bother trying and simply fill the column with "other", "unknown", "<missing>", or similar.

That wouldn't provide any real value (though could be a subtle hint to the user that they can provide it), but would make the output look a little nicer (no gaps), would be easy, and would not be inaccurate.

If you like that option and give me an exact term to use, I can get that implemented later. Otherwise, I guess we're done?

[tonistiigi]

LGTM but squash the commits