Ensure Dockerfile WORKDIR C:\ becomes / in LLB

[TBBle]

Putting this up for review as a first step towards implementing #616 (comment), to make sure we're happy with this approach. parseAdd will need similar fixes for its sources and destination arguments.

This change doesn't make WORKDIR actually work, as there are different issues when applying the LLB back to the filesystem, but those can be fixed separately.

[tonistiigi]

Maybe instead of using filepath and making decisions based on the native architecture, it would be better to move this to dockerfile2llb and use the platform of the dispatchstate

buildkit/frontend/dockerfile/dockerfile2llb/convert.go

Line 546 in 3f3957d

platform *specs.Platform

to make the decision about converting paths. Would like that windows dockerfile that doesn't use RUN would work on Linux as well, like it works today if unix paths are used.

This looks somewhat similar to #1560 that you also might want to take.

[TBBle]

To be clear, is the idea that WORKDIR C:\test should work when running a Linux-based build against the Windows targetPlatform? That seems reasonable, but will require a little bit more learning on my part.

I assume WORKDIR C:\test should fail if the targetPlatform is Linux... Or rather, that's possibly a valid filename, but it's relative, and it's the filesystem layer's problem to reject it.

I like this more, because one of the outstanding concerns I had with my tests was the runtime platform check. I can split the current test into two, and confirm that we get rational behaviour for both Linux and Windows targetPlatform, irrespective of the execution platform.

[tonistiigi]

To be clear, is the idea that WORKDIR C:\test should work when running a Linux-based build against the Windows targetPlatform?

Yes. This works today (outside Moby integration). But I think today you just always need to use unix paths that should work even if you build for windows. Normally, I would just always recommend using unix paths in Dockerfiles but because old builder already supports windows paths BuildKit should as well. But the same Dockerfile should be usable for building natively on Windows and cross-building on Linux.

I assume WORKDIR C:\test should fail if the targetPlatform is Linux...

Yes, we shouldn't try to do any normalization magic for linux targets. If target is windows we can check if it is a windows style path and normalize to unix path if needed.

[tonistiigi]

ping @TBBle

[TBBle]

Sorry, I haven't had a chance to get back to this PR and rework it to apply the discussion outcome from the first review.

I planned to get back to this when the containerd PRs for Windows support landed, but they still haven't, and the whole thing fell off my radar.

Converted to draft, to make that clear. It really should have always been a draft, based on my own initial description. ^_^

[tonistiigi]

I guess this has been superseded with the WCOW patches.

[TBBle]

Probably true, yeah. I'll keep the branch around since all I'd done was add some tests, and it's possible it's still a coverage gap to close. But I expect the actual code is operating correctly now.