Add support for specifying multiple base images to use in the spec#492
Add support for specifying multiple base images to use in the spec#492cpuguy83 merged 4 commits intoproject-dalec:mainfrom
Conversation
cpuguy83
force-pushed
the
multi-base
branch
6 times, most recently
from
65d3fec to
87ac6a8
Compare
cpuguy83
force-pushed
the
multi-base
branch
2 times, most recently
from
81e57da to
cabf8c6
Compare
| // which means we end up overwriting map keys when there are multiple windows | ||
| // platform images being output but with different OSVerions. | ||
| // platforms.FormatAll takes OSVersion into account. | ||
| func dcBuild(ctx context.Context, bc *dockerui.Client, fn dockerui.BuildFunc) (*resultBuilder, error) { |
There was a problem hiding this comment.
Is it really a good idea to fork this code? I generally think that's not a good idea, as we won't benefit from upstream changes if there are any.
There was a problem hiding this comment.
We have to fork it to get the functionality we need.
There was a problem hiding this comment.
Can these changes be submitted upstream? Then we wouldn't need to fork it.
I'm sure there's a way to preserve the old behavior while allowing the new.
There was a problem hiding this comment.
Here is how it could be done:
pmengelbert/buildkit@7552611
Whether or not that's acceptible upstream, I think we should at least commit to a plan to undo the fork if there's no practical way around forking.
There was a problem hiding this comment.
moby/buildkit#5614 upstreams this work.
There was a problem hiding this comment.
Can you add a link to moby/buildkit#5614 in the comment?
There was a problem hiding this comment.
Upstream is merged.
Looks like you have an issue already to unfork this. Seems like should be good?
MorrisLaw
left a comment
MorrisLaw
left a comment
There was a problem hiding this comment.
First pass lgtm! Don't have a whole lot of context though. Left some nits and clarification questions.
This is pretty much to support building for multiple windows versions from windowscross but can also be useful (in the future) for allowing users to specify something other than a registry reference to provide an image. The actual support for multiple bases, where it can be supported, will come in a follow-up commit.
Signed-off-by: Brian Goff <cpuguy83@gmail.com>
This is useful for when you want to have one image for different versions of Windows, given that for Windows containers the container OS must match the host OS. Signed-off-by: Brian Goff <cpuguy83@gmail.com>
Signed-off-by: Brian Goff <cpuguy83@gmail.com>
3 participants
Allows windowscross to produce a single multiplatform image using the specified base images.
Supersedes #480
Closes #480