Ok seems it follows compose behavior https://docs.docker.com/reference/cli/docker/compose/#set-up-environment-variables but I would tend to error out in such case. Although with bake we print the loaded definitions during build so at least user is aware of performed definitions.

I would tend to error out in such case

That would be pretty unusual and unexpected I would think. If it contradicted with another variable/flag that would be one thing. Otherwise I wouldn't see it as any different than BUILDKIT_PROGRESS vs. --progress, BUILDX_BUILDER vs. --builder, etc., where a flag always silently wins over the environment. I guess in those cases it's a little different in that those are single value options, whereas these files are multi-value, and maybe you're concerned about user expectations regarding whether flags and environment are additive in this case?

Let me know if I misunderstood or misinterpreted your comment (or if I did read it correctly and you still feel it should be an error).

Otherwise I wouldn't see it as any different than BUILDKIT_PROGRESS vs. --progress, BUILDX_BUILDER vs. --builder, etc., where a flag always silently wins over the environment.

Yes indeed, hence why we print builder used and bake definitions loaded in the logs as "hints". So agreed to stay consistent.

we print builder used and bake definitions loaded in the logs as "hints"

Incidentally, that is one of the main reasons I deviated from compose's behavior of converting environment-provided files to absolute paths (which I noted in the commit comment). An absolute path made the output very ugly. And after seeing the input files were added as trace attributes, I figured not forcing an absolute path would simply avoid the question of whether it would be (or needed to be) scrubbed or not.

Just wanted to call that out in case it triggers any thoughts. I thought/hoped by putting the processing where I did, the questions/rules about where relative paths are anchored would be no different than for --file.

And after seeing the input files were added as trace attributes, I figured not forcing an absolute path would simply avoid the question of whether it would be (or needed to be) scrubbed or not.

You mean through the traced command in

What I meant by "hence why we print builder used and bake definitions loaded in the logs" is when invoking the command you have the loaded bake definitions displayed in progress output:

$ docker buildx bake --print
#1 [internal] load local bake definitions
#1 reading docker-bake.hcl 8.03kB / 8.03kB done
#1 DONE 0.0s

I was thinking that maybe we could mark files loaded from an env var with another message like:

$ docker buildx bake --print
#1 [internal] load bake definitions from BUILDX_BAKE_FILE env
#1 reading docker-bake.hcl 8.03kB / 8.03kB done
#1 DONE 0.0s

So user is not confused.

You mean through the traced command in

Yes, exactly; I didn't follow the entire code path, but that is exactly the code I saw as I was trying to figure out where best to add my new code. I know many folks consider an absolute path leakage (e.g., seeing a user's directory structure and username). I don't know if a full path would have actually been passed through or if this community would have cared even if it was. Of course if a user specifies absolute paths that's a different story. But I saw no technical reason/advantage to converting relative paths to absolute, and a few downsides.

What I meant by [...]
I was thinking that maybe we could mark files loaded from an env var with another message like

Yup, I knew what you meant (the UI portion), but I didn't realize your concern was around communicating which mechanism was used to specify the file; I thought it was around that a file was used (vs. ignored). Sorry I missed that!

So yes, I can do that. Quickly looking at the code, I think there's going to be a choice between doing it:

• "simply"; doing a second lookup in readBakeFiles where that printing occurs to see if that environment variable exists, and if does, assume it was respected. This is obviously the case today, but not really future-safe.
• "correctly"; tracking the fact it was specified via environment, as opposed to the simple sleight-of-hand I'm currently doing (directly populating in.files from the environment as if they had been specified via -f)

I'll try to find an unobtrusive way to do this "correctly", but if you actually prefer "simply" in this case, let me know.

Let me know what you think of what I pushed. Most of the 'good' solutions I could see entailed adding code disproportionate to the feature itself or changing the flow (which you already said you liked). I left it as a separate commit anticipating you might want (possibly substantial) changes or even opt to drop it.

I assumed that your suggestion to to specify the full message on the one test was intended to cover all the tests, so I did just that.