Add support for dockerfile_inline #1116
Conversation
Zeglius
force-pushed
the
dockerfile_inline
branch
3 times, most recently
from
e2c928b to
f2492f6
Compare
Fixes containers#864 Signed-off-by: Zeglius <[email protected]>
Zeglius
force-pushed
the
dockerfile_inline
branch
from
f2492f6 to
da10fdf
Compare
|
|
||
| args = container_to_build_args(c, cnt, args, lambda path: True) | ||
|
|
||
| import os |
There was a problem hiding this comment.
Should be imported at the top of the file.
podman_compose.py
Outdated
| dockerfile.close() | ||
| dockerfile = dockerfile.name | ||
| # Dont override the temporary dockerfile | ||
| if dockerfile_inline is None: |
podman_compose.py
Outdated
| dockerfile.write(dockerfile_inline.encode()) | ||
| dockerfile.close() | ||
| dockerfile = dockerfile.name | ||
| # Dont override the temporary dockerfile |
There was a problem hiding this comment.
Obvious comment, not necessary.
podman_compose.py
Outdated
| # Error if both `dockerfile_inline` and `dockerfile` are set | ||
| if dockerfile and dockerfile_inline: | ||
| raise OSError("dockerfile_inline and dockerfile can't be used simultaneously") | ||
| # Create a temporary dockerfile |
There was a problem hiding this comment.
Obvious comment, not necessary.
| if dockerfile and dockerfile_inline: | ||
| raise OSError("dockerfile_inline and dockerfile can't be used simultaneously") | ||
| # Create a temporary dockerfile | ||
| dockerfile = tempfile.NamedTemporaryFile(delete=False, suffix=".containerfile") |
There was a problem hiding this comment.
Current code does not remove the temporary file, does it? I think this is primary deficiency of the current approach. Not sure how to best solve it right now.
One solution would be to introduce a list of cleanup functions that need to be called before podman-compose exits. Then pass this list to container_to_build_args which would append an appropriate os.remove call to that list. And then after podman build completes, remove the file in e.g. finally.
Remove temporary dockerfile generated when using dockerfile_inline field
podman_compose.py
Outdated
| dockerfile = dockerfile.name | ||
| def cleanup_temp_dockfile(): | ||
| os.remove(dockerfile) if os.path.exists(dockerfile) else None | ||
| list.append(kwargs["cleanup_callbacks"], cleanup_temp_dockfile) |
There was a problem hiding this comment.
Why not:
if cleanup_callbacks is not None:
cleanup_callbacks.append(cleanup_temp_dockfile)
podman_compose.py
Outdated
| dockerfile.close() | ||
| dockerfile = dockerfile.name | ||
| def cleanup_temp_dockfile(): | ||
| os.remove(dockerfile) if os.path.exists(dockerfile) else None |
There was a problem hiding this comment.
Since return value is not used, normal if is more readable:
if os.path.exists(dockerfile):
os.remove(dockerfile)
podman_compose.py
Outdated
| @@ -2471,27 +2472,41 @@ async def compose_push(compose, args): | |||
| await compose.podman.run([], "push", [cnt["image"]]) | |||
|
|
|||
|
|
|||
| def container_to_build_args(compose, cnt, args, path_exists): | |||
| def container_to_build_args(compose, cnt, args, path_exists, **kwargs): | |||
There was a problem hiding this comment.
Better have cleanup_callbacks=None explicitly
|
Serious issues have been fixes, stylistic issues is the only thing left. Thank you for looking into this feature. |
…ed to None Signed-off-by: Zeglius <[email protected]>
Signed-off-by: Zeglius <[email protected]>
Signed-off-by: Zeglius <[email protected]>
Fixes #864
We dump the contents of
service.*.build.dockerfile_inlinein a temporary file, which gets fed to the-fflag.Also raises errors whenever both
dockerfile_inlineanddockerfileare set, just like docker does, see https://docs.docker.com/reference/compose-file/build/#dockerfile_inline