oci: Fix multiple srcs, hardlink accros device #75
Conversation
elthariel
changed the title
|
Thanks for these fixes @elthariel !. @Tatskaari do you think the tempdir config belongs in [buildenv] or should it be [buildconfig] with a CONFIG.get(), or doesn't it really matter? |
|
I think it would make more sense to do: And handle this in the build language rather than bash. |
|
So this is really a specific edge case and it's only used for one tmpdir, so that's why I haven't spent too much time there, but I'll do as you suggest |
|
I've added it both the build rule argument with a default from the config, as it seems to be the common way to do this |
oci/oci.build_defs
Outdated
| @@ -1,7 +1,8 @@ | |||
| def container_image( | |||
| name:str, base_image='', transitive_base_images=[], srcs=[],image='', version='', dockerfile='', | |||
| containerfile='', entrypoint=[], cmd=[], repo=CONFIG.get('DEFAULT_DOCKER_REPO', ''), | |||
| labels=[], run_args:str='', push_args:str='', test_only=False, visibility:list=None | |||
| oci_tmpdir=CONFIG.get('OCI_TMPDIR', '/tmp'), labels=[], run_args:str='', push_args:str='', | |||
There was a problem hiding this comment.
I see why you've done it but I would avoid having this in the arguments. It belongs in plzconfig only I think, as its to do with the platform, people with different tempdirs should ALWAYS be able to pull someone else's code and use the same rule in the same way, instead of relying on that field not being set in the build rule.
There was a problem hiding this comment.
I hear you, I thought about it afterward but got lazy. thanks for keeping me on the right path :)
|
Thanks, lgtm, over to you @Tatskaari |
This PR enables having multiple files in the
srcsparam. Without it, the whole list is treated as a single argument tomvand it fails with a file not found error.Also, if you ever happen to have you code on a different fs than /tmp (tmpfs anyone?), the oci build breaks when using a base image, because of the usage of hardlinks. This PR makes this specific tmp folder location configurable.