expand additional context relative paths#385
Conversation
|
@crazy-max does it sound reasonable to assume a custom context type will always include some |
Named context target can also be a path. Such as with buildx: docker buildx build --build-context project=path/to/project/source .Otherwise yes it's either More info: https://docs.docker.com/engine/reference/commandline/buildx_build/#build-context |
|
yes, that's my point: when not prefix is given, we can check this is a path or git url (just like for |
milas
left a comment
There was a problem hiding this comment.
Change to Mapping from MappingWithEquals violates the spec
|
Thanks for fixing this issue! Please consider updating the compose spec to clarify that relative paths should be expanded. |
Signed-off-by: Nicolas De Loof <nicolas.deloof@gmail.com>
bc97dc4 to
1592cda
Compare
When using `extends.file` to load a service from another Compose file, if the `build.context` field wasn't explicitly set, it would get ignored by the path resolution. As a result, the build context would end up as the working directory of the _root_ Compose file instead of the working directory of the _loaded_ Compose file. This was because the relative path resolution logic ignored empty `build.context` values. Unfortunately, removing that restriction is itself not sufficient, as it then attempted to verify that the local path existed in an attempt to avoid unintentionally joining the working directory with a remote context value (e.g. `/foo/https://github.com/my/repo.git`). This is problematic because the working directory itself might be relative (rooted to an unknown location that is != `.`), so it will be resolved relative to the current process working directory, and thus fail the existence check. In practice, this happens when using `extends.file`, where we do resolve paths but intentionally pass a potentially relative value for the working dir, thus making it unsafe to be doing real filesystem operations here. In fact, even if a context was specified, it was possible to break this by running Compose from a different directory than the _root_ Compose file while specifying the path to it. As there's no formal specification for determining local path vs. remote build contexts (see discussion in compose-spec#385), I'm simply eliminating the existence check. This COULD mean that Compose begins to erroneously consider remote context values as local paths if builders add new unsupported syntaxes, but I think it's fair for us to be more restrictive. Additionally, I've ensured that when path resolution is happening, it _always_ resolves the `build.context` to an absolute path for consistency. In particular, this should help make it easier to use the output of `docker compose config` from arbitrary working directories. There's a new test that covers the `extends.file` + `build.context` behavior, and everal existing test adjustments to account for the fact that Compose was emitting relative `build.context` paths from `docker compose config` despite everything else being absolute (such as volumes). Fixes docker/for-mac#6912. Signed-off-by: Milas Bowman <milas.bowman@docker.com>
When using `extends.file` to load a service from another Compose file, if the `build.context` field wasn't explicitly set, it would get ignored by the path resolution. As a result, the build context would end up as the working directory of the _root_ Compose file instead of the working directory of the _loaded_ Compose file. This was because the relative path resolution logic ignored empty `build.context` values. Unfortunately, removing that restriction is itself not sufficient, as it then attempted to verify that the local path existed in an attempt to avoid unintentionally joining the working directory with a remote context value (e.g. `/foo/https://github.com/my/repo.git`). This is problematic because the working directory itself might be relative (rooted to an unknown location that is != `.`), so it will be resolved relative to the current process working directory, and thus fail the existence check. In practice, this happens when using `extends.file`, where we do resolve paths but intentionally pass a potentially relative value for the working dir, thus making it unsafe to be doing real filesystem operations here. In fact, even if a context was specified, it was possible to break this by running Compose from a different directory than the _root_ Compose file while specifying the path to it. As there's no formal specification for determining local path vs. remote build contexts (see discussion in compose-spec#385), I'm simply eliminating the existence check. This COULD mean that Compose begins to erroneously consider remote context values as local paths if builders add new unsupported syntaxes, but I think it's fair for us to be more restrictive. Additionally, I've ensured that when path resolution is happening, it _always_ resolves the `build.context` to an absolute path for consistency. In particular, this should help make it easier to use the output of `docker compose config` from arbitrary working directories. There's a new test that covers the `extends.file` + `build.context` behavior, and everal existing test adjustments to account for the fact that Compose was emitting relative `build.context` paths from `docker compose config` despite everything else being absolute (such as volumes). Fixes docker/for-mac#6912. Signed-off-by: Milas Bowman <milas.bowman@docker.com>
With introduction for
additional_contextsa user can set context to a local path, and as we support relative paths in many places it also make sense to expand relative paths for such additional contexts. Challenge here is that such a context can be a path or URL (just like context) but also builder specific string. I'm assuming here such a custom string will ALWAYS use a<type>://prefix as buildx doessee docker/compose#10369