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

Add support for custom build-prerequisites Dockerfile #260

Merged

Conversation

slonopotamus
Copy link
Collaborator

This commit adds --prerequisites-dockerfile=[path] flag that allows injecting custom image in place of ue4-build-prerequisites.

Note that vanilla ue4-build-prerequisites is still build and available with ue4-base-build-prerequisites name, so user-provided Dockerfile can use it as a base.

Primary goal of this feature is to allow installation of platform SDKs before engine is checked out into image. Placing platform SDKs in to ue4-build-prerequisites image allows to reuse the same SDKs for multiple engine builds.

@TBBle
Copy link
Collaborator

TBBle commented May 31, 2022

This seems like it would have "interesting" interactions with the 'export combined' mode. We might want to have a template that shows how to create a rational custom Dockerfile (i.e. difference in FROM calls between combined and separate mode), or block it; either one is clear.

Also, if I'm parsing this correctly, we copy the whole context directory before we run build. It's cheap for the built-in images, but might be expensive if the custom Dockerfile is pulling in, say a large console SDK image, which can easily be over a gigabyte.

In fact, I realise that copying the context directory is probably needless in all cases, as we're only doing it (AFAICT) so we can read the Jinja'd Dockerfile instead of the one in the tree, and as it happens, the Dockerfile doesn't need to be in the context, so we could template-process the Dockerfile into a tempfile, and use the in-tree context with the tempfile as the Dockerfile.

Unless that's something that BuildKit introduced; I realise now I've not checked this explicitly with the legacy builder, but I habitually include Dockerfile in my .dockerignore without issues on very old Docker daemons (pre-BuildKit). Anyway, this is probably a question distinct from this PR.

@slonopotamus
Copy link
Collaborator Author

slonopotamus commented May 31, 2022

we copy the whole context directory before we run build.

This PR already stopped doing that, see https://github.com/adamrehn/ue4-docker/pull/260/files#diff-5e6ae65114a21de05700d099ef76362cd0b68348cefe4f45b2b17f9a99033509L91-L94

Now only Jinja-processed Dockerfile is written to temp dir.

The separation of Dockerfile/context paths is the main reason of this PR size, I had to tweak several things that relied on them living in the same directory.

@TBBle
Copy link
Collaborator

TBBle commented May 31, 2022

Ohh, whoops. I thought the shutil.copytree had moved because I saw it later in the PR, but I realise (now that you pointed it out) that the remaining usage is part of the 'export' mode. >_< Sorry for the noise.

@slonopotamus
Copy link
Collaborator Author

There are some design questions here:

  1. Should we only support path to Dockerfile (and infer context path from it) or we should support context path (and infer Dockerfile from it) PLUS support custom path to Dockerfile, like Docker itself does.
  2. Should we always build both images (ue4-base-build-prerequisites/ue4-build-prerequisites) and use some empty stub for default ue4-build-prerequisites or we should go the way I did it, when there are no traces of ue4-base-build-prerequisites unless you use --prerequisites-dockerfile feature.

This commit adds `--prerequisites-dockerfile=[path]` flag that allows injecting custom image in place of ue4-build-prerequisites.

Note that vanilla ue4-build-prerequisites is still build and available with ue4-base-build-prerequisites name, so user-provided Dockerfile can use it as a base.

Primary goal of this feature is to allow installation of platform SDKs before engine is checked out into image. Placing platform SDKs in to ue4-build-prerequisites image allows to reuse the same SDKs for multiple engine builds.
@slonopotamus slonopotamus force-pushed the custom-prerequisites-dockerfile branch from 93a674c to 48e929f Compare June 7, 2022 16:01
@slonopotamus
Copy link
Collaborator Author

Okay, merging this as-is (but we will pretend that this feature is experimental and subject to change).

@slonopotamus slonopotamus merged commit d973bd8 into adamrehn:master Jun 8, 2022
@slonopotamus slonopotamus deleted the custom-prerequisites-dockerfile branch June 8, 2022 07:14
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.

2 participants